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

Path-dependent type on singleton value emits spurious outer test warnings #12398

Closed
neko-kai opened this issue May 17, 2021 · 6 comments · Fixed by scala/scala#9653
Closed

Path-dependent type on singleton value emits spurious outer test warnings #12398

neko-kai opened this issue May 17, 2021 · 6 comments · Fixed by scala/scala#9653

Comments

@neko-kai
Copy link

neko-kai commented May 17, 2021

reproduction steps

using Scala 2.13.6,

object AnnotationTools {
  def getAllTypeAnnotations(u: Universe)(typ: u.Type): List[u.Annotation] = {
    typ.finalResultType.dealias match {
      case t: u.AnnotatedTypeApi =>
        t.annotations
      case _ =>
        Nil
    }
  }
}

problem

Results in a warning:

/izumi/fundamentals/fundamentals-reflection/src/main/scala/izumi/fundamentals/reflection/AnnotationTools.scala:16:12
The outer reference in this type test cannot be checked at run time.
      case t: u.AnnotatedTypeApi =>

When decompiled, the warning is true, the outer reference is in fact not checked:

   public List getAllTypeAnnotations(final Universe u, final TypeApi typ) {
      TypeApi var6 = .MODULE$.stripByName(u, typ.finalResultType().dealias());
      Object var3;
      if (var6 instanceof AnnotatedTypeApi && true) {
         var3 = ((AnnotatedTypeApi)var6).annotations();
      } else {
         var3 = scala.package..MODULE$.Nil();
      }

      return (List)var3;
   }

However, since u. is a literal prefix of AnnotatedTypeApi, is it not supposed to be checkable at runtime?

clarification

I've submitted this report assuming the new warning in 2.13.6 was incorrect and the type prefix was checked. On decompiling I found the warning is correct, and the behavior of all previous Scala versions was incorrect – they did not check the prefix AND did not issue the warning. So I rewrote the report to complain about lack of runtime type prefix comparison; 🤦
I am not sure if the behavior is incorrect, although from a language perspective it would seem that the implementation should always be able to compare prefixes when they are present syntactically in the type.

@neko-kai neko-kai changed the title Spurious "The outer reference in this type test cannot be checked at run time." Type prefix outer reference is not checked in type tests May 17, 2021
@dwijnand
Copy link
Member

dwijnand commented May 17, 2021

This is #4440. I spent more time looking at the variant where one is matching a nested final case class (rather than a trait) and what I understood is that it had been decided that adding a field for the outer reference was too much of an overhead to each case class, so it was optimistically elided. My understanding is the field is captured in Scala 3 so it's no longer incorrect. But in 2.13 we're stuck with what we've got. 😞

From my work on scala/scala#9622 if you can introduce the use of the Singleton type it would drop the warning.

@dwijnand dwijnand added duplicate fixed in Scala 3 This issue does not exist in the Scala 3 compiler (https://github.com/lampepfl/dotty/) labels May 17, 2021
@neko-kai
Copy link
Author

There doesn't seem to be a way to interact with scala-reflect without warnings now, since using an Extractor object also generates warnings 🤔 :

  def getAllTypeAnnotations(u: Universe)(typ: u.Type): List[u.Annotation] = {
    typ.finalResultType.dealias match {
      case t @ u.AnnotatedType(_, _) =>
        t.annotations
      case _ =>
        Nil
    }
/izumi/fundamentals/fundamentals-reflection/src/main/scala/izumi/fundamentals/reflection/AnnotationTools.scala:17:31
abstract type pattern u.AnnotatedType is unchecked since it is eliminated by erasure
      case t @ u.AnnotatedType(_, _) =>

@dwijnand
Copy link
Member

Yeah, it's very apparent in scala-reflect, with all its nested classes. Can you change to u: Universe with Singleton?

@neko-kai
Copy link
Author

neko-kai commented May 17, 2021

@dwijnand

if you can introduce the use of the Singleton type it would drop the warning.

This appears to be the case, but it also becomes very odd now, since a type projection is now warningless:

  def getAllTypeAnnotationsUSharp[U <: SingletonUniverse](u: U)(typ: U#Type): List[U#Annotation] = {
    typ.finalResultType.dealias match {
      case t: U#AnnotatedTypeApi =>
        t.annotations
      case _ =>
        Nil
    }
  }

But there doesn't seem to be a way to make the value-prefix match warningless:

  def getAllTypeAnnotations[U <: Universe with Singleton](u: U)(typ: U#Type): List[U#Annotation] = {
    typ.finalResultType.dealias match {
      case t: u.AnnotatedTypeApi =>
        t.annotations
      case _ =>
        Nil
    }
  }
/izumi/fundamentals/fundamentals-reflection/src/main/scala/izumi/fundamentals/reflection/AnnotationTools.scala:24:12
The outer reference in this type test cannot be checked at run time.
      case t: u.AnnotatedTypeApi =>

@dwijnand

Can you change to u: Universe with Singleton?

Yeah, in the snippet above, this does not seem to help the value case (even written like (u.type)#AnnotatedTypeApi)

@dwijnand
Copy link
Member

I'll try to look at how u.AnnotatedTypeApi is behaving in prefixAligns.

@dwijnand dwijnand self-assigned this May 17, 2021
@neko-kai
Copy link
Author

The following workaround does get rid of the warning when only a value prefix is around:

def getAllTypeAnnotations(u: Universe)(typ: u.Type): List[u.Annotation] = {
  object xa { type U <: u.type with Singleton }
  typ.finalResultType.dealias match {
    case t: xa.U#AnnotatedTypeApi =>
      t.annotations
    case _ =>
      Nil
  }
}

neko-kai added a commit to 7mind/izumi that referenced this issue May 17, 2021
neko-kai added a commit to 7mind/izumi that referenced this issue May 28, 2021
neko-kai added a commit to 7mind/izumi that referenced this issue May 29, 2021
neko-kai added a commit to 7mind/izumi that referenced this issue May 29, 2021
…calac 2.13.6 (#1479)

* Use custom scala version

* Use `-Xsource:3`

* Use kind-projector `underscore-placeholders`

* Swap ? and _ with new kind-projector & `-Xsource:3`

* Workaround `scala.reflect.internal.FatalError:[error]   NoSymbol.clone()` error that happens on `Universe with Singleton` matches on 2.13.6-SNAPSHOT

* regenerate build

* `-Xsource:3` fixes

* Add 2.12 support

* Update to Scala 2.13.6

* Return older implementations without FatalError

* Add `nowarn`s for scala/bug#12398

* Update to Scala 2.12.14

* fix build

* fix build

* regenerate build

* Fix regression introduced in `Return older implementations without FatalError`

* Fix unintended `?`->`_` replacements

* remove remaining `*` placeholders

* Disable `-Xsource:3` in docs due to mdoc failures:

```
error: basics.md:97 (mdoc generated code) could not find implicit value for parameter t: pprint.TPrint[zio.ZIO[zio.Has[zio.console.Console.Service],Throwable,β$0$]]
val injector: Injector[RIO[Console, _]] = Injector[RIO[Console, _]](); $doc.binder(injector, 2, 4, 2, 12)
                                                                                  ^

error: basics.md:109 (mdoc generated code) could not find implicit value for parameter t: pprint.TPrint[zio.ZIO[zio.Has[zio.console.Console.Service],Throwable,β$0$]]
val resource = injector.produce(plan); $doc.binder(resource, 4, 4, 4, 12)
                                                  ^

error: basics.md:1359 (mdoc generated code) could not find implicit value for parameter t: pprint.TPrint[zio.ZIO[zio.Has[zio.console.Console.Service],Throwable,β$9$]]
val res51 = chooseInterpreters(true); $doc.binder(res51, 26, 0, 26, 24)
```

* Enable -Xsource:3 in unidoc after disabling in mdoc
@dwijnand dwijnand removed duplicate fixed in Scala 3 This issue does not exist in the Scala 3 compiler (https://github.com/lampepfl/dotty/) labels Jun 3, 2021
@dwijnand dwijnand changed the title Type prefix outer reference is not checked in type tests Path-dependent type on singleton value emits spurious outer test warnings Jun 3, 2021
@dwijnand dwijnand added this to the 2.13.7 milestone Jun 3, 2021
@dwijnand dwijnand added the has PR label Jun 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants