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

surface: Simplify extractSymbol - avoid .tree to make library working with Scala 3.4.2 #3533

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

OndrejSpanel
Copy link
Contributor

Followup to #3521

The method extractSymbol used to dealias inner types for both opaque types and type aliases does not work when the library is used from Scala 3.4.2 application. The reason is infamous .tree method again, which is known and documented to be unreliable.

I am not sure why it was used, the code seems simpler without it, but maybe I am missing something. After the simplification all tests still pass.

@OndrejSpanel OndrejSpanel marked this pull request as draft May 14, 2024 07:47
@OndrejSpanel
Copy link
Contributor Author

OndrejSpanel commented May 14, 2024

After the simplification all tests still pass.

The airframe tests passed locally, but some tests for other modules fail on CI. I will check what is going on.

@OndrejSpanel
Copy link
Contributor Author

The issue is with:

diJVM/testOnly *.PathDependentTypeTest

Compiler crashes with:

exception while retyping JdbcBackend.this of class This # -1

[error] java.lang.AssertionError: assertion failed: missing outer accessor in class PathDependentTypeTest
[error] scala.runtime.Scala3RunTime$.assertFailed(Scala3RunTime.scala:8)
[error] dotty.tools.dotc.transform.ExplicitOuter$.dotty$tools$dotc$transform$ExplicitOuter$$$outerParamAccessor(ExplicitOuter.scala:237)

My fix causes bad representation of type Database = DatabaseDef, which is now represented as Database:=Database instead of Database:=DatabaseDef.

@OndrejSpanel
Copy link
Contributor Author

I am afraid I am unable to fix this. I was missing one dealias, once I have added it, types both with and without the fix seem correct, without the fix the type is represented as _ >: MyBackend.this.DatabaseDef <: MyBackend.this.DatabaseDef, which somehow prevents something which happens latter with MyBackend.this.DatabaseDef, the type used with the fix.

It works fine in my "light" fork without the method accessors.

Unless this is fixed, expect trouble once users of the library port to 3.4.2.

No fix logs:

dealiased wvlet.airframe.surface.PathDependentType.MyProfile#Backend.Database of wvlet.airframe.surface.PathDependentType.MyProfile#Backend.Database
  maybeOwner.declarations <init>,Database,DatabaseDef,DatabaseDef,DatabaseDef$
Match 1 _ >: MyBackend.this.DatabaseDef <: MyBackend.this.DatabaseDef in Some(type Database)

With the fix:

dealiased wvlet.airframe.surface.PathDependentType.MyProfile#Backend.Database of wvlet.airframe.surface.PathDependentType.MyProfile#Backend.Database
  maybeOwner.declarations <init>,Database,DatabaseDef,DatabaseDef,DatabaseDef$
Match 1 type Database in Some(type Database) as MyBackend.this.Database -> MyBackend.this.DatabaseDef

Note MyBackend.this.DatabaseDef, which reminds of #3411. The error itself reminds of #3440

@xerial xerial added the bug label May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants