Skip to content

Inconsistency between reflect Typed and #12221

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

Closed
nicolasstucki opened this issue Apr 26, 2021 · 4 comments · Fixed by #12236
Closed

Inconsistency between reflect Typed and #12221

nicolasstucki opened this issue Apr 26, 2021 · 4 comments · Fixed by #12236
Assignees
Milestone

Comments

@nicolasstucki
Copy link
Contributor

Compiler version

3.0.0-RC3

Issue

Both TypeTests will may match the same tpd.Typed tree

This makes the type hierarchy unsound.

UnapplyTypeTest.unapply only matches tpd.Typed for historical reasons. Back when trees were separated from patterns we needed to handle this specially.

Solution

UnapplyTypeTest.unapply only match tpd.Unapply

@Snowflake8
Copy link

Snowflake8 commented Apr 26, 2021

I think this is what I encountered (quoted from gitter) (or this since I noticed a problem with the Unapply and a mix with #12222 ). I can provide the code if needed

Hello I've been been working in metaprogramming with scala.reflect and have stumbled across what could be a bug. Basically I have this fuction

def treePrint(tree: Tree): Unit = {
            tree match {
                case body : Term => {
                        body match 
                           case typed: Typed =>
                              // Point A
                              // Code for Typed
                        //other Terms
                       }
                // case Unapply(_, _, _) => 
                         // Point C
                case typed : Typed =>
                        // Point B
                        // This should never run
                // other Tree things
            }
        }

When I run the function point B is sometimes reached even though Typed should always go to Point A (since Typed is a subtype of Term). Even stranger, if I uncomment the Unapply case, instead of point B getting reached, it is point C instead.
I can provide a failing input if necessary

@nicolasstucki nicolasstucki linked a pull request Apr 26, 2021 that will close this issue
@nicolasstucki
Copy link
Contributor Author

@Snowflake8 does sound like it would be caused by this bug. Could you create a minimal self-contained code example that fails with your use case. This way we can ensure that everything is properly fixed.

nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Apr 26, 2021
Remove old workaround for `Unapply`.

Users may see an extra `Typed` in the trees of patterns

Fixes scala#12221
@nicolasstucki nicolasstucki linked a pull request Apr 26, 2021 that will close this issue
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Apr 27, 2021
Remove old workaround for `Unapply`.

Users may see an extra `Typed` in the trees of patterns

Fixes scala#12221
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Apr 27, 2021
Remove old workaround for `Unapply`.

Users may see an extra `Typed` in the trees of patterns

Fixes scala#12221
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Apr 27, 2021
Remove old workaround for `Unapply`.

Users may see an extra `Typed` in the trees of patterns

Fixes scala#12221
@nicolasstucki nicolasstucki reopened this Apr 27, 2021
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Apr 28, 2021
Remove old workaround for `Unapply`.

Users may see an extra `Typed` in the trees of patterns

Fixes scala#12221
@Snowflake8
Copy link

@nicolasstucki You were too quick
Here was the code for legacy purpose
https://github.com/Snowflake8/Scala_Typed_bug

@nicolasstucki
Copy link
Contributor Author

nicolasstucki commented Apr 29, 2021

@Snowflake8 no problem, I will add that extra test or reference it in one of the related issues if it hits another bug.

nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Apr 29, 2021
liufengyun added a commit that referenced this issue Apr 29, 2021
michelou pushed a commit to michelou/scala3 that referenced this issue Apr 29, 2021
@Kordyjan Kordyjan added this to the 3.0.1 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants