Skip to content

Conversation

Akirathan
Copy link
Member

@Akirathan Akirathan commented Sep 8, 2025

Pull Request Description

Semi-formal spec for method invocation. Documents the desired behavior that is being implemented in #13962.

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    TypeScript,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • Unit tests have been written where possible.
  • If meaningful changes were made to logic or tests affecting Enso Cloud integration in the libraries,
    or the Snowflake database integration, a run of the Extra Tests has been scheduled.
    • If applicable, it is suggested to paste a link to a successful run of the Extra Tests.

@Akirathan Akirathan self-assigned this Sep 8, 2025
@Akirathan Akirathan added the CI: No changelog needed Do not require a changelog entry for this PR. label Sep 8, 2025
@Akirathan
Copy link
Member Author

RFC engine and libs team (@JaroslavTulach , @jdunkerley , @4e6 , @hubertp , @GregoryTravis ): Do you consider this kind of semi-formal specification for method resolution and method evaluation useful? Is it better than looking at our tests or in our code base? Do you want me to finish it? Do you have any other suggestions how to improve it? Note that this PR is WIP, obviously.

- Every type has an implicit parent type `Any`.
- Except for `Number` which is a special builtin case with a deeper hierarchy.
- `Any` has no parent type.
- 2.2. No parent type is present. Raise `No_Such_Method` panic and stop.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- 2.2. No parent type is present. Raise `No_Such_Method` panic and stop.
- 2.2. No parent type is present. Raise `No_Such_Method` panic and terminate.


### Lexical scope lookup

- If `symbol` is defined in the current lexical scope, select it and stop.
Copy link
Member

@JaroslavTulach JaroslavTulach Sep 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • This is not really a "dynamic dispatch"
  • but it is an interesting topic
  • and complicated - e.g. term like all transitively imported modules sounds scary
  • I'd move it to its own "lexical dispatch" document

@enso-bot enso-bot bot mentioned this pull request Sep 15, 2025
2 tasks
@Akirathan
Copy link
Member Author

I believe that as of ed299f5, this is really the final specification that should work for all our use cases. As demonstrated by the updated tests in 7b58937

@JaroslavTulach
Copy link
Member

I believe that as of ed299f5, this is really the final specification that should work for all our use cases. As demonstrated by the updated tests in 7b58937

This diff is more important than #13961 (comment) and it’s 7b58937 as it shows the diff against current behavior. Can we make it less red?

@Akirathan
Copy link
Member Author

This diff is more important than #13961 (comment) and it’s 7b58937 as it shows the diff against current behavior. Can we make it less red?

@JaroslavTulach Done. See #13962 (comment)

var exp1 = raw;
var exp2 = ctx.ensoContext().getBuiltins().any();
assertArrayEquals(
"allTypes(Singleton_Type.type) == [Singleton_Type.type, Any]",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or is it

Suggested change
"allTypes(Singleton_Type.type) == [Singleton_Type.type, Any]",
"allTypes(Singleton_Type) == [Singleton_Type, Any]",

;-?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am really not sure about this one. See the line 159 with assertThat("is eigen type", raw.isEigenType(), is(true)). So I think it should be written as Singleton_Type.type.

@Akirathan
Copy link
Member Author

Akirathan commented Oct 7, 2025

Changes introduced in Type.fillInTypes necessary to make the newly introduced TypeChainTests green, breaks other stuff. For example in https://github.com/enso-org/enso/actions/runs/18308891816/job/52133745709?pr=13961#step:14:1114.

Can be reproduced with:

vec = [1,2,3]
vec.at 100 == Nothing

which, on this PR, returns False, but should return Index_Out_Of_Bounds error.

This is caused by the fact, that on develop allTypes(Error) == [Error], but on this PR, allTypes(Error) == [Error, Any]. In turn, this means that an UnresolvedSymbol("==").resolveFor(Error) will resolve to Any.== which is incorrect.

TypeChainTest as well as changes in Type are necessary for #13962. Let's move it to that PR and keep this PR only about documentation.


Moved in 1522b50 and 5a40ad8

Akirathan added a commit that referenced this pull request Oct 7, 2025
@JaroslavTulach
Copy link
Member

keep this PR only about documentation.

  • (Unit) tests are essential part of documentation.
  • I prefer to keep the test and just disable (or @Ignore) the parts that are known to diverge
  • Leave comments with references to tasks that will fix the disabled parts

@Akirathan
Copy link
Member Author

@JaroslavTulach Tests are again in this PR, with a single ignore.

@Akirathan Akirathan merged commit 55c6354 into develop Oct 8, 2025
104 of 106 checks passed
@Akirathan Akirathan deleted the wip/akirathan/docs-methods branch October 8, 2025 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI: No changelog needed Do not require a changelog entry for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants