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

Conflicting Extension Methods Make Resolution Nondeterministic #1243

Closed
radeusgd opened this issue Oct 23, 2020 · 12 comments · Fixed by #9844
Closed

Conflicting Extension Methods Make Resolution Nondeterministic #1243

radeusgd opened this issue Oct 23, 2020 · 12 comments · Fixed by #9844
Assignees
Labels
--bug Type: bug -compiler p-medium Should be completed in the next few sprints

Comments

@radeusgd
Copy link
Member

radeusgd commented Oct 23, 2020

See comment below for updated repro.

General Summary

When multiple modules define extension methods with the same name and for the same type and both modules are imported, resolution of that method is not deterministic.

Steps to Reproduce

  1. Create new project (here called Multitest) with the following files:
  • File1.enso:
from Base import all
Number.foo = 42
  • File2.enso:
from Base import all
Number.foo = "answer"
  • Main.enso:
from Base import all
import Multitest.File1 
import Multitest.File2

main =
  a = 23.foo
  IO.println a

Or download the project from here: multitest.zip

  1. Run it several times

Expected Result

The above code should result in an ambiguity error.

Ideally, the error should be detected when importing the module that brings the ambiguous resolution. If that's not possible, executing that method in runtime should yield a runtime error indicating the ambiguity.

Actual Result

The program sometimes prints answer and sometimes 42, behaving nondeterministically.

Enso Version

Enso Launcher
Version:    0.1.0
Built with: scala-2.13.3 for GraalVM 20.2.0
Built from: wip/rw/launcher-locking* @ 0bd1686264e50e55a07b73df27b9efb0cac0e37f
Built on:   Linux (amd64)
Current default Enso engine: 
Enso Compiler and Runtime
Version:    0.1.0
Built with: scala-2.13.3 for GraalVM 20.2.0
Built from: wip/rw/separate-component-manager* @ 746521f8b20de95f4300fb950207c12c8ee687c0
Running on: OpenJDK 64-Bit Server VM, GraalVM Community, JDK 11.0.8+10-jvmci-20.2-b03
            Linux 4.15.0-122-generic (amd64)
@radeusgd radeusgd added Category: Interpreter p-medium Should be completed in the next few sprints labels Oct 23, 2020
@iamrecursion
Copy link
Contributor

Check for ambiguity in ModuleScope and then raise a runtime error in UnresolvedSymbol.

@iamrecursion iamrecursion added this to the Alpha 4 milestone Mar 25, 2021
@iamrecursion iamrecursion assigned iamrecursion and unassigned kustosz Mar 25, 2021
@iamrecursion iamrecursion modified the milestones: Alpha 4, Alpha 5 Apr 26, 2021
@iamrecursion iamrecursion removed this from the Alpha 5 milestone May 11, 2021
@wdanilo wdanilo closed this as completed Apr 14, 2022
@radeusgd
Copy link
Member Author

This issue still persist to this day.

Updated repro: create a project with following 4 files:

Mod.enso:

type My_Typ
    Value x

F1.enso:

import project.Mod.My_Typ

My_Typ.foo self = self.x+3

F2.enso:

import project.Mod.My_Typ

My_Typ.foo self = self.x+4

Main.enso:

from Standard.Base import all

import project.Mod.My_Typ

import project.F2
import project.F1

main =
    o = My_Typ.Value 1000
    IO.println (o.foo)

(or download project456.zip)

and run it.

It sometimes prints 1004 and sometimes 1003. If the nondeterministic behaviour does not show up immediately, you may try reordering the F1 and F2 imports and running a few more times.

@radeusgd
Copy link
Member Author

Related problem - it is possible to define an extension method on a type that already has a method with that name. In such case, the extension method simply seems to be ignored during resolution.

I think we should indicate an error when trying to define an extension method on a type which already has such a method - as this extension method will never be able to be called.

Repro:
Mod.enso:

from Standard.Base import all

type My_Typ
    Value x
    foo self = self.x+1

Main.enso:

from Standard.Base import all

import project.Mod.My_Typ

My_Typ.foo self = self.x+2

main =
    o = My_Typ.Value 1000
    IO.println (o.foo)

Running it seems to always yield 1001, so the extension method defined in Main.enso is always ignored.

I think this could be solved as part of this issue as both problems are essentially about ambiguity of extension methods (there's just 2 cases: ambiguity between 2 extension methods from different modules AND ambiguity between an extension method and a 'normal' one).

@radeusgd
Copy link
Member Author

Interestingly, if 2 extension methods are defined in a single module, we do get an error:

from Standard.Base import all

import project.Mod.My_Typ

My_Typ.foo self = self.x+2
My_Typ.foo self = self.x+22

main =
    o = My_Typ.Value 1000
    IO.println (o.foo)
C:\NBO\repr\project123\src\Main.enso:6:1: error: Method overloads are not supported: My_Typ.foo is defined multiple times in this module.
    6 | My_Typ.foo self = self.x+22
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
Aborting due to 1 errors and 0 warnings.
Execution finished with an error: Compilation aborted due to errors.

@enso-bot
Copy link

enso-bot bot commented May 2, 2024

Pavel Marek reports a new STANDUP for today (2024-05-02):

Progress: - Ambiguous resolution seems very weak - we can even redefine an imported type without an error.

  • Writing some tests
  • Figuring out the best location where this check should be done.
    • Optimally during compilation.
    • Seems like IrToTruffle is the best place.
  • This issue is not only about extension method names conflicting, but about all the imported shadowed entities. It should be finished by 2024-05-07.

@enso-bot
Copy link

enso-bot bot commented May 3, 2024

Pavel Marek reports a new STANDUP for today (2024-05-03):

Progress: - Small bug fix - "from conversion propagates dataflow error" in #9856.

  • Still trying to reproduce the UnresolvedMethod(==) transient error in [WIP] Add logging for UnresolvedMethod bug #9763 with no success.
  • Ensured that the RecursionBenchmarks now succeeds.
  • At the end, figured out a way how to check for extension method ambiguity errors during static compilation. It should be finished by 2024-05-07.

@enso-bot
Copy link

enso-bot bot commented May 6, 2024

Pavel Marek reports a new STANDUP for today (2024-05-06):

Progress: - Helping with module path on ydoc server

  • All helidon modules are on module-path.
  • Implement ydoc-server/run convenient command.
  • The UnresolvedSymbol(==) cannot be reproduced anymore.
    • Probably Radek's change of reordering the tests "fixed" it.
  • Trying to fix failing test of conflicting extension methods.
    • It seems that I will have to add another isDefinedInType boolean flag to Method$Explicit IR? It should be finished by 2024-05-07.

@enso-bot
Copy link

enso-bot bot commented May 7, 2024

Pavel Marek reports a new STANDUP for today (2024-05-07):

Progress: - More debugging of module-path issues in ydoc server

  • Finally found the issue - we had to export some package from module-info.
  • Fixing the OverloadsResolution pass to only iterate over extension methods.
    • We need to differentiate between a static method and an extension method.
    • Only extension methods can result in an ambiguous overload.
    • static method implies that it is defined inside a body of a type, and types can be shadowed.
  • Reviewing other PRs. It should be finished by 2024-05-07.

@enso-bot
Copy link

enso-bot bot commented May 20, 2024

Pavel Marek reports a new 🔴 DELAY for the provided date (2024-05-17):

Summary: There is 15 days delay in implementation of the Conflicting Extension Methods Make Resolution Nondeterministic (#1243) task.
It will cause 15 days delay for the delivery of this weekly plan.

Delay Cause: Getting back to the issue after a long vacation.

@enso-bot
Copy link

enso-bot bot commented May 20, 2024

Pavel Marek reports a new STANDUP for the provided date (2024-05-17):

Progress: - Giving up on the static compilation way - let's fix that in IrToTruffle. It should be finished by 2024-05-22.

@enso-bot
Copy link

enso-bot bot commented May 20, 2024

Pavel Marek reports a new STANDUP for today (2024-05-20):

Progress: - Bumped into a problem with module ordering from import/export resolution.

  • It seems that imported modules might not be compiled.
  • "Fixing" the import/export module order is difficult, let's try to fix that by introducing a separate check that will be run after Truffle codegen phase (at the end of the compilation pipeline). It should be finished by 2024-05-22.

@enso-bot
Copy link

enso-bot bot commented May 21, 2024

Pavel Marek reports a new STANDUP for today (2024-05-21):

Progress: - The runtime check in UnresolvedSymbol is easy to implement, but may introduce some overhead.

  • Writing some tests for method collection in ModuleScope. The order of these methods must be deterministic. It should be finished by 2024-05-22.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
--bug Type: bug -compiler p-medium Should be completed in the next few sprints
Projects
Status: 🟢 Accepted
Development

Successfully merging a pull request may close this issue.

7 participants