Merge pull request #9350 from TheElectronWill/bytecode-deprecated

Emit "deprecated" bytecode attribute

Author: TheElectronWill

compiler/src/dotty/tools/backend/jvm/BTypesFromSymbols.scala

@@ -298,10 +298,9 @@ class BTypesFromSymbols[I <: DottyBackendInterface](val int: I) extends BTypes {
    */
   final def javaFlags(sym: Symbol): Int = {
 
-
     val privateFlag = sym.is(Private) || (sym.isPrimaryConstructor && sym.owner.isTopLevelModuleClass)
 
-    val finalFlag = sym.is(Final) &&  !toDenot(sym).isClassConstructor && !(sym.is(Mutable)) &&  !(sym.enclosingClass.is(Trait))
+    val finalFlag = sym.is(Final) && !toDenot(sym).isClassConstructor && !sym.is(Mutable) && !sym.enclosingClass.is(Trait)
 
     import asm.Opcodes._
     GenBCodeOps.mkFlags(
@@ -318,23 +317,25 @@ class BTypesFromSymbols[I <: DottyBackendInterface](val int: I) extends BTypes {
         //  Mixin forwarders are bridges and can be final, but final bridges confuse some frameworks
         !sym.is(Bridge))
         ACC_FINAL else 0,
+
       if (sym.isStaticMember) ACC_STATIC else 0,
       if (sym.is(Bridge)) ACC_BRIDGE | ACC_SYNTHETIC else 0,
       if (sym.is(Artifact)) ACC_SYNTHETIC else 0,
       if (sym.isClass && !sym.isInterface) ACC_SUPER else 0,
       if (sym.isAllOf(JavaEnumTrait)) ACC_ENUM else 0,
       if (sym.is(JavaVarargs)) ACC_VARARGS else 0,
       if (sym.is(Synchronized)) ACC_SYNCHRONIZED else 0,
-      if (false /*sym.isDeprecated*/) asm.Opcodes.ACC_DEPRECATED else 0, // TODO: add an isDeprecated method in SymUtils
-      if (sym.is(Enum)) asm.Opcodes.ACC_ENUM else 0
+      if (sym.isDeprecated) ACC_DEPRECATED else 0,
+      if (sym.is(Enum)) ACC_ENUM else 0
     )
   }
 
   def javaFieldFlags(sym: Symbol) = {
+    import asm.Opcodes._
     javaFlags(sym) | GenBCodeOps.mkFlags(
-      if (sym hasAnnotation TransientAttr) asm.Opcodes.ACC_TRANSIENT else 0,
-      if (sym hasAnnotation VolatileAttr)  asm.Opcodes.ACC_VOLATILE  else 0,
-      if (sym.is(Mutable)) 0 else asm.Opcodes.ACC_FINAL
+      if (sym.hasAnnotation(TransientAttr)) ACC_TRANSIENT else 0,
+      if (sym.hasAnnotation(VolatileAttr)) ACC_VOLATILE else 0,
+      if (sym.is(Mutable)) 0 else ACC_FINAL
     )
   }
 }

compiler/src/dotty/tools/dotc/core/Annotations.scala

@@ -206,36 +206,6 @@ object Annotations {
     Annotation(defn.ThrowsAnnot.typeRef.appliedTo(tref), Ident(tref))
   }
 
-  /** A decorator that provides queries for specific annotations
-   *  of a symbol.
-   */
-  implicit class AnnotInfo(val sym: Symbol) extends AnyVal {
-
-    def isDeprecated(using Context): Boolean =
-      sym.hasAnnotation(defn.DeprecatedAnnot)
-
-    def deprecationMessage(using Context): Option[String] =
-      for {
-        annot <- sym.getAnnotation(defn.DeprecatedAnnot)
-        arg <- annot.argumentConstant(0)
-      }
-      yield arg.stringValue
-
-    def migrationVersion(using Context): Option[Try[ScalaVersion]] =
-      for {
-        annot <- sym.getAnnotation(defn.MigrationAnnot)
-        arg <- annot.argumentConstant(1)
-      }
-      yield ScalaVersion.parse(arg.stringValue)
-
-    def migrationMessage(using Context): Option[Try[ScalaVersion]] =
-      for {
-        annot <- sym.getAnnotation(defn.MigrationAnnot)
-        arg <- annot.argumentConstant(0)
-      }
-      yield ScalaVersion.parse(arg.stringValue)
-  }
-
   /** Extracts the type of the thrown exception from an annotation.
    *
    *  Supports both "old-style" `@throws(classOf[Exception])`

compiler/src/dotty/tools/dotc/reporting/messages.scala

@@ -1925,27 +1925,25 @@ object messages {
     def explain = "A sealed class or trait can only be extended in the same file as its declaration"
   }
 
-  class SymbolHasUnparsableVersionNumber(symbol: Symbol, migrationMessage: => String)(using Context)
+  class SymbolHasUnparsableVersionNumber(symbol: Symbol, errorMessage: String)(using Context)
   extends SyntaxMsg(SymbolHasUnparsableVersionNumberID) {
-    def msg = em"${symbol.showLocated} has an unparsable version number: $migrationMessage"
+    def msg = em"${symbol.showLocated} has an unparsable version number: $errorMessage"
     def explain =
-      em"""$migrationMessage
-          |
-          |The ${symbol.showLocated} is marked with ${hl("@migration")} indicating it has changed semantics
+      em"""The ${symbol.showLocated} is marked with ${hl("@migration")} indicating it has changed semantics
           |between versions and the ${hl("-Xmigration")} settings is used to warn about constructs
           |whose behavior may have changed since version change."""
   }
 
   class SymbolChangedSemanticsInVersion(
     symbol: Symbol,
-    migrationVersion: ScalaVersion
+    migrationVersion: ScalaVersion,
+    migrationMessage: String
   )(using Context) extends SyntaxMsg(SymbolChangedSemanticsInVersionID) {
-    def msg = em"${symbol.showLocated} has changed semantics in version $migrationVersion"
-    def explain = {
+    def msg = em"${symbol.showLocated} has changed semantics in version $migrationVersion: $migrationMessage"
+    def explain =
       em"""The ${symbol.showLocated} is marked with ${hl("@migration")} indicating it has changed semantics
           |between versions and the ${hl("-Xmigration")} settings is used to warn about constructs
           |whose behavior may have changed since version change."""
-    }
   }
 
   class UnableToEmitSwitch(tooFewCases: Boolean)(using Context)

compiler/src/dotty/tools/dotc/transform/Memoize.scala

@@ -96,10 +96,13 @@ class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase =>
         case _ => ()
       }
 
-    def removeAnnotations(denot: SymDenotation): Unit =
+    def removeUnwantedAnnotations(denot: SymDenotation): Unit =
       if (sym.annotations.nonEmpty) {
         val cpy = sym.copySymDenotation()
-        cpy.annotations = Nil
+        // Keep @deprecated annotation so that accessors can
+        // be marked as deprecated in the bytecode.
+        // TODO check the meta-annotations to know what to keep
+        cpy.filterAnnotations(_.matches(defn.DeprecatedAnnot))
         cpy.installAfter(thisPhase)
       }
 
@@ -135,7 +138,7 @@ class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase =>
           else transformFollowingDeep(ref(field))(using ctx.withOwner(sym))
         val getterDef = cpy.DefDef(tree)(rhs = getterRhs)
         addAnnotations(fieldDef.denot)
-        removeAnnotations(sym)
+        removeUnwantedAnnotations(sym)
         Thicket(fieldDef, getterDef)
       }
       else if (sym.isSetter) {
@@ -145,7 +148,7 @@ class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase =>
           if (isErasableBottomField(tree.vparamss.head.head.tpt.tpe.classSymbol)) Literal(Constant(()))
           else Assign(ref(field), adaptToField(ref(tree.vparamss.head.head.symbol)))
         val setterDef = cpy.DefDef(tree)(rhs = transformFollowingDeep(initializer)(using ctx.withOwner(sym)))
-        removeAnnotations(sym)
+        removeUnwantedAnnotations(sym)
         setterDef
       }
       else tree // curiously, some accessors from Scala2 have ' ' suffixes. They count as

compiler/src/dotty/tools/dotc/transform/SymUtils.scala

@@ -223,7 +223,10 @@ class SymUtils(val self: Symbol) extends AnyVal {
     self.is(ModuleClass) && self.sourceModule.is(Extension) && !self.sourceModule.isExtensionMethod
 
   def isScalaStatic(using Context): Boolean =
-    self.hasAnnotation(ctx.definitions.ScalaStaticAnnot)
+    self.hasAnnotation(defn.ScalaStaticAnnot)
+
+  def isDeprecated(using Context): Boolean =
+    self.hasAnnotation(defn.DeprecatedAnnot)
 
   /** Is symbol assumed or declared as an infix symbol? */
   def isDeclaredInfix(using Context): Boolean =

compiler/src/dotty/tools/dotc/typer/RefChecks.scala

@@ -11,17 +11,16 @@ import util.Spans._
 import util.{Store, SourcePosition}
 import scala.collection.{ mutable, immutable }
 import ast._
-import Trees._
 import MegaPhase._
 import config.Printers.{checks, noPrinter}
-import scala.util.Failure
-import config.NoScalaVersion
+import scala.util.{Try, Failure, Success}
+import config.{ScalaVersion, NoScalaVersion}
 import Decorators._
 import typer.ErrorReporting._
 import config.Feature.warnOnMigration
 
 object RefChecks {
-  import tpd._
+  import tpd.{Tree, MemberDef}
   import reporting.messages._
 
   val name: String = "refchecks"
@@ -817,24 +816,39 @@ object RefChecks {
   // 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: SourcePosition)(using Context): Unit = {
-    // 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 && !ctx.owner.ownersIterator.exists(_.isDeprecated))
-      ctx.deprecationWarning("%s is deprecated%s".format(
-        sym.showLocated, sym.deprecationMessage map (": " + _) getOrElse ""), pos)
-    // Similar to deprecation: check if the symbol is marked with @migration
-    // indicating it has changed semantics between versions.
+  private def checkUndesiredProperties(sym: Symbol, pos: SourcePosition)(using Context): Unit =
+    checkDeprecated(sym, pos)
+
     val xMigrationValue = ctx.settings.Xmigration.value
-    if (sym.hasAnnotation(defn.MigrationAnnot) && xMigrationValue != NoScalaVersion)
-      sym.migrationVersion.get match {
-        case scala.util.Success(symVersion) if xMigrationValue < symVersion=>
-          ctx.warning(SymbolChangedSemanticsInVersion(sym, symVersion), pos)
+    if xMigrationValue != NoScalaVersion then
+      checkMigration(sym, pos, xMigrationValue)
+
+
+  /** If @deprecated is present, 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: SourcePosition)(using Context): Unit =
+    for
+      annot <- sym.getAnnotation(defn.DeprecatedAnnot)
+      if !ctx.owner.ownersIterator.exists(_.isDeprecated)
+    do
+      val msg = annot.argumentConstant(0).map(": " + _.stringValue).getOrElse("")
+      val since = annot.argumentConstant(1).map(" since " + _.stringValue).getOrElse("")
+      ctx.deprecationWarning(s"${sym.showLocated} is deprecated${since}${msg}", pos)
+
+  /** If @migration is present (indicating that the symbol has changed semantics between versions),
+   *  emit a warning.
+   */
+  private def checkMigration(sym: Symbol, pos: SourcePosition, xMigrationValue: ScalaVersion)(using Context): Unit =
+    for annot <- sym.getAnnotation(defn.MigrationAnnot) do
+      val migrationVersion = ScalaVersion.parse(annot.argumentConstant(1).get.stringValue)
+      migrationVersion match
+        case Success(symVersion) if xMigrationValue < symVersion =>
+          val msg = annot.argumentConstant(0).get.stringValue
+          ctx.warning(SymbolChangedSemanticsInVersion(sym, symVersion, msg), pos)
         case Failure(ex) =>
-          ctx.warning(SymbolHasUnparsableVersionNumber(sym, ex.getMessage()), pos)
+          ctx.warning(SymbolHasUnparsableVersionNumber(sym, ex.getMessage), pos)
         case _ =>
-      }
-  }
 
   /** Check that a deprecated val or def does not override a
    *  concrete, non-deprecated method.  If it does, then

compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala

@@ -938,7 +938,7 @@ class TestBCode extends DottyBytecodeTest {
         |
         |}
       """.stripMargin
-    checkBCode(List(code)) { dir =>
+    checkBCode(code) { dir =>
       val c = loadClassNode(dir.lookupName("C.class", directory = false).input)
 
       assertInvoke(getMethod(c, "f1"), "[Ljava/lang/String;", "clone") // array descriptor as receiver
@@ -947,6 +947,36 @@ class TestBCode extends DottyBytecodeTest {
       assertInvoke(getMethod(c, "f4"), "java/lang/Object", "toString")
     }
   }
+
+  @Test
+  def deprecation(): Unit = {
+    val code =
+      """@deprecated
+        |class Test {
+        |  @deprecated
+        |  val v = 0
+        |
+        |  @deprecated
+        |  var x = 0
+        |
+        |  @deprecated("do not use this function!")
+        |  def f(): Unit = ()
+        |}
+      """.stripMargin
+
+    checkBCode(code) { dir =>
+      val c = loadClassNode(dir.lookupName("Test.class", directory = false).input)
+      assert((c.access & Opcodes.ACC_DEPRECATED) != 0)
+      assert((getMethod(c, "f").access & Opcodes.ACC_DEPRECATED) != 0)
+
+      assert((getField(c, "v").access & Opcodes.ACC_DEPRECATED) != 0)
+      assert((getMethod(c, "v").access & Opcodes.ACC_DEPRECATED) != 0)
+
+      assert((getField(c, "x").access & Opcodes.ACC_DEPRECATED) != 0)
+      assert((getMethod(c, "x").access & Opcodes.ACC_DEPRECATED) != 0)
+      assert((getMethod(c, "x_$eq").access & Opcodes.ACC_DEPRECATED) != 0)
+    }
+  }
 }
 
 object invocationReceiversTestCode {

language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala

@@ -19,7 +19,6 @@ import scala.io.Codec
 import dotc._
 import ast.{Trees, tpd, untpd}
 import core._, core.Decorators._
-import Annotations.AnnotInfo
 import Comments._, Constants._, Contexts._, Flags._, Names._, NameOps._, Symbols._, SymDenotations._, Trees._, Types._
 import classpath.ClassPathEntries
 import reporting._
@@ -856,7 +855,7 @@ object DottyLanguageServer {
       item.setDocumentation(hoverContent(None, documentation))
     }
 
-    item.setDeprecated(completion.symbols.forall(_.isDeprecated))
+    item.setDeprecated(completion.symbols.forall(_.hasAnnotation(defn.DeprecatedAnnot)))
     completion.symbols.headOption.foreach(s => item.setKind(completionItemKind(s)))
     item
   }