Make extension method resolution deterministic

[Akirathan]

Fixes #1243

Pull Request Description

Fixes the non-deterministic method resolution in some cases. Normally, method resolution tries to lookup the method in:

1. In the scope where the type is defined
2. In the current module scope
3. In imported module scopes

The third step used to be non-deterministic. I have made it deterministic simply by replacing java.util.HashMap by java.util.LinkedHashMap in ModuleScope.

Important Notes

I have attempted three times to implement this feature:

• In static compilation in a separate compiler pass
• Right after static compilation in IrToTruffle, just before execution.
• At runtime, just before method is executed in UnresolvedSymbol, as an error.
  All these attempts failed. More details in comment below.

At the end, I have decided to fix this bug by making the method resolution deterministic and not throwing any errors.

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.

[JaroslavTulach]

Looks like you managed to detect this error during static compilation. Good!

[Akirathan]

All attempts to check extension method conflicts during compilation failed:

• OverloadResolution compiler pass does not have information from org.enso.interpreter.runtime.scope.ModuleScope, and without it, it is very difficult to query defined methods in imported modules.
• Checking this in IrToTruffle failed as well, because the imported modules there might not be compiled, i.e, their stage might not be CompilationStage.AFTER_CODEGEN. Initially, I suspected this is a bug in import/export resolution, that it orders the modules in non-topological order, so I have defined tests in https://github.com/enso-org/enso/blob/097f016741420cf921345f9cb143c6ae0e57f50e/engine/runtime-integration-tests/src/test/java/org/enso/compiler/ImportExportResolutionOrderTest.java, but it turns out that this is a "feature" not a bug. And changing the behavior of import export resolution is very tricky.

At the end, I have resolved to checks at runtime, inside UnresolvedSymbol.

GitHub

enso/engine/runtime-integration-tests/src/test/java/org/enso/compiler/ImportExportResolutionOrderTest.java at 097f016741420cf921345f9cb143c6ae0e57f50e · enso-org/enso

Hybrid visual and textual functional programming. Contribute to enso-org/enso development by creating an account on GitHub.

[Akirathan]

cc @hubertp. Note that I am getting rid of ConcurrentHashMap here and no tests are failing. This should be a proof that ModuleScope is intended to be single-threaded. Note that it would even not be possible to use it from more threads, as, e.g., in reset method in https://github.com/enso-org/enso/pull/9844/files#diff-c3869665e0ec34703f8e3eea0464924c0e4260eab392dbccdc202ec1f0270379L399, a normal HashMap is reassigned to the methods field instead of ConcurrentHashMap.