Skip to content

Commit 368cca1

Browse files
oderskyodersky
committed
Reorganize RefChecks tasks
 - Split out forward reference checks and cross version (i.e. experimental/deprecated) checks into their own miniphases. These have nothing to do with the core tasks of RefChecks. - Move RefChecks earlier in the pipeline, so that it is not affected by ElimByName's questionable type manipulations.
1 parent 59734f5 commit 368cca1

File tree

5 files changed

+344
-274
lines changed

5 files changed

+344
-274
lines changed

‎compiler/src/dotty/tools/dotc/Compiler.scala

+7-5
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ import typer.{TyperPhase, RefChecks}
77
import parsing.Parser
88
import Phases.Phase
99
import transform._
10-
import dotty.tools.backend.jvm.{CollectSuperCalls, GenBCode}
1110
import dotty.tools.backend
12-
import dotty.tools.dotc.transform.localopt.StringInterpolatorOpt
11+
import backend.jvm.{CollectSuperCalls, GenBCode}
12+
import localopt.StringInterpolatorOpt
1313

1414
/** The central class of the dotc compiler. The job of a compiler is to create
1515
* runs, which process given `phases` in a given `rootContext`.
@@ -68,15 +68,17 @@ class Compiler {
6868
new BetaReduce, // Reduce closure applications
6969
new InlineVals, // Check right hand-sides of an `inline val`s
7070
new ExpandSAMs, // Expand single abstract method closures to anonymous classes
71-
new ElimRepeated) :: // Rewrite vararg parameters and arguments
71+
new ElimRepeated, // Rewrite vararg parameters and arguments
72+
new RefChecks) :: // Various checks mostly related to abstract members and overriding
7273
List(new init.Checker) :: // Check initialization of objects
73-
List(new ProtectedAccessors, // Add accessors for protected members
74+
List(new CrossVersionChecks, // Check issues related to deprecated and experimental
75+
new ProtectedAccessors, // Add accessors for protected members
7476
new ExtensionMethods, // Expand methods of value classes with extension methods
7577
new UncacheGivenAliases, // Avoid caching RHS of simple parameterless given aliases
7678
new ElimByName, // Map by-name parameters to functions
7779
new HoistSuperArgs, // Hoist complex arguments of supercalls to enclosing scope
80+
new ForwardDepChecks, // Check that there are no forward references to local vals
7881
new SpecializeApplyMethods, // Adds specialized methods to FunctionN
79-
new RefChecks, // Various checks mostly related to abstract members and overriding
8082
new TryCatchPatterns, // Compile cases in try/catch
8183
new PatternMatcher) :: // Compile pattern matches
8284
List(new ElimOpaque, // Turn opaque into normal aliases

‎compiler/src/dotty/tools/dotc/transform/ElimByName.scala

+6-2
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import NameKinds.SuperArgName
1313
import core.StdNames.nme
1414
import MegaPhase.*
1515
import Decorators.*
16+
import typer.RefChecks
1617
import reporting.trace
1718

1819
/** This phase implements the following transformations:
@@ -55,12 +56,15 @@ class ElimByName extends MiniPhase, InfoTransformer:
5556

5657
override def phaseName: String = ElimByName.name
5758

58-
override def runsAfterGroupsOf: Set[String] = Set(ExpandSAMs.name, ElimRepeated.name)
59+
override def runsAfterGroupsOf: Set[String] = Set(ExpandSAMs.name, ElimRepeated.name, RefChecks.name)
5960
// - ExpanSAMs applied to partial functions creates methods that need
6061
// to be fully defined before converting. Test case is pos/i9391.scala.
61-
// - ByNameLambda needs to run in a group after ElimRepeated since ElimRepeated
62+
// - ElimByName needs to run in a group after ElimRepeated since ElimRepeated
6263
// works on simple arguments but not converted closures, and it sees the arguments
6364
// after transformations by subsequent miniphases in the same group.
65+
// - ElimByName should run in a group after RefChecks, since RefChecks does heavy
66+
// comparisons of signatures, and ElimByName distorts these signatures by not
67+
// replacing `=>` with `() ?=> T` everywhere.
6468

6569
override def changesParents: Boolean = true
6670
// Expr types in parent type arguments are changed to function types.

‎compiler/src/dotty/tools/dotc/transform/ForwardDepChecks.scala

+111
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
package dotty.tools
2+
package dotc
3+
package transform
4+
5+
import core.*
6+
import Symbols.*, Types.*, Contexts.*, Flags.*, Decorators.*, reporting.*
7+
import util.Spans.Span
8+
import util.Store
9+
import collection.immutable
10+
import ast.tpd
11+
import MegaPhase.MiniPhase
12+
13+
object ForwardDepChecks:
14+
15+
import tpd.*
16+
17+
val name: String = "forwardDepChecks"
18+
19+
type LevelAndIndex = immutable.Map[Symbol, (LevelInfo, Int)]
20+
21+
class OptLevelInfo {
22+
def levelAndIndex: LevelAndIndex = Map()
23+
def enterReference(sym: Symbol, span: Span): Unit = ()
24+
}
25+
26+
/** A class to help in forward reference checking */
27+
class LevelInfo(outerLevelAndIndex: LevelAndIndex, stats: List[Tree])(using Context)
28+
extends OptLevelInfo {
29+
override val levelAndIndex: LevelAndIndex =
30+
stats.foldLeft(outerLevelAndIndex, 0) {(mi, stat) =>
31+
val (m, idx) = mi
32+
val m1 = stat match {
33+
case stat: MemberDef => m.updated(stat.symbol, (this, idx))
34+
case _ => m
35+
}
36+
(m1, idx + 1)
37+
}._1
38+
var maxIndex: Int = Int.MinValue
39+
var refSpan: Span = _
40+
var refSym: Symbol = _
41+
42+
override def enterReference(sym: Symbol, span: Span): Unit =
43+
if (sym.exists && sym.owner.isTerm)
44+
levelAndIndex.get(sym) match {
45+
case Some((level, idx)) if (level.maxIndex < idx) =>
46+
level.maxIndex = idx
47+
level.refSpan = span
48+
level.refSym = sym
49+
case _ =>
50+
}
51+
}
52+
53+
val NoLevelInfo: OptLevelInfo = new OptLevelInfo()
54+
55+
class ForwardDepChecks extends MiniPhase:
56+
import ForwardDepChecks.*
57+
import tpd.*
58+
59+
override def phaseName: String = ForwardDepChecks.name
60+
61+
override def runsAfter: Set[String] = Set(ElimByName.name)
62+
63+
private var LevelInfo: Store.Location[OptLevelInfo] = _
64+
private def currentLevel(using Context): OptLevelInfo = ctx.store(LevelInfo)
65+
66+
override def initContext(ctx: FreshContext): Unit =
67+
LevelInfo = ctx.addLocation(NoLevelInfo)
68+
69+
override def prepareForStats(trees: List[Tree])(using Context): Context =
70+
if (ctx.owner.isTerm)
71+
ctx.fresh.updateStore(LevelInfo, new LevelInfo(currentLevel.levelAndIndex, trees))
72+
else ctx
73+
74+
override def transformValDef(tree: ValDef)(using Context): ValDef =
75+
val sym = tree.symbol
76+
if sym.exists && sym.owner.isTerm && !sym.is(Lazy) then
77+
currentLevel.levelAndIndex.get(sym) match
78+
case Some((level, symIdx)) if symIdx <= level.maxIndex =>
79+
report.error(ForwardReferenceExtendsOverDefinition(sym, level.refSym),
80+
ctx.source.atSpan(level.refSpan))
81+
case _ =>
82+
tree
83+
84+
override def transformIdent(tree: Ident)(using Context): Ident = {
85+
currentLevel.enterReference(tree.symbol, tree.span)
86+
tree
87+
}
88+
89+
override def transformApply(tree: Apply)(using Context): Apply = {
90+
if (isSelfConstrCall(tree)) {
91+
assert(currentLevel.isInstanceOf[LevelInfo], s"${ctx.owner}/" + i"$tree")
92+
val level = currentLevel.asInstanceOf[LevelInfo]
93+
if (level.maxIndex > 0) {
94+
// An implementation restriction to avoid VerifyErrors and lazyvals mishaps; see SI-4717
95+
report.debuglog("refsym = " + level.refSym)
96+
report.error("forward reference not allowed from self constructor invocation",
97+
ctx.source.atSpan(level.refSpan))
98+
}
99+
}
100+
tree
101+
}
102+
103+
override def transformNew(tree: New)(using Context): New = {
104+
currentLevel.enterReference(tree.tpe.typeSymbol, tree.span)
105+
tree.tpe.dealias.foreachPart {
106+
case TermRef(_, s: Symbol) => currentLevel.enterReference(s, tree.span)
107+
case _ =>
108+
}
109+
tree
110+
}
111+
end ForwardDepChecks

‎compiler/src/dotty/tools/dotc/typer/CrossVersionChecks.scala

+210
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,210 @@
1+
package dotty.tools
2+
package dotc
3+
package transform
4+
5+
import core.*
6+
import Symbols.*, Types.*, Contexts.*, Flags.*, SymUtils.*, Decorators.*, reporting.*
7+
import util.SrcPos
8+
import config.{ScalaVersion, NoScalaVersion, Feature, ScalaRelease}
9+
import MegaPhase.MiniPhase
10+
import scala.util.{Failure, Success}
11+
import ast.tpd
12+
13+
class CrossVersionChecks extends MiniPhase:
14+
import tpd.*
15+
16+
def phaseName = "crossVersionChecks"
17+
18+
override def runsAfterGroupsOf: Set[String] = Set(FirstTransform.name)
19+
// We assume all type trees except TypeTree have been eliminated
20+
21+
// Note: if a symbol has both @deprecated and @migration annotations and both
22+
// warnings are enabled, only the first one checked here will be emitted.
23+
// I assume that's a consequence of some code trying to avoid noise by suppressing
24+
// warnings after the first, but I think it'd be better if we didn't have to
25+
// arbitrarily choose one as more important than the other.
26+
private def checkUndesiredProperties(sym: Symbol, pos: SrcPos)(using Context): Unit =
27+
checkDeprecated(sym, pos)
28+
checkExperimental(sym, pos)
29+
checkSinceAnnot(sym, pos)
30+
31+
val xMigrationValue = ctx.settings.Xmigration.value
32+
if xMigrationValue != NoScalaVersion then
33+
checkMigration(sym, pos, xMigrationValue)
34+
35+
36+
/** If @deprecated is present, and the point of reference is not enclosed
37+
* in either a deprecated member or a scala bridge method, issue a warning.
38+
*/
39+
private def checkDeprecated(sym: Symbol, pos: SrcPos)(using Context): Unit =
40+
41+
/** is the owner an enum or its companion and also the owner of sym */
42+
def isEnumOwner(owner: Symbol)(using Context) =
43+
// pre: sym is an enumcase
44+
if owner.isEnumClass then owner.companionClass eq sym.owner
45+
else if owner.is(ModuleClass) && owner.companionClass.isEnumClass then owner eq sym.owner
46+
else false
47+
48+
def isDeprecatedOrEnum(owner: Symbol)(using Context) =
49+
// pre: sym is an enumcase
50+
owner.isDeprecated
51+
|| isEnumOwner(owner)
52+
53+
/**Scan the chain of outer declaring scopes from the current context
54+
* a deprecation warning will be skipped if one the following holds
55+
* for a given declaring scope:
56+
* - the symbol associated with the scope is also deprecated.
57+
* - if and only if `sym` is an enum case, the scope is either
58+
* a module that declares `sym`, or the companion class of the
59+
* module that declares `sym`.
60+
*/
61+
def skipWarning(using Context) =
62+
ctx.owner.ownersIterator.exists(if sym.isEnumCase then isDeprecatedOrEnum else _.isDeprecated)
63+
64+
for annot <- sym.getAnnotation(defn.DeprecatedAnnot) do
65+
if !skipWarning then
66+
val msg = annot.argumentConstant(0).map(": " + _.stringValue).getOrElse("")
67+
val since = annot.argumentConstant(1).map(" since " + _.stringValue).getOrElse("")
68+
report.deprecationWarning(s"${sym.showLocated} is deprecated${since}${msg}", pos)
69+
70+
private def checkExperimental(sym: Symbol, pos: SrcPos)(using Context): Unit =
71+
if sym.isExperimental && !ctx.owner.isInExperimentalScope then
72+
Feature.checkExperimentalDef(sym, pos)
73+
74+
private def checkExperimentalSignature(sym: Symbol, pos: SrcPos)(using Context): Unit =
75+
class Checker extends TypeTraverser:
76+
def traverse(tp: Type): Unit =
77+
if tp.typeSymbol.isExperimental then
78+
Feature.checkExperimentalDef(tp.typeSymbol, pos)
79+
else
80+
traverseChildren(tp)
81+
if !sym.isInExperimentalScope then
82+
new Checker().traverse(sym.info)
83+
84+
private def checkExperimentalAnnots(sym: Symbol)(using Context): Unit =
85+
if !sym.isInExperimentalScope then
86+
for annot <- sym.annotations if annot.symbol.isExperimental do
87+
Feature.checkExperimentalDef(annot.symbol, annot.tree)
88+
89+
private def checkSinceAnnot(sym: Symbol, pos: SrcPos)(using Context): Unit =
90+
for
91+
annot <- sym.getAnnotation(defn.SinceAnnot)
92+
releaseName <- annot.argumentConstantString(0)
93+
do
94+
ScalaRelease.parse(releaseName) match
95+
case Some(release) if release > ctx.scalaRelease =>
96+
report.error(
97+
i"$sym was added in Scala release ${releaseName.show}, therefore it cannot be used in the code targeting Scala ${ctx.scalaRelease.show}",
98+
pos)
99+
case None =>
100+
report.error(i"$sym has an unparsable release name: '${releaseName}'", annot.tree.srcPos)
101+
case _ =>
102+
103+
private def checkSinceAnnotInSignature(sym: Symbol, pos: SrcPos)(using Context) =
104+
new TypeTraverser:
105+
def traverse(tp: Type) =
106+
if tp.typeSymbol.hasAnnotation(defn.SinceAnnot) then
107+
checkSinceAnnot(tp.typeSymbol, pos)
108+
else
109+
traverseChildren(tp)
110+
.traverse(sym.info)
111+
112+
/** If @migration is present (indicating that the symbol has changed semantics between versions),
113+
* emit a warning.
114+
*/
115+
private def checkMigration(sym: Symbol, pos: SrcPos, xMigrationValue: ScalaVersion)(using Context): Unit =
116+
for annot <- sym.getAnnotation(defn.MigrationAnnot) do
117+
val migrationVersion = ScalaVersion.parse(annot.argumentConstant(1).get.stringValue)
118+
migrationVersion match
119+
case Success(symVersion) if xMigrationValue < symVersion =>
120+
val msg = annot.argumentConstant(0).get.stringValue
121+
report.warning(SymbolChangedSemanticsInVersion(sym, symVersion, msg), pos)
122+
case Failure(ex) =>
123+
report.warning(SymbolHasUnparsableVersionNumber(sym, ex.getMessage), pos)
124+
case _ =>
125+
126+
/** Check that a deprecated val or def does not override a
127+
* concrete, non-deprecated method. If it does, then
128+
* deprecation is meaningless.
129+
*/
130+
private def checkDeprecatedOvers(tree: Tree)(using Context): Unit = {
131+
val symbol = tree.symbol
132+
if (symbol.isDeprecated) {
133+
val concrOvers =
134+
symbol.allOverriddenSymbols.filter(sym =>
135+
!sym.isDeprecated && !sym.is(Deferred))
136+
if (!concrOvers.isEmpty)
137+
report.deprecationWarning(
138+
symbol.toString + " overrides concrete, non-deprecated symbol(s):" +
139+
concrOvers.map(_.name).mkString(" ", ", ", ""), tree.srcPos)
140+
}
141+
}
142+
143+
/** Check that classes extending experimental classes or nested in experimental classes have the @experimental annotation. */
144+
private def checkExperimentalInheritance(cls: ClassSymbol)(using Context): Unit =
145+
if !cls.isAnonymousClass && !cls.hasAnnotation(defn.ExperimentalAnnot) then
146+
cls.info.parents.find(_.typeSymbol.isExperimental) match
147+
case Some(parent) =>
148+
report.error(em"extension of experimental ${parent.typeSymbol} must have @experimental annotation", cls.srcPos)
149+
case _ =>
150+
end checkExperimentalInheritance
151+
152+
override def transformValDef(tree: ValDef)(using Context): ValDef =
153+
checkDeprecatedOvers(tree)
154+
checkExperimentalAnnots(tree.symbol)
155+
checkExperimentalSignature(tree.symbol, tree)
156+
checkSinceAnnot(tree.symbol, tree.srcPos)
157+
checkSinceAnnotInSignature(tree.symbol, tree)
158+
tree
159+
160+
override def transformDefDef(tree: DefDef)(using Context): DefDef =
161+
checkDeprecatedOvers(tree)
162+
checkExperimentalAnnots(tree.symbol)
163+
checkExperimentalSignature(tree.symbol, tree)
164+
checkSinceAnnotInSignature(tree.symbol, tree)
165+
tree
166+
167+
override def transformTemplate(tree: Template)(using Context): Tree =
168+
val cls = ctx.owner.asClass
169+
checkExperimentalInheritance(cls)
170+
checkExperimentalAnnots(cls)
171+
tree
172+
173+
override def transformIdent(tree: Ident)(using Context): Ident = {
174+
checkUndesiredProperties(tree.symbol, tree.srcPos)
175+
tree
176+
}
177+
178+
override def transformSelect(tree: Select)(using Context): Select = {
179+
checkUndesiredProperties(tree.symbol, tree.srcPos)
180+
tree
181+
}
182+
183+
override def transformNew(tree: New)(using Context): New = {
184+
checkUndesiredProperties(tree.tpe.typeSymbol, tree.srcPos)
185+
tree
186+
}
187+
188+
override def transformTypeTree(tree: TypeTree)(using Context): TypeTree = {
189+
val tpe = tree.tpe
190+
tpe.foreachPart {
191+
case TypeRef(_, sym: Symbol) =>
192+
checkDeprecated(sym, tree.srcPos)
193+
checkExperimental(sym, tree.srcPos)
194+
checkSinceAnnot(sym, tree.srcPos)
195+
case TermRef(_, sym: Symbol) =>
196+
checkDeprecated(sym, tree.srcPos)
197+
checkExperimental(sym, tree.srcPos)
198+
checkSinceAnnot(sym, tree.srcPos)
199+
case _ =>
200+
}
201+
tree
202+
}
203+
204+
override def transformTypeDef(tree: TypeDef)(using Context): TypeDef = {
205+
checkExperimentalAnnots(tree.symbol)
206+
checkSinceAnnot(tree.symbol, tree.srcPos)
207+
tree
208+
}
209+
210+
end CrossVersionChecks

0 commit comments

Comments
 (0)