More accurate outer checks in patterns

[retronym]

Avoids eliding outer checks that matter (run/t11534b.scala) and
avoids emitting checks that don't (pos/t11534.scala) which avoids
compiler warnings when the tested class doesn't have an outer
field.

The latter stops the annoying unchecked warning that appeared since
a recent refactoring made TermName a final class.

Fixes scala/bug#11534

[retronym]

Pushed another version to try to walk the tightrope between pos/t11534 and run/reflect-priv-ctor.scala. Marking as WIP as this isn't super well thought through...

[eed3si9n]

@retronym Would this make 2.12.9?

[retronym]

@eed3si9n No, I'd rather not rush this one.

[sjrd]

Could I remind people about this PR? I still have to compile the Scala.js compiler plugin without -Xfatal-warnings in the latest 2.12.x nightly.

[adriaanm]

This PR needs more work before we can merge it. I can't pick it up right now.

[dwijnand]

This PR needs more work before we can merge it. I can't pick it up right now.

@adriaanm / @retronym what's remaining and how terrible would it be if this landed as is? It's looks nice and small, to my amateur eyes.

[dwijnand]

@retronym could you look at this one, please? Even just to get more going on improving it. (I'd like to get rid of the "The outer reference in this type test cannot be checked at run time" exclusion we have in our build.sbt.)

[joroKr21]

I'm not sure what is the reasoning here 🤔

[joroKr21]

Ideally the compiler would be smart enough to avoid the runtime test in this case as we statically know that the prefixes are the same.

I can't convince myself we know that statically.

[joroKr21]

Neither B nor C is a subclass of the other.

In that case the conclusion could be no - we can't elide the check.

[joroKr21]

NVM I see that you already use baseType - I should have checked. However I can't make any sense of prefixAlign2.

[retronym]

However I can't make any sense of prefixAlign2

Sorry, more docs and a good set of tests are still needed here! The idea is as follows.

Given:

(x: a.B) match { case _: a.C }

Where C is subclass of A

Is it possible that a value conforms to a.B and a1.C where a ne a1?

To answer this, I create a new prefix based on a fresh symbol and check the base type of TypeRef(freshPrefix, typePatternSymbol, args) at the binder symbol. If that is prefixed by the fresh symbol, they are statically the same.

[dwijnand]

@dwijnand Formulating the test this way might make my point a bit clearer:

Thanks. I tried to disprove the assertion with

  trait BI extends o2.B
  class D2 extends o1.B with o1.C with BI

  // expected O(1) as outer of value conforming to pattern `b: o1.B`, but got O(2)
  def main(args: Array[String]): Unit = {
    pat1(new o1.D)
    pat1(new D2)
  }

but it won't let you define such a D2:

outer-checks.scala:16: error: illegal inheritance;
 class D2 inherits different type instances of class B:
Test.o2.B and Test.o1.B
  class D2 extends o1.B with o1.C with BI
        ^

which I think I might've seen recently pop up as not being caught in Scala 3, but I might be confusing with something similar looking.

I'll try to review the patch.

[dwijnand]

LGTM, for its results rather than large sections of how it's done. I understand the intent though. Nice tests and nice fix. I left a couple of questions.

[retronym]

@adriaanm Perhaps you'd also like to take a look at where this ended up.

[dwijnand]

Likely going to avoid the risk in 2.12 and land #9504 instead, for 2.13.