Skip to content

Commit

Permalink
Check deprecation of inline methods (#19914)
Browse files Browse the repository at this point in the history
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


### Changes

- Define `CrossVersionChecks.checkRef` that checks both deprecation and
experimental.
- Used in `Inlines` and `PostTyper` to check the deprecation of inlined
calls.
- Rename `checkDeprecated` to `checkDeprecatedRef`
- Move `checkDeprecatedRef` and `skipWarning` to `object
CrossVersionChecks`
  • Loading branch information
nicolasstucki committed Mar 19, 2024
2 parents 65972b4 + f2ba6f4 commit 1eb466c
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 48 deletions.
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/inlines/Inlines.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/PostTyper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Expand Down
95 changes: 50 additions & 45 deletions compiler/src/dotty/tools/dotc/typer/CrossVersionChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 _ =>
}
Expand All @@ -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
3 changes: 2 additions & 1 deletion scaladoc-testcases/src/tests/hugetype.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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[_, _]]

Expand Down
8 changes: 8 additions & 0 deletions tests/warn/i19913.check
Original file line number Diff line number Diff line change
@@ -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
13 changes: 13 additions & 0 deletions tests/warn/i19913.scala
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 1eb466c

Please sign in to comment.