Invoke instance methods for Any overrides #6441
Conversation
This change modifies method dispatch for methods that override Any's definitions. When an overrided method is invoked statically we call Any's method to stay consistent. This change primarily addresses the plethora of problems related to `to_text` invocations. It does not attempt to completely modify method dispatch logic. Closes #6300.
hubertp
force-pushed
the
wip/hubert/6300-to-text
branch
from
67c3ffb to
190cdd5
Compare
hubertp
added
the
CI: No changelog needed
hubertp
requested review from
jdunkerley,
radeusgd,
GregoryTravis,
4e6 and
JaroslavTulach
as code owners
| // and the method is invoked statically, i.e. type of self is the eigentype, | ||
| // then we want to disambiguate method resolution by always resolved to the one in Any. | ||
| if (where instanceof MethodRootNode node && typeCanOverride(node, EnsoContext.get(this))) { | ||
| Function anyFun = symbol.getScope().lookupMethodDefinition(EnsoContext.get(this).getBuiltins().any(), symbol.getName()); |
There was a problem hiding this comment.
Alternatively, we could resolve symbol on Any first and then on selfTpe. That unfortunately leads to more complicated checks (one problematic example is for example is_a that is defined on Any, Error, Meta and who knows what else) so I didn't pursue it further.
...polyglot/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
🎉 Awesome
Looks good, I'm glad our error messages will be less painful now.
We have 2 workarounds in the test suite that should now become obsolete, both of the form:
matcher_text = case matcher.to_text of
text : Text -> text
_ -> Meta.meta matcher . to_text
thanks to your fix, we should be able to replace them with just matcher_text = matcher.to_text (earlier we had this Meta.meta to go around this static dispatch issue, it was a brute-force band-aid fix).
Could you please remove these workarounds along this PR?
Pull Request Description
This change modifies method dispatch for methods that override Any's definitions. When an overrided method is invoked statically we call Any's method to stay consistent.
This change primarily addresses the plethora of problems related to
to_textinvocations. It does not attempt to completely modify method dispatch logic.Closes #6300.
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.