Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Equality on types with Any as LHS does not work #5612

Closed
Akirathan opened this issue Feb 10, 2023 · 15 comments · Fixed by #7033
Closed

Equality on types with Any as LHS does not work #5612

Akirathan opened this issue Feb 10, 2023 · 15 comments · Fixed by #7033
Assignees
Labels
--bug Type: bug -compiler p-low Low priority
Milestone

Comments

@Akirathan
Copy link
Member

Any == Boolean returns a function Any.type.== instead of False. This is because it resolves as Any.type.== Any Boolean method call (which expects 3 arguments rather than 2). Note that Boolean == Any returns False as expected, because it resolves to Any.== Boolean Any

May be related to #4077

Related pending test: https://github.com/enso-org/enso/blob/develop/test/Tests/src/Semantic/Equals_Spec.enso#L172

@jdunkerley jdunkerley moved this from ❓New to 📤 Backlog in Issues Board Feb 10, 2023
@jdunkerley jdunkerley added the p-low Low priority label Feb 10, 2023
@jdunkerley jdunkerley added this to the Beta Release milestone Feb 10, 2023
@JaroslavTulach
Copy link
Member

If we implement #6300 then Any == Boolean should treat == as an instance invocation of == on Any and this issue should be resolved.

@wdanilo
Copy link
Member

wdanilo commented Apr 18, 2023

Why Any == Boolean resolves to Any.type.==? What is the logic behind this resolution?

@Akirathan
Copy link
Member Author

Why Any == Boolean resolves to Any.type.==? What is the logic behind this resolution?

When the engine executes Any == Boolean, it invokes MethodResolverNode.execute(self=Any, type=Any.type, symbol=UnresolvedSymbol<==>), which invokes UnresolvedSymbol(==).resolveFor(Any.type), which finds Any.type.== because == is both a static and instance method, as are all the other methods. And Any.type.== expects 3 arguments, as opposed to Any.==. Therefore, it returns a curried function.

This problem is related to #6216. This is also the reason why, e.g., Boolean == Vector will resolve to Any.==

Note that, however, Boolean == Vector will correctly resolve to Any.== Boolean.type Vector.type, as it transitively calls into UnresolvedSymbol(==).resolveFor(Boolean.type) and Boolean.type has Any as its parent, therefore, the instance method Any.== is found.

I know this is all really kind of a mess. This is why there is a huge discussion in #6216 that should resolve this, and all the related problems with instance VS static methods, once and for all.

@JaroslavTulach
Copy link
Member

... huge discussion in #6216 that should resolve this, and all the related problems with instance VS static methods, ...

The discussion in #6216 is huge and touches a lot of important topics including syntax of the Enso language. As such I'd like to reword your conclusion Pavel:

#6300 is a small and non-controversial subset of the #6216 discussion with as limited scope as possible - only methods defined on Any would resolve differently for the Type objects - e.g. as instance methods. As == is defined on Any, it would resolve as an instance method and that's enough to fix the #5612 problem.

once and for all.

Not only #6300 solves this problem, but there are other "duplicates" it solves as well. As such #6300 got a "go" yesterday. However I wouldn't say it fixes "everything for once and all" - that's the goal for #6216 which remains open for discussion.

@JaroslavTulach
Copy link
Member

JaroslavTulach commented Apr 28, 2023

#6300 got integrated, but alas, Any == Boolean still resolves to a function, not False.

Additions to #6300

Every instance method defined on Any has default self argument set to the type. My_Type_Without_To_Text.to_text becomes Any.to_text self=My_Type_Without_To_Text. If one wants the static method, one would write My_Type_Without_To_Text.to_text... and the result would be a function with one (defaulted) self argument.

To invoke any instance method (defined on Any) one could:

  • My_Type.m - a shortcut for My_Type.m self=My_Type
  • My_Type.m self=other - one could define different self as a named argument
  • My_Type.m other - first argument of the function is going to be self, so this has the same meaning as the previous line
  • My_Type.m... - returns a function with defaulted self argument

Current behavior on Any is:

Any.to_text
>>> org.enso.interpreter.runtime.callable.function.Function - that's wrong, we need defaulted self
Any.to_text self=Boolean
>>> Boolean - good
> Any.to_text Boolean
>>> Boolean - good
> Any.to_text...
>>> org.enso.interpreter.runtime.callable.function.Function - that's correct

the same would work for == as well:

> Any == Boolean
>>> org.enso.interpreter.runtime.callable.function.Function - that's wrong, we need defaulted self
> Any.== self=Any Boolean
>>> false - good
> Any.== Any Boolean
>>> false - good
> Any.== self=Boolean Boolean
>>> true - good
> Any.==...
>>> org.enso.interpreter.runtime.callable.function.Function - that's correct

Nothing from the above is new, all the code already parses and runs somehow. For example following code already behaves as needed:

> f = Any.==...
> f
>>> org.enso.interpreter.runtime.callable.function.Function
> f self=Boolean Boolean
>>> true
> f Boolean Boolean
>>> true
> f Any Boolean
>>> false

What doesn't work is:

> f Boolean
>>> org.enso.interpreter.runtime.callable.function.Function - with defaulted self, this'd be false
> f Any
>>> org.enso.interpreter.runtime.callable.function.Function - this should be true

We just need to provide the defaulted self argument for instance methods defined on Any. Then we have a chance to resolve the Any == Boolean as well as Any.to_text still returning function.

@hubertp hubertp assigned hubertp and unassigned Akirathan Apr 28, 2023
@hubertp
Copy link
Contributor

hubertp commented Apr 28, 2023

I wasn't aware of this ticket. Will add it to my list of follow ups.

@jdunkerley jdunkerley moved this from 📤 Backlog to ❓New in Issues Board May 30, 2023
@jdunkerley jdunkerley moved this from ❓New to 📤 Backlog in Issues Board May 30, 2023
@Akirathan Akirathan moved this from 📤 Backlog to 🔧 Implementation in Issues Board Jun 14, 2023
@Akirathan Akirathan linked a pull request Jun 14, 2023 that will close this issue
5 tasks
@enso-bot
Copy link

enso-bot bot commented Jun 14, 2023

Pavel Marek reports a new STANDUP for today (2023-06-14):

Progress: - Looking at relevant discussions, fiddling around

  • Scatching a solution in a draft PR It should be finished by 2023-06-20.

@enso-bot
Copy link

enso-bot bot commented Jun 16, 2023

Pavel Marek reports a new STANDUP for yesterday (2023-06-15):

Progress: Writing some tests, gathering information from relevant issues. It should be finished by 2023-06-20.

@enso-bot
Copy link

enso-bot bot commented Jun 16, 2023

Pavel Marek reports a new STANDUP for today (2023-06-16):

Progress: - Finally understood the specification that is desired.

  • Tweaking InvokeMethodNode such that the static method calls on Any are handled specifically - mostly a self default argument is prepended. It should be finished by 2023-06-20.

@JaroslavTulach
Copy link
Member

  • a self default argument is prepended

I believe you need to append (not prepend) the default self argument. That's syntactically impossible in Enso language (there is a check in IR passes that self is always the first argument), but internally achievable - there should be no problem in having self as the last argument in the runtime, I believe.

@enso-bot
Copy link

enso-bot bot commented Jun 19, 2023

Pavel Marek reports a new STANDUP for today (2023-06-19):

Progress: - Still hacking InvokeMethodNode to build a new InvokeFunctionNode for static method calls on Any.

  • Still bumping into issues with consistency of FunctionSchema of a method on Any.type versus the call argument informations. It should be finished by 2023-06-20.

@enso-bot
Copy link

enso-bot bot commented Jun 20, 2023

Pavel Marek reports a new STANDUP for today (2023-06-20):

Progress: - More progress on the Any static method calls.

  • Seems pretty complicated now, as a node replacement is required.
  • Import/export discussion, private keyword introduction proposal. It should be finished by 2023-06-20.

@enso-bot
Copy link

enso-bot bot commented Jun 22, 2023

Pavel Marek reports a new 🔴 DELAY for yesterday (2023-06-21):

Summary: There is 3 days delay in implementation of the Equality on types with Any as LHS does not work (#5612) task.
It will cause 0 days delay for the delivery of this weekly plan.

I introduced a hack and I am not sure it will be accepted by reviewers.

Delay Cause: A slight delay of PR reviews.

@enso-bot
Copy link

enso-bot bot commented Jun 22, 2023

Pavel Marek reports a new STANDUP for yesterday (2023-06-21):

Progress: - Resolved more issues, tests seem to be passing now. It should be finished by 2023-06-23.

@enso-bot
Copy link

enso-bot bot commented Jun 22, 2023

Pavel Marek reports a new STANDUP for today (2023-06-22):

Progress: - Integrated all the PR review, all tests are passing.

  • Should be merged today, or tomorrow. It should be finished by 2023-06-23.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
--bug Type: bug -compiler p-low Low priority
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants