From f2ba6f42794ebfdf4593a6442fd0adfcd123522e Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Mon, 11 Mar 2024 09:03:48 +0100 Subject: [PATCH] Check deprecation of inline methods We must check these constraint just before inlining as later on there on the call might completely disappear. We do the same as we did for experimental definition checks. Fixes #19913 --- .../dotty/tools/dotc/inlines/Inlines.scala | 2 +- .../tools/dotc/transform/PostTyper.scala | 2 +- .../tools/dotc/typer/CrossVersionChecks.scala | 95 ++++++++++--------- scaladoc-testcases/src/tests/hugetype.scala | 3 +- tests/warn/i19913.check | 8 ++ tests/warn/i19913.scala | 13 +++ 6 files changed, 75 insertions(+), 48 deletions(-) create mode 100644 tests/warn/i19913.check create mode 100644 tests/warn/i19913.scala diff --git a/compiler/src/dotty/tools/dotc/inlines/Inlines.scala b/compiler/src/dotty/tools/dotc/inlines/Inlines.scala index fef1a5f90141..65792d09f88c 100644 --- a/compiler/src/dotty/tools/dotc/inlines/Inlines.scala +++ b/compiler/src/dotty/tools/dotc/inlines/Inlines.scala @@ -100,7 +100,7 @@ object Inlines: if ctx.isAfterTyper then // During typer we wait with cross version checks until PostTyper, in order // not to provoke cyclic references. See i16116 for a test case. - CrossVersionChecks.checkExperimentalRef(tree.symbol, tree.srcPos) + CrossVersionChecks.checkRef(tree.symbol, tree.srcPos) if tree.symbol.isConstructor then return tree // error already reported for the inline constructor definition diff --git a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala index 4ce41ca21bfe..3bcec80b5b10 100644 --- a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala +++ b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala @@ -369,7 +369,7 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase => } case tree @ Inlined(call, bindings, expansion) if !tree.inlinedFromOuterScope => val pos = call.sourcePos - CrossVersionChecks.checkExperimentalRef(call.symbol, pos) + CrossVersionChecks.checkRef(call.symbol, pos) withMode(Mode.NoInline)(transform(call)) val callTrace = Inlines.inlineCallTrace(call.symbol, pos)(using ctx.withSource(pos.source)) cpy.Inlined(tree)(callTrace, transformSub(bindings), transform(expansion)(using inlineContext(tree))) diff --git a/compiler/src/dotty/tools/dotc/typer/CrossVersionChecks.scala b/compiler/src/dotty/tools/dotc/typer/CrossVersionChecks.scala index c22d03ca77d7..1e0907ee74a6 100644 --- a/compiler/src/dotty/tools/dotc/typer/CrossVersionChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/CrossVersionChecks.scala @@ -24,54 +24,13 @@ class CrossVersionChecks extends MiniPhase: // 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: SrcPos)(using Context): Unit = - checkDeprecated(sym, pos) - checkExperimentalRef(sym, pos) + checkRef(sym, pos) val xMigrationValue = ctx.settings.Xmigration.value if xMigrationValue != NoScalaVersion then checkMigration(sym, pos, xMigrationValue) end checkUndesiredProperties - /**Skip warnings for synthetic members of case classes during declaration and - * scan the chain of outer declaring scopes from the current context - * a deprecation warning will be skipped if one the following holds - * for a given declaring scope: - * - the symbol associated with the scope is also deprecated. - * - if and only if `sym` is an enum case, the scope is either - * a module that declares `sym`, or the companion class of the - * module that declares `sym`. - */ - def skipWarning(sym: Symbol)(using Context): Boolean = - - /** is the owner an enum or its companion and also the owner of sym */ - def isEnumOwner(owner: Symbol)(using Context) = - // pre: sym is an enumcase - if owner.isEnumClass then owner.companionClass eq sym.owner - else if owner.is(ModuleClass) && owner.companionClass.isEnumClass then owner eq sym.owner - else false - - def isDeprecatedOrEnum(owner: Symbol)(using Context) = - // pre: sym is an enumcase - owner.isDeprecated || isEnumOwner(owner) - - (ctx.owner.is(Synthetic) && sym.is(CaseClass)) - || ctx.owner.ownersIterator.exists(if sym.isEnumCase then isDeprecatedOrEnum else _.isDeprecated) - end skipWarning - - - /** 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: SrcPos)(using Context): Unit = - - // Also check for deprecation of the companion class for synthetic methods - val toCheck = sym :: (if sym.isAllOf(SyntheticMethod) then sym.owner.companionClass :: Nil else Nil) - for sym <- toCheck; annot <- sym.getAnnotation(defn.DeprecatedAnnot) do - if !skipWarning(sym) then - val msg = annot.argumentConstant(0).map(": " + _.stringValue).getOrElse("") - val since = annot.argumentConstant(1).map(" since " + _.stringValue).getOrElse("") - report.deprecationWarning(em"${sym.showLocated} is deprecated${since}${msg}", pos) - private def checkExperimentalAnnots(sym: Symbol)(using Context): Unit = if sym.exists && !sym.isInExperimentalScope then for annot <- sym.annotations if annot.symbol.isExperimental do @@ -160,11 +119,11 @@ class CrossVersionChecks extends MiniPhase: tpe.foreachPart { case TypeRef(_, sym: Symbol) => if tree.span.isSourceDerived then - checkDeprecated(sym, tree.srcPos) + checkDeprecatedRef(sym, tree.srcPos) checkExperimentalRef(sym, tree.srcPos) case TermRef(_, sym: Symbol) => if tree.span.isSourceDerived then - checkDeprecated(sym, tree.srcPos) + checkDeprecatedRef(sym, tree.srcPos) checkExperimentalRef(sym, tree.srcPos) case _ => } @@ -186,9 +145,55 @@ object CrossVersionChecks: val name: String = "crossVersionChecks" val description: String = "check issues related to deprecated and experimental" + /** Check that a reference to an experimental definition with symbol `sym` meets cross-version constraints + * for `@deprecated` and `@experimental`. + */ + def checkRef(sym: Symbol, pos: SrcPos)(using Context): Unit = + checkDeprecatedRef(sym, pos) + checkExperimentalRef(sym, pos) + /** Check that a reference to an experimental definition with symbol `sym` is only * used in an experimental scope */ - def checkExperimentalRef(sym: Symbol, pos: SrcPos)(using Context): Unit = + private[CrossVersionChecks] def checkExperimentalRef(sym: Symbol, pos: SrcPos)(using Context): Unit = if sym.isExperimental && !ctx.owner.isInExperimentalScope then Feature.checkExperimentalDef(sym, pos) + + /** 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[CrossVersionChecks] def checkDeprecatedRef(sym: Symbol, pos: SrcPos)(using Context): Unit = + + // Also check for deprecation of the companion class for synthetic methods + val toCheck = sym :: (if sym.isAllOf(SyntheticMethod) then sym.owner.companionClass :: Nil else Nil) + for sym <- toCheck; annot <- sym.getAnnotation(defn.DeprecatedAnnot) do + if !skipWarning(sym) then + val msg = annot.argumentConstant(0).map(": " + _.stringValue).getOrElse("") + val since = annot.argumentConstant(1).map(" since " + _.stringValue).getOrElse("") + report.deprecationWarning(em"${sym.showLocated} is deprecated${since}${msg}", pos) + + /** Skip warnings for synthetic members of case classes during declaration and + * scan the chain of outer declaring scopes from the current context + * a deprecation warning will be skipped if one the following holds + * for a given declaring scope: + * - the symbol associated with the scope is also deprecated. + * - if and only if `sym` is an enum case, the scope is either + * a module that declares `sym`, or the companion class of the + * module that declares `sym`. + */ + private def skipWarning(sym: Symbol)(using Context): Boolean = + + /** is the owner an enum or its companion and also the owner of sym */ + def isEnumOwner(owner: Symbol)(using Context) = + // pre: sym is an enumcase + if owner.isEnumClass then owner.companionClass eq sym.owner + else if owner.is(ModuleClass) && owner.companionClass.isEnumClass then owner eq sym.owner + else false + + def isDeprecatedOrEnum(owner: Symbol)(using Context) = + // pre: sym is an enumcase + owner.isDeprecated || isEnumOwner(owner) + + (ctx.owner.is(Synthetic) && sym.is(CaseClass)) + || ctx.owner.ownersIterator.exists(if sym.isEnumCase then isDeprecatedOrEnum else _.isDeprecated) + end skipWarning diff --git a/scaladoc-testcases/src/tests/hugetype.scala b/scaladoc-testcases/src/tests/hugetype.scala index 2f0f7f79540b..fe1905cb87cc 100644 --- a/scaladoc-testcases/src/tests/hugetype.scala +++ b/scaladoc-testcases/src/tests/hugetype.scala @@ -3,6 +3,7 @@ package tests.hugetype import compiletime._ import compiletime.ops.int._ import scala.annotation.experimental +import scala.annotation.nowarn /** * a particular group of people or things that share similar characteristics and form a smaller division of a larger set: @@ -39,7 +40,7 @@ trait XD extends E: */ @experimental @deprecated - protected override final implicit transparent inline abstract infix def same[A](a: A): A = a + protected override final implicit transparent inline abstract infix def same[A](a: A): A = a: @nowarn trait Parent[X, A[_], B[_, _]] diff --git a/tests/warn/i19913.check b/tests/warn/i19913.check new file mode 100644 index 000000000000..4f532393aa45 --- /dev/null +++ b/tests/warn/i19913.check @@ -0,0 +1,8 @@ +-- Deprecation Warning: tests/warn/i19913.scala:13:25 ------------------------------------------------------------------ +13 | Deprecated.inlineMethod() // warn + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | method inlineMethod in object Deprecated is deprecated: test +-- Deprecation Warning: tests/warn/i19913.scala:12:13 ------------------------------------------------------------------ +12 | Deprecated.method() // warn + | ^^^^^^^^^^^^^^^^^ + | method method in object Deprecated is deprecated: test diff --git a/tests/warn/i19913.scala b/tests/warn/i19913.scala new file mode 100644 index 000000000000..f3b0a95858a4 --- /dev/null +++ b/tests/warn/i19913.scala @@ -0,0 +1,13 @@ +//> using options -deprecation + +object Deprecated: + + @deprecated("test") + def method() = ??? + + @deprecated("test") + inline def inlineMethod() = ??? + +object Test: + Deprecated.method() // warn + Deprecated.inlineMethod() // warn