Improve implementation of implicit class defined in extern object (#3549

)

* Better handling of implicit class/extension defined within extern object + more tests
* Guard against usage of exten in non extern methods
* Fix test using non-allowed exten object defined in function

nativelib/src/main/scala/scala/scalanative/annotation/nonExtern.scala

@@ -0,0 +1,8 @@
+package scala.scalanative
+package annotation
+
+/** Internal annotation used in compiler plugin to exclude subset of extern type
+ *  definitions from of being treated as extern
+ */
+private final abstract class nonExtern()
+    extends scala.annotation.StaticAnnotation

nscplugin/src/main/scala-2/scala/scalanative/nscplugin/NirDefinitions.scala

@@ -43,6 +43,9 @@ trait NirDefinitions {
     lazy val ExternClass = getRequiredClass(
       "scala.scalanative.unsafe.package$extern"
     )
+    lazy val NonExternClass = getRequiredClass(
+      "scala.scalanative.annotation.nonExtern"
+    )
     lazy val BlockingClass = getRequiredClass(
       "scala.scalanative.unsafe.package$blocking"
     )

nscplugin/src/main/scala-2/scala/scalanative/nscplugin/NirGenExpr.scala

@@ -685,7 +685,7 @@ trait NirGenExpr[G <: nsc.Global with Singleton] { self: NirGenPhase[G] =>
       } else {
         val ty = genType(tree.symbol.tpe)
         val name = genFieldName(tree.symbol)
-        if (sym.owner.isExternType) {
+        if (sym.isExtern) {
           val externTy = genExternType(tree.symbol.tpe)
           genLoadExtern(ty, externTy, tree.symbol)
         } else {
@@ -712,7 +712,7 @@ trait NirGenExpr[G <: nsc.Global with Singleton] { self: NirGenPhase[G] =>
           val qual = genExpr(qualp)
           val rhs = genExpr(rhsp)
           val name = genFieldName(sym)
-          if (sym.owner.isExternType) {
+          if (sym.isExtern) {
             val externTy = genExternType(sym.tpe)
             genStoreExtern(externTy, sym, rhs)
           } else {
@@ -1126,8 +1126,13 @@ trait NirGenExpr[G <: nsc.Global with Singleton] { self: NirGenPhase[G] =>
       val Apply(fun, args) = app
 
       implicit val pos: nir.Position = app.pos
-
+      def fail(msg: String) = {
+        reporter.error(app.pos, msg)
+        Val.Null
+      }
       fun match {
+        case _ if fun.symbol == ExternMethod =>
+          fail(s"extern can be used only from non-inlined extern methods")
         case _: TypeApply =>
           genApplyTypeApply(app)
         case Select(Super(_, _), _) =>
@@ -2516,7 +2521,7 @@ trait NirGenExpr[G <: nsc.Global with Singleton] { self: NirGenPhase[G] =>
         selfp: Tree,
         argsp: Seq[Tree]
     )(implicit pos: nir.Position): Val = {
-      if (sym.owner.isExternType && sym.isAccessor) {
+      if (sym.isExtern && sym.isAccessor) {
         genApplyExternAccessor(sym, argsp)
       } else if (sym.isStaticMember) {
         genApplyStaticMethod(sym, selfp, argsp)
@@ -2531,7 +2536,7 @@ trait NirGenExpr[G <: nsc.Global with Singleton] { self: NirGenPhase[G] =>
         receiver: Tree,
         argsp: Seq[Tree]
     )(implicit pos: nir.Position): Val = {
-      require(!sym.owner.isExternType, sym.owner)
+      require(!sym.isExtern, sym.owner)
       val name = genStaticMemberName(sym, receiver.symbol)
       val method = Val.Global(name, nir.Type.Ptr)
       val sig = genMethodSig(sym)
@@ -2556,7 +2561,7 @@ trait NirGenExpr[G <: nsc.Global with Singleton] { self: NirGenPhase[G] =>
     def genLoadExtern(ty: nir.Type, externTy: nir.Type, sym: Symbol)(implicit
         pos: nir.Position
     ): Val = {
-      assert(sym.owner.isExternType, "loadExtern was not extern")
+      assert(sym.isExtern, "loadExtern was not extern")
 
       val name = Val.Global(genName(sym), Type.Ptr)
       val syncAttrs =
@@ -2572,7 +2577,7 @@ trait NirGenExpr[G <: nsc.Global with Singleton] { self: NirGenPhase[G] =>
     def genStoreExtern(externTy: nir.Type, sym: Symbol, value: Val)(implicit
         pos: nir.Position
     ): Val = {
-      assert(sym.owner.isExternType, "storeExtern was not extern")
+      assert(sym.isExtern, "storeExtern was not extern")
       val name = Val.Global(genName(sym), Type.Ptr)
       val externValue = toExtern(externTy, value)
       val syncAttrs =
@@ -2615,13 +2620,10 @@ trait NirGenExpr[G <: nsc.Global with Singleton] { self: NirGenPhase[G] =>
       val owner = sym.owner
       val name = genMethodName(sym)
       val origSig = genMethodSig(sym)
-      val isExtern = owner.isExternType
+      val isExtern = sym.isExtern
       val sig =
-        if (isExtern) {
-          genExternMethodSig(sym)
-        } else {
-          origSig
-        }
+        if (isExtern) genExternMethodSig(sym)
+        else origSig
       val args = genMethodArgs(sym, argsp)
       val method =
         if (statically || owner.isStruct || isExtern) {
@@ -2645,7 +2647,7 @@ trait NirGenExpr[G <: nsc.Global with Singleton] { self: NirGenPhase[G] =>
     }
 
     def genMethodArgs(sym: Symbol, argsp: Seq[Tree]): Seq[Val] =
-      if (sym.owner.isExternType) genExternMethodArgs(sym, argsp)
+      if (sym.isExtern) genExternMethodArgs(sym, argsp)
       else genSimpleArgs(argsp)
 
     private def genSimpleArgs(argsp: Seq[Tree]): Seq[Val] = argsp.map(genExpr)

nscplugin/src/main/scala-2/scala/scalanative/nscplugin/NirGenName.scala

@@ -72,7 +72,7 @@ trait NirGenName[G <: Global with Singleton] {
     }
 
     owner.member {
-      if (sym.owner.isExternType) {
+      if (sym.isExtern) {
         nir.Sig.Extern(id)
       } else {
         nir.Sig.Field(id, scope)
@@ -94,7 +94,7 @@ trait NirGenName[G <: Global with Singleton] {
 
     val paramTypes = tpe.params.toSeq.map(p => genType(p.info))
 
-    def isExtern = sym.owner.isExternType
+    def isExtern = sym.isExtern
 
     if (sym == String_+)
       genMethodName(StringConcatMethod)
@@ -156,7 +156,7 @@ trait NirGenName[G <: Global with Singleton] {
   private def nativeIdOf(sym: Symbol): String = {
     sym.getAnnotation(NameClass).flatMap(_.stringArg(0)).getOrElse {
       val name = sym.javaSimpleName.toString()
-      val id: String = if (sym.owner.isExternType) {
+      val id: String = if (sym.isExtern) {
         // Don't use encoded names for externs
         sym.decodedName.trim()
       } else if (sym.isField) {

nscplugin/src/main/scala-2/scala/scalanative/nscplugin/NirGenStat.scala

@@ -239,7 +239,7 @@ trait NirGenStat[G <: nsc.Global with Singleton] { self: NirGenPhase[G] =>
 
       for (f <- sym.info.decls
           if !f.isMethod && f.isTerm && !f.isModule) {
-        if (f.owner.isExternType && !f.isMutable) {
+        if (f.isExtern && !f.isMutable) {
           reporter.error(f.pos, "`extern` cannot be used in val definition")
         }
         val ty = genType(f.tpe)
@@ -665,8 +665,7 @@ trait NirGenStat[G <: nsc.Global with Singleton] { self: NirGenPhase[G] =>
         val sym = dd.symbol
         val owner = curClassSym.get
         // implicit class is erased to method at this point
-        def isImplicitClass = sym.isImplicit && sym.isSynthetic
-        val isExtern = owner.isExternType && !isImplicitClass
+        val isExtern = sym.isExtern
         val attrs = genMethodAttrs(sym, isExtern)
         val name = genMethodName(sym)
         val sig = genMethodSig(sym)
@@ -681,14 +680,14 @@ trait NirGenStat[G <: nsc.Global with Singleton] { self: NirGenPhase[G] =>
               )
             )
 
-          case _ if dd.symbol.isConstructor && owner.isExternType =>
+          case _ if dd.symbol.isConstructor && isExtern =>
             validateExternCtor(dd.rhs)
             None
 
           case _ if dd.name == nme.CONSTRUCTOR && owner.isStruct =>
             None
 
-          case rhs if owner.isExternType && !isImplicitClass =>
+          case rhs if isExtern =>
             checkExplicitReturnTypeAnnotation(dd, "extern method")
             genExternMethod(attrs, name, sig, dd)
 
@@ -836,7 +835,7 @@ trait NirGenStat[G <: nsc.Global with Singleton] { self: NirGenPhase[G] =>
       }
 
       def isCallingExternMethod(sym: Symbol) =
-        sym.owner.isExternType
+        sym.isExtern
 
       def isExternMethodAlias(target: Symbol) =
         (name, genName(target)) match {
@@ -1158,7 +1157,7 @@ trait NirGenStat[G <: nsc.Global with Singleton] { self: NirGenPhase[G] =>
       }
 
       m.isDeferred || m.isConstructor || m.hasAccessBoundary ||
-        m.owner.isExternType ||
+        m.isExtern ||
         isOfJLObject
     }
 

nscplugin/src/main/scala-2/scala/scalanative/nscplugin/NirGenType.scala

@@ -20,12 +20,15 @@ trait NirGenType[G <: Global with Singleton] { self: NirGenPhase[G] =>
       sym.isModuleClass && !sym.isLifted
 
     def isStaticInNIR: Boolean =
-      sym.owner.isExternType || sym.isStaticMember
+      sym.isExtern || sym.isStaticMember
 
     def isExternType: Boolean =
       (isScalaModule || sym.isTraitOrInterface) &&
         sym.annotations.exists(_.symbol == ExternClass)
 
+    def isExtern: Boolean = (sym.isExternType || sym.owner.isExternType) &&
+      !sym.annotations.exists(_.symbol == NonExternClass)
+
     def isBlocking: Boolean =
       sym.annotations.exists(_.symbol == BlockingClass)
 
@@ -239,7 +242,7 @@ trait NirGenType[G <: Global with Singleton] { self: NirGenPhase[G] =>
       isExtern: Boolean
   ): Seq[nir.Type] = {
     val params = sym.tpe.params
-    if (!isExtern && !sym.owner.isExternType)
+    if (!isExtern && !sym.isExtern)
       params.map { p => genType(p.tpe) }
     else {
       val wereRepeated = exitingPhase(currentRun.typerPhase) {

nscplugin/src/main/scala-2/scala/scalanative/nscplugin/PrepNativeInterop.scala

@@ -104,7 +104,6 @@ abstract class PrepNativeInterop[G <: Global with Singleton](
           // Replace call by literal constant containing type
           if (typer.checkClassTypeOrModule(tpeArg)) {
             val widenedTpe = tpeArg.tpe.dealias.widen
-            println("rewriting class of for" + widenedTpe)
             typer.typed { Literal(Constant(widenedTpe)) }
           } else {
             reporter.error(tpeArg.pos, s"Type ${tpeArg} is not a class type")
@@ -161,6 +160,16 @@ abstract class PrepNativeInterop[G <: Global with Singleton](
         case modDef: ModuleDef =>
           enterOwner(OwnerKind.NonEnumScalaMod) { super.transform(modDef) }
 
+        case dd: DefDef if isExternType(dd.symbol.owner) =>
+          val sym = dd.symbol
+          val resultType = sym.tpe.finalResultType.typeSymbol
+          val isImplicitClassCtor = (sym.isImplicit && sym.isSynthetic) &&
+            resultType.isClass && resultType.isImplicit &&
+            resultType.name.toTermName == sym.name
+
+          if (isImplicitClassCtor) sym.addAnnotation(NonExternClass)
+          super.transform(tree)
+
         // ValOrDefDef's that are local to a block must not be transformed
         case vddef: ValOrDefDef if vddef.symbol.isLocalToBlock =>
           super.transform(tree)
@@ -236,6 +245,18 @@ abstract class PrepNativeInterop[G <: Global with Singleton](
     }
   }
 
+  private def isExternType(sym: Symbol): Boolean = {
+    sym != null &&
+    (sym.isModuleClass || sym.isTraitOrInterface) &&
+    sym.annotations.exists(_.symbol == ExternAnnotationClass)
+  }
+
+  // Differs from ExternClass defined in NirDefinitions, but points to the same type
+  // At the phases of prepNativeInterop the symbol has different name
+  private lazy val ExternAnnotationClass = rootMirror.getRequiredClass(
+    "scala.scalanative.unsafe.extern"
+  )
+
   private def isScalaEnum(implDef: ImplDef) =
     implDef.symbol.tpe.typeSymbol isSubClass EnumerationClass
 

nscplugin/src/main/scala-3/scala/scalanative/nscplugin/NirDefinitions.scala

@@ -30,6 +30,7 @@ final class NirDefinitions()(using ctx: Context) {
   @tu lazy val NameClass = requiredClass("scala.scalanative.unsafe.name")
   @tu lazy val LinkClass = requiredClass("scala.scalanative.unsafe.link")
   @tu lazy val ExternClass = requiredClass("scala.scalanative.unsafe.extern")
+  @tu lazy val NonExternClass = requiredClass("scala.scalanative.annotation.nonExtern")
   @tu lazy val BlockingClass = requiredClass("scala.scalanative.unsafe.blocking")
   @tu lazy val StructClass = requiredClass("scala.scalanative.runtime.struct")
   @tu lazy val ResolvedAtLinktimeClass = requiredClass("scala.scalanative.unsafe.resolvedAtLinktime")

nscplugin/src/main/scala-3/scala/scalanative/nscplugin/NirGenExpr.scala

@@ -93,7 +93,14 @@ trait NirGenExpr(using Context) {
       def qualifier = qualifier0.withSpan(qualifier0.span.orElse(fun.span))
       def arg = args.head
 
+      inline def fail(msg: String)(using Context) = {
+        report.error(msg, app.srcPos)
+        Val.Null
+      }
       fun match {
+        case _ if sym == defnNir.UnsafePackage_extern =>
+          fail(s"extern can be used only from non-inlined extern methods")
+
         case _: TypeApply => genApplyTypeApply(app)
         case Select(Super(_, _), _) =>
           genApplyMethod(

nscplugin/src/main/scala-3/scala/scalanative/nscplugin/NirGenStat.scala

@@ -259,17 +259,7 @@ trait NirGenStat(using Context) {
       val sym = dd.symbol
       val owner = curClassSym.get
 
-      // warning: In Scala3 we cannot reliably distinguish implicit class from implicit def!
-      // This means that non-extern implicit def methods can be defined in extern object
-      // To distinguish betwen extern def and allowed implicit class check rhs = unsafe.extern()
-      // Extensions can be easilly identified via flag
-      def isExtension =
-        sym.flags.is(Extension) || {
-          sym.flags.isAllOf(Implicit | Final) &&
-          dd.paramss.headOption.exists(_.size == 1) &&
-          !ApplyExtern.unapply(dd.rhs)
-        }
-      val isExtern = sym.isExtern && !isExtension
+      val isExtern = sym.isExtern
 
       val attrs = genMethodAttrs(sym, isExtern)
       val name = genMethodName(sym)
@@ -726,8 +716,7 @@ trait NirGenStat(using Context) {
     }
   }
 
-  /** Gen the static forwarders for the methods of a module class.
-   *
+  /** Gen the static forwarders for the methods of a module class. l
    *  Precondition: `isCandidateForForwarders(moduleClass)` is true
    */
   private def genStaticForwardersFromModuleClass(

nscplugin/src/main/scala-3/scala/scalanative/nscplugin/NirGenType.scala

@@ -52,7 +52,14 @@ trait NirGenType(using Context) {
       sym.owner.isExternType ||
       sym.hasAnnotation(defnNir.ExternClass) ||
       (sym.is(Accessor) && sym.field.isExtern)
-    }
+    } && !sym.hasAnnotation(
+      defnNir.NonExternClass
+    ) // Added in PrepNativeInterop
+
+    def isExtensionMethod: Boolean =
+      sym.flags.isAllOf(Extension | Method) || {
+        sym.flags.isAllOf(Final | Implicit | Method)
+      }
 
     def isExternType: Boolean =
       (isScalaModule || sym.isTraitOrInterface) &&

nscplugin/src/main/scala-3/scala/scalanative/nscplugin/PrepNativeInterop.scala

@@ -79,6 +79,26 @@ class PrepNativeInterop extends PluginPhase with NativeInteropUtil {
       sym.addAnnotation(defnNir.ExternClass)
     }
 
+    if (sym.owner.isExternType) {
+      def isImplicitClassCtor = sym.paramInfo.stripPoly.stripped match {
+        case core.Types.MethodTpe(_, _, resultTpe) =>
+          resultTpe.typeSymbol.isClass && resultTpe.typeSymbol.is(Implicit) &&
+            resultTpe.typeSymbol.fullName.toSimpleName == sym.fullName.toSimpleName
+        case _ => false
+      }
+      val isExtension = sym.is(Extension)
+      if isExtension || isImplicitClassCtor
+      then
+        sym.addAnnotation(defnNir.NonExternClass)
+        if isExtension &&
+            dd.rhs.existsSubTree(_.symbol == defnNir.UnsafePackage_extern)
+        then
+          report.error(
+            "Extensions cannot be defined as extern methods",
+            dd.rhs.srcPos
+          )
+    }
+
     if sym.is(Inline) then
       if sym.isExtern then
         report.error("Extern method cannot be inlined", dd.srcPos)

nscplugin/src/test/scala-3/scala/NativeCompilerTest.scala

@@ -112,3 +112,21 @@ class NativeCompilerTest:
       |}
       |""".stripMargin
   )
+
+  @Test def disallowExtensionInExternWithExtern(): Unit = {
+    val err = assertThrows(
+      classOf[CompilationFailedException],
+      () => NIRCompiler(_.compile("""import scala.scalanative.unsafe.extern
+        |@extern object Dummy { 
+        |  extension(v: Int) { 
+        |    def convert(): Long = extern
+        |  }
+        |}
+        |""".stripMargin))
+    )
+    assertTrue(
+      err
+        .getMessage()
+        .contains("Extensions cannot be defined as extern methods")
+    )
+  }