Merge pull request scala#3246 from xeno-by/topic/compile-time-only

streamlines refchecking undesired symbol properties

src/compiler/scala/tools/nsc/typechecker/RefChecks.scala

@@ -1265,22 +1265,22 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
         false
     }
 
-    /** If symbol is deprecated, and the point of reference is not enclosed
-     *  in either a deprecated member or a scala bridge method, issue a warning.
-     */
-    private def checkDeprecated(sym: Symbol, pos: Position) {
+    // Note: if a symbol has both @deprecated and @migration annotations and both
+    // warnings are enabled, only the first one checked here will be emitted.
+    // I assume that's a consequence of some code trying to avoid noise by suppressing
+    // warnings after the first, but I think it'd be better if we didn't have to
+    // arbitrarily choose one as more important than the other.
+    private def checkUndesiredProperties(sym: Symbol, pos: Position) {
+      // If symbol is deprecated, and the point of reference is not enclosed
+      // in either a deprecated member or a scala bridge method, issue a warning.
       if (sym.isDeprecated && !currentOwner.ownerChain.exists(x => x.isDeprecated || x.hasBridgeAnnotation)) {
         unit.deprecationWarning(pos, "%s%s is deprecated%s".format(
           sym, sym.locationString, sym.deprecationMessage map (": " + _) getOrElse "")
         )
       }
-    }
-
-    /** Similar to deprecation: check if the symbol is marked with @migration
-     *  indicating it has changed semantics between versions.
-     */
-    private def checkMigration(sym: Symbol, pos: Position) = {
-      if (sym.hasMigrationAnnotation) {
+      // Similar to deprecation: check if the symbol is marked with @migration
+      // indicating it has changed semantics between versions.
+      if (sym.hasMigrationAnnotation && settings.Xmigration.value != NoScalaVersion) {
         val changed = try
           settings.Xmigration.value < ScalaVersion(sym.migrationVersion.get)
         catch {
@@ -1292,9 +1292,7 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
         if (changed)
           unit.warning(pos, s"${sym.fullLocationString} has changed semantics in version ${sym.migrationVersion.get}:\n${sym.migrationMessage.get}")
       }
-    }
-
-    private def checkCompileTimeOnly(sym: Symbol, pos: Position) = {
+      // See an explanation of compileTimeOnly in its scaladoc at scala.annotation.compileTimeOnly.
       if (sym.isCompileTimeOnly) {
         def defaultMsg =
           sm"""Reference to ${sym.fullLocationString} should not have survived past type checking,
@@ -1416,8 +1414,8 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
       case TypeRef(pre, sym, args) =>
         tree match {
           case tt: TypeTree if tt.original == null => // SI-7783 don't warn about inferred types
-          case _ =>
-            checkDeprecated(sym, tree.pos)
+                                                      // FIXME: reconcile this check with one in resetAllAttrs
+          case _ => checkUndesiredProperties(sym, tree.pos)
         }
         if(sym.isJavaDefined)
           sym.typeParams foreach (_.cookJavaRawInfo())
@@ -1449,7 +1447,6 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
 
     private def applyRefchecksToAnnotations(tree: Tree): Unit = {
       def applyChecks(annots: List[AnnotationInfo]) = {
-        annots foreach (annot => checkCompileTimeOnly(annot.atp.typeSymbol, annot.pos))
         checkAnnotations(annots map (_.atp), tree)
         transformTrees(annots flatMap (_.args))
       }
@@ -1541,16 +1538,7 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
       val Select(qual, _) = tree
       val sym = tree.symbol
 
-      /* Note: if a symbol has both @deprecated and @migration annotations and both
-       * warnings are enabled, only the first one checked here will be emitted.
-       * I assume that's a consequence of some code trying to avoid noise by suppressing
-       * warnings after the first, but I think it'd be better if we didn't have to
-       * arbitrarily choose one as more important than the other.
-       */
-      checkDeprecated(sym, tree.pos)
-      if(settings.Xmigration.value != NoScalaVersion)
-        checkMigration(sym, tree.pos)
-      checkCompileTimeOnly(sym, tree.pos)
+      checkUndesiredProperties(sym, tree.pos)
       checkDelayedInitSelect(qual, sym, tree.pos)
 
       if (!sym.exists)
@@ -1705,7 +1693,7 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
             tree
 
           case Ident(name) =>
-            checkCompileTimeOnly(tree.symbol, tree.pos)
+            checkUndesiredProperties(sym, tree.pos)
             transformCaseApply(tree,
               if (name != nme.WILDCARD && name != tpnme.WILDCARD_STAR) {
                 assert(sym != NoSymbol, "transformCaseApply: name = " + name.debugString + " tree = " + tree + " / " + tree.getClass) //debug

test/files/neg/compile-time-only-a.check

@@ -1,49 +1,79 @@
-compile-time-only-a.scala:9: error: C3
+compile-time-only-a.scala:10: error: C3
 @compileTimeOnly("C3") case class C3(x: Int)
                                   ^
-compile-time-only-a.scala:11: error: C4
+compile-time-only-a.scala:12: error: C4
 @compileTimeOnly("C4") case class C4(x: Int)
                                   ^
-compile-time-only-a.scala:16: error: C5
+compile-time-only-a.scala:17: error: C5
   implicit class C5(val x: Int) {
                  ^
-compile-time-only-a.scala:28: error: C1
+compile-time-only-a.scala:32: error: C1
   new C1()
       ^
-compile-time-only-a.scala:32: error: C2
+compile-time-only-a.scala:36: error: C2
   C2
   ^
-compile-time-only-a.scala:34: error: C3
+compile-time-only-a.scala:38: error: C3
   new C3(2)
       ^
-compile-time-only-a.scala:37: error: C4
+compile-time-only-a.scala:41: error: C4
   new C4(2)
       ^
-compile-time-only-a.scala:41: error: C5
+compile-time-only-a.scala:45: error: C5
   2.ext
   ^
-compile-time-only-a.scala:42: error: C5
+compile-time-only-a.scala:46: error: C5
   C5(2)
   ^
-compile-time-only-a.scala:45: error: C6.x
+compile-time-only-a.scala:49: error: C6.x
   val _ = c6.x
              ^
-compile-time-only-a.scala:46: error: C6.foo
+compile-time-only-a.scala:50: error: C6.foo
   c6.foo
      ^
-compile-time-only-a.scala:48: error: C6.y
+compile-time-only-a.scala:51: error: C6.Foo
+  type Foo = c6.Foo
+                ^
+compile-time-only-a.scala:52: error: C6.y
   c6.y = c6.y
      ^
-compile-time-only-a.scala:48: error: C6.y
+compile-time-only-a.scala:52: error: C6.y
   c6.y = c6.y
             ^
-compile-time-only-a.scala:54: error: placebo
-@placebo
- ^
-compile-time-only-a.scala:56: error: placebo
+compile-time-only-a.scala:54: error: C7
+  val c701: (C7, C7) = ???
+            ^
+compile-time-only-a.scala:55: error: C7
+  val c702: (C7 => C7) = ???
+                ^
+compile-time-only-a.scala:56: error: C7
+  val c703: { val x: C7 } = ???
+            ^
+compile-time-only-a.scala:57: error: C7
+  val c704: AnyRef with C7 = ???
+            ^
+compile-time-only-a.scala:60: error: C7
+  val c706: C7 Either C7 = ???
+               ^
+compile-time-only-a.scala:61: error: C7
+  val c707a: List[C7] = ???
+             ^
+compile-time-only-a.scala:63: error: C7
+  val c708a: T forSome { type T <: C7 } = ???
+               ^
+compile-time-only-a.scala:66: error: C8
+  val c709: (C8[Int], C8[C7]) = ???
+            ^
+compile-time-only-a.scala:67: error: C8
+  val c710: (C8[_] => C8[_]) = ???
+                   ^
+compile-time-only-a.scala:74: error: placebo
+class Test {
+      ^
+compile-time-only-a.scala:75: error: placebo
   @placebo def x = (2: @placebo)
-   ^
-compile-time-only-a.scala:56: error: placebo
+               ^
+compile-time-only-a.scala:75: error: placebo
   @placebo def x = (2: @placebo)
                         ^
-16 errors found
+26 errors found

test/files/neg/compile-time-only-a.scala

@@ -1,4 +1,5 @@
 import scala.annotation.compileTimeOnly
+import scala.language.existentials
 
 @compileTimeOnly("C1") class C1
 object C1
@@ -24,6 +25,9 @@ class C6(@compileTimeOnly("C6.x") val x: Int) {
   @compileTimeOnly("C6.y") var y = 3
 }
 
+@compileTimeOnly("C7") class C7
+@compileTimeOnly("C8") class C8[T]
+
 object Test extends App {
   new C1()
   C1
@@ -46,6 +50,21 @@ object Test extends App {
   c6.foo
   type Foo = c6.Foo
   c6.y = c6.y
+
+  val c701: (C7, C7) = ???
+  val c702: (C7 => C7) = ???
+  val c703: { val x: C7 } = ???
+  val c704: AnyRef with C7 = ???
+  // https://groups.google.com/forum/#!topic/scala-internals/5n07TiCnBZU
+  // val c705: ({ @compileTimeOnly("C7") type C7[T] = List[T] })#C7[_] = ???
+  val c706: C7 Either C7 = ???
+  val c707a: List[C7] = ???
+  val c707b = List[C7]()
+  val c708a: T forSome { type T <: C7 } = ???
+  // https://groups.google.com/forum/#!topic/scala-internals/5n07TiCnBZU
+  // val c708b: T forSome { @compileTimeOnly("C7") type T } = ???
+  val c709: (C8[Int], C8[C7]) = ???
+  val c710: (C8[_] => C8[_]) = ???
 }
 
 @compileTimeOnly("placebo")