SI-8192 adds ClassSymbol.isPrimaryConstructor

Exposes a popular code pattern in macros as a dedicated reflection API.
This simple commit, however, ended up being not so simple, as it often
happens with our compiler.

When writing a test for the new API, I realized that our (pre-existing)
MethodSymbol.isPrimaryConstructor API returns nonsensical results for
implementation artifacts (trait mixin ctors, module class ctors).

What’s even more funny is that according to our reflection internals,
even Java classes have primary constructors. Well, that’s not surprising,
because `primaryConstructor` is just `decl(ctorName).alternatives.head`.
Good thing that package classes don’t have constructors or that would
elevate the situation to three fries short of a happy meal.

At the moment, I’m too scared to fiddle with internal#Symbol.primaryConstructor,
because that could easily break someone right before RC1, so I simply
documented the findings in SI-8193 and postponed the actual work, except
for one thing - isJavaDefined symbols no longer have primary constructors.

8 files changed, 148 insertions, 8 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/Erasure.scala b/src/compiler/scala/tools/nsc/transform/Erasure.scala
index 60c1553ef3..eba2e1399d 100644
--- a/src/compiler/scala/tools/nsc/transform/Erasure.scala
+++ b/src/compiler/scala/tools/nsc/transform/Erasure.scala
@@ -597,7 +597,7 @@ abstract class Erasure extends AddInterfaces
           if (tree.symbol == NoSymbol) {
             tree
           } else if (name == nme.CONSTRUCTOR) {
-            if (tree.symbol.owner == AnyValClass) tree.symbol = ObjectClass.primaryConstructor
+            if (tree.symbol.owner == AnyValClass) tree.symbol = ObjectClass.info.decl(nme.CONSTRUCTOR)
             tree
           } else if (tree.symbol == Any_asInstanceOf)
             adaptMember(atPos(tree.pos)(Select(qual, Object_asInstanceOf)))
@@ -737,7 +737,7 @@ abstract class Erasure extends AddInterfaces
     /** TODO - adapt SymbolPairs so it can be used here. */
     private def checkNoDeclaredDoubleDefs(base: Symbol) {
       val decls = base.info.decls
-      
+
       // SI-8010 force infos, otherwise makeNotPrivate in ExplicitOuter info transformer can trigger
       //         a scope rehash while were iterating and we can see the same entry twice!
       //         Inspection of SymbolPairs (the basis of OverridingPairs), suggests that it is immune
@@ -748,13 +748,13 @@ abstract class Erasure extends AddInterfaces
       //         we do these checks, so that we're comparing same-named methods based on the expanded names that actually
       //         end up in the bytecode.
       exitingPostErasure(decls.foreach(_.info))
-      
+
       var e = decls.elems
       while (e ne null) {
         if (e.sym.isTerm) {
           var e1 = decls lookupNextEntry e
           while (e1 ne null) {
-            assert(e.sym ne e1.sym, s"Internal error: encountered ${e.sym.debugLocationString} twice during scope traversal. This might be related to SI-8010.")            
+            assert(e.sym ne e1.sym, s"Internal error: encountered ${e.sym.debugLocationString} twice during scope traversal. This might be related to SI-8010.")
             if (sameTypeAfterErasure(e.sym, e1.sym))
               doubleDefError(new SymbolPair(base, e.sym, e1.sym))
 
diff --git a/src/reflect/scala/reflect/api/Symbols.scala b/src/reflect/scala/reflect/api/Symbols.scala
index 6fff965de1..8e26679e62 100644
--- a/src/reflect/scala/reflect/api/Symbols.scala
+++ b/src/reflect/scala/reflect/api/Symbols.scala
@@ -952,6 +952,19 @@ trait Symbols { self: Universe =>
      *  @group Class
      */
     def typeParams: List[Symbol]
+
+    /** For a Scala class or module class, the primary constructor of the class.
+     *  For a Scala trait, its mixin constructor.
+     *  For a Scala package class, NoSymbol.
+     *  For a Java class, NoSymbol.
+     *
+     *  @group Class
+     */
+    // TODO: SI-8193 I think we should only return a non-empty symbol if called for Scala classes
+    // returning something for traits and module classes is outright confusing
+    // This, however, will require some refactoring in the compiler, so I'll leave it for later
+    // as at the moment we don't have time or risk tolerance for that
+    def primaryConstructor: Symbol
   }
 
   /** The API of free term symbols.
diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala
index dae9eba878..94b60bafae 100644
--- a/src/reflect/scala/reflect/internal/Symbols.scala
+++ b/src/reflect/scala/reflect/internal/Symbols.scala
@@ -1862,7 +1862,6 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     /** The next enclosing method. */
     def enclMethod: Symbol = if (isSourceMethod) this else owner.enclMethod
 
-    /** The primary constructor of a class. */
     def primaryConstructor: Symbol = NoSymbol
 
     /** The self symbol (a TermSymbol) of a class with explicit self type, or else the
@@ -3136,7 +3135,8 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
 
     override def primaryConstructor = {
       val c = info decl primaryConstructorName
-      if (c.isOverloaded) c.alternatives.head else c
+      if (isJavaDefined) NoSymbol // need to force info before checking the flag
+      else if (c.isOverloaded) c.alternatives.head else c
     }
 
     override def associatedFile = (
diff --git a/test/files/run/t6392b.check b/test/files/run/t6392b.check
index c2cc103373..3f191c7960 100644
--- a/test/files/run/t6392b.check
+++ b/test/files/run/t6392b.check
@@ -1 +1 @@
-ModuleDef(Modifiers(), TermName("C")#MOD, Template(List(Select(Ident(scala#PK), TypeName("AnyRef")#TPE)), noSelfType, List(DefDef(Modifiers(), nme.CONSTRUCTOR#PCTOR, List(), List(List()), TypeTree(), Block(List(Apply(Select(Super(This(TypeName("C")), tpnme.EMPTY), nme.CONSTRUCTOR#PCTOR), List())), Literal(Constant(())))))))
+ModuleDef(Modifiers(), TermName("C")#MOD, Template(List(Select(Ident(scala#PK), TypeName("AnyRef")#TPE)), noSelfType, List(DefDef(Modifiers(), nme.CONSTRUCTOR#PCTOR, List(), List(List()), TypeTree(), Block(List(Apply(Select(Super(This(TypeName("C")), tpnme.EMPTY), nme.CONSTRUCTOR#CTOR), List())), Literal(Constant(())))))))
diff --git a/test/files/run/t7582-private-within.check b/test/files/run/t7582-private-within.check
index b2743ffa06..1b9a0910af 100644
--- a/test/files/run/t7582-private-within.check
+++ b/test/files/run/t7582-private-within.check
@@ -2,7 +2,7 @@ private[package pack] class JavaPackagePrivate
 private[package pack] module JavaPackagePrivate
 private[package pack] module class JavaPackagePrivate
 private[package pack] field field
-private[package pack] primary constructor <init>
+private[package pack] constructor <init>
 private[package pack] method meth
 private[package pack] field staticField
 private[package pack] method staticMeth
diff --git a/test/files/run/t8192.check b/test/files/run/t8192.check
new file mode 100644
index 0000000000..7195703e19
--- /dev/null
+++ b/test/files/run/t8192.check
@@ -0,0 +1,44 @@
+compile-time
+class File
+primary constructor: NoSymbol
+def <init>(x$1: String): java.io.File => false
+def <init>(x$1: String,x$2: String): java.io.File => false
+def <init>(x$1: java.io.File,x$2: String): java.io.File => false
+def <init>(x$1: java.net.URI): java.io.File => false
+package scala
+primary constructor: NoSymbol
+object List
+primary constructor: def <init>(): scala.collection.immutable.List.type => true
+def <init>(): scala.collection.immutable.List.type => true
+trait Product1
+primary constructor: def $init$(): Unit => true
+class UninitializedFieldError
+primary constructor: def <init>(msg: String): UninitializedFieldError => true
+def <init>(msg: String): UninitializedFieldError => true
+def <init>(obj: Any): UninitializedFieldError => false
+class C
+primary constructor: def <init>(x: Int): C => true
+def <init>(x: Int): C => true
+def <init>(x: String): C => false
+runtime
+class File
+primary constructor: NoSymbol
+def <init>(x$1: java.io.File,x$2: java.lang.String): java.io.File => false
+def <init>(x$1: java.lang.String): java.io.File => false
+def <init>(x$1: java.lang.String,x$2: java.lang.String): java.io.File => false
+def <init>(x$1: java.net.URI): java.io.File => false
+package scala
+primary constructor: NoSymbol
+object List
+primary constructor: def <init>(): scala.collection.immutable.List.type => true
+def <init>(): scala.collection.immutable.List.type => true
+trait Product1
+primary constructor: def $init$(): Unit => true
+class UninitializedFieldError
+primary constructor: def <init>(msg: String): UninitializedFieldError => true
+def <init>(msg: String): UninitializedFieldError => true
+def <init>(obj: Any): UninitializedFieldError => false
+class C
+primary constructor: def <init>(x: Int): C => true
+def <init>(x: Int): C => true
+def <init>(x: String): C => false
diff --git a/test/files/run/t8192/Macros_1.scala b/test/files/run/t8192/Macros_1.scala
new file mode 100644
index 0000000000..2089273d6d
--- /dev/null
+++ b/test/files/run/t8192/Macros_1.scala
@@ -0,0 +1,44 @@
+import scala.reflect.macros.whitebox._
+import scala.language.experimental.macros
+import java.io._
+
+object Macros {
+  def impl(c: Context) = {
+    var messages = List[String]()
+    def println(msg: String) = messages :+= msg
+
+    import c.universe._
+    def test(sym: ClassSymbol): Unit = {
+      def fullyInitializeSymbol(sym: Symbol): Unit = {
+        val internal = c.universe.asInstanceOf[scala.reflect.internal.SymbolTable]
+        internal.definitions.fullyInitializeSymbol(sym.asInstanceOf[internal.Symbol])
+      }
+      def defString(sym: Symbol): String = {
+        val internal = c.universe.asInstanceOf[scala.reflect.internal.SymbolTable]
+        sym.asInstanceOf[internal.Symbol].defString
+      }
+      def showCtor(sym: Symbol): String = {
+        fullyInitializeSymbol(sym)
+        if (sym == NoSymbol) "NoSymbol"
+        else s"${defString(sym)} => ${sym.asMethod.isPrimaryConstructor}"
+      }
+      sym.typeSignature
+      println(sym.toString)
+      println(s"primary constructor: ${showCtor(sym.primaryConstructor)}")
+      val ctors = sym.typeSignature.members.filter(_.name == nme.CONSTRUCTOR).map(sym => showCtor(sym))
+      ctors.toList.sorted.foreach(println)
+    }
+
+    println("compile-time")
+    test(typeOf[File].typeSymbol.asClass)
+    test(definitions.ScalaPackageClass)
+    test(definitions.ListModule.moduleClass.asClass)
+    test(typeOf[Product1[_]].typeSymbol.asClass)
+    test(typeOf[UninitializedFieldError].typeSymbol.asClass)
+    test(c.mirror.staticClass("C").asClass)
+
+    q"..${messages.map(msg => q"println($msg)")}"
+  }
+
+  def foo: Any = macro impl
+}
\ No newline at end of file
diff --git a/test/files/run/t8192/Test_2.scala b/test/files/run/t8192/Test_2.scala
new file mode 100644
index 0000000000..15f684eb3f
--- /dev/null
+++ b/test/files/run/t8192/Test_2.scala
@@ -0,0 +1,39 @@
+import java.io._
+import scala.reflect.runtime.universe._
+import scala.reflect.runtime.{universe => ru}
+
+class C(x: Int) {
+  def this(x: String) = this(x.toInt)
+}
+
+object Test extends App {
+  def test(sym: ClassSymbol): Unit = {
+    def fullyInitializeSymbol(sym: Symbol): Unit = {
+      val internal = ru.asInstanceOf[scala.reflect.internal.SymbolTable]
+      internal.definitions.fullyInitializeSymbol(sym.asInstanceOf[internal.Symbol])
+    }
+    def defString(sym: Symbol): String = {
+      val internal = ru.asInstanceOf[scala.reflect.internal.SymbolTable]
+      sym.asInstanceOf[internal.Symbol].defString
+    }
+    def showCtor(sym: Symbol): String = {
+      fullyInitializeSymbol(sym)
+      if (sym == NoSymbol) "NoSymbol"
+      else s"${defString(sym)} => ${sym.asMethod.isPrimaryConstructor}"
+    }
+    sym.typeSignature
+    println(sym.toString)
+    println(s"primary constructor: ${showCtor(sym.primaryConstructor)}")
+    val ctors = sym.typeSignature.members.filter(_.name == nme.CONSTRUCTOR).map(sym => showCtor(sym))
+    ctors.toList.sorted.foreach(println)
+  }
+
+  Macros.foo
+  println("runtime")
+  test(typeOf[File].typeSymbol.asClass)
+  test(definitions.ScalaPackageClass)
+  test(definitions.ListModule.moduleClass.asClass)
+  test(typeOf[Product1[_]].typeSymbol.asClass)
+  test(typeOf[UninitializedFieldError].typeSymbol.asClass)
+  test(typeOf[C].typeSymbol.asClass)
+}