Skip to content

Commit 1f072dd

Browse files
committed
Revise SafeRefs checking to make it work for inlined defs
We want to check the call of an inline def for safe references, not the expansion. To do this, we need to move safe reference checking earlier, where inlining has not yet happened or the original call of an Inline node is still available. This means moving it to PostTyper.
1 parent b6e5747 commit 1f072dd

21 files changed

Lines changed: 151 additions & 125 deletions

File tree

compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -270,11 +270,6 @@ class CheckCaptures extends Recheck, SymTransformer:
270270

271271
def newRechecker()(using Context) = CaptureChecker(ctx)
272272

273-
override def runOn(units: List[CompilationUnit])(using runCtx: Context): List[CompilationUnit] =
274-
if Feature.ccEnabledSomewhere then
275-
SafeRefs.init()(using ctx.withPhase(thisPhase))
276-
super.runOn(units)
277-
278273
override protected def run(using Context): Unit =
279274
if Feature.ccEnabled then
280275
super.run
@@ -766,7 +761,6 @@ class CheckCaptures extends Recheck, SymTransformer:
766761
// charged for the prefix `p` in `p.x`.
767762
markFree(sym.info.captureSet, tree)
768763

769-
SafeRefs.checkSafe(tree, pt)
770764
mapResultRoots(super.recheckIdent(tree, pt), tree)
771765
}
772766

@@ -824,8 +818,6 @@ class CheckCaptures extends Recheck, SymTransformer:
824818
}
825819
case _ => denot
826820

827-
SafeRefs.checkSafe(tree, pt)
828-
829821
// Don't allow update methods to be called unless the qualifier captures
830822
// an exclusive reference.
831823
if tree.symbol.isUpdateMethod then
@@ -1099,9 +1091,6 @@ class CheckCaptures extends Recheck, SymTransformer:
10991091
case fun => fun.symbol
11001092
def methDescr = if meth.exists then i"$meth's type " else ""
11011093

1102-
if meth == defn.Any_asInstanceOf && Feature.safeEnabled then
1103-
report.error(em"Cannot use asInstanceOf in safe mode", tree.srcPos)
1104-
11051094
markFreeTypeArgs(tree.fun, meth, tree.args)
11061095

11071096
val funType = super.recheckTypeApply(tree, pt)
@@ -1289,10 +1278,6 @@ class CheckCaptures extends Recheck, SymTransformer:
12891278
override def seqLiteralElemProto(tree: SeqLiteral, pt: Type, declared: Type)(using Context) =
12901279
super.seqLiteralElemProto(tree, pt, declared).boxed
12911280

1292-
override def recheckNew(tree: New, pt: Type)(using Context): Type =
1293-
SafeRefs.checkSafe(tree, pt)
1294-
super.recheckNew(tree, pt)
1295-
12961281
/** Recheck val and var definitions:
12971282
* - disallow `any` in the type of mutable vars.
12981283
* - for externally visible definitions: check that their inferred type
@@ -1304,7 +1289,6 @@ class CheckCaptures extends Recheck, SymTransformer:
13041289
val savedEnv = curEnv
13051290
val runInConstructor = !sym.isOneOf(Param | ParamAccessor | Lazy | NonMember)
13061291
try
1307-
SafeRefs.checkSafeAnnots(sym)
13081292
if sym.is(Mutable) then
13091293
if !sym.hasAnnotation(defn.UncheckedCapturesAnnot) then
13101294
val addendum = setup.capturedBy.get(sym) match
@@ -1403,13 +1387,6 @@ class CheckCaptures extends Recheck, SymTransformer:
14031387
if ac.isEmpty then ctx
14041388
else ctx.withProperty(CaptureSet.AssumedContains, Some(ac))
14051389

1406-
SafeRefs.checkSafeAnnots(sym)
1407-
for params <- tree.paramss; param <- params do
1408-
SafeRefs.checkSafeAnnots(param.symbol)
1409-
param match
1410-
case param: ValDef => SafeRefs.checkSafeAnnotsInType(param.tpt)
1411-
case param: TypeDef => SafeRefs.checkSafeAnnotsInType(param.rhs)
1412-
14131390
checkNoUnboxedReaches(tree)
14141391

14151392
try checkInferredResult(super.recheckDefDef(tree, sym)(using bodyCtx), tree)
@@ -1640,7 +1617,6 @@ class CheckCaptures extends Recheck, SymTransformer:
16401617
markFreeTypeArgs(tpt, fn.typeSymbol, args.map(TypeTree(_)))
16411618
case _ =>
16421619

1643-
SafeRefs.checkSafeAnnots(cls)
16441620
checkFieldCaptures(cls)
16451621

16461622
super.recheckClassDef(tree, impl, cls)
@@ -1682,10 +1658,6 @@ class CheckCaptures extends Recheck, SymTransformer:
16821658
tree.srcPos)
16831659
tp
16841660

1685-
override def recheckTypeTree(tree: TypeTree)(using Context): Type =
1686-
SafeRefs.checkSafeAnnotsInType(tree)
1687-
super.recheckTypeTree(tree)
1688-
16891661
/* Currently not needed, since capture checking takes place after ElimByName.
16901662
* Keep around in case we need to get back to it
16911663
def recheckByNameArg(tree: Tree, pt: Type)(using Context): Type =

compiler/src/dotty/tools/dotc/cc/SafeRefs.scala

Lines changed: 62 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,14 @@ import ast.tpd.*
1414
import SymDenotations.*
1515
import Flags.*
1616
import Types.*
17-
import config.Feature
1817
import config.Printers.capt
19-
import typer.ProtoTypes.SelectionProto
2018

2119
/** Check whether references from safe mode should be allowed */
2220
object SafeRefs {
2321

2422
val assumedSafePackages = List(
2523
"scala", "scala.runtime", "scala.collection.immutable", "scala.compiletime.ops",
26-
"scala.math", "scala.util", "java.math", "java.time",
24+
"scala.math", "scala.util", "scala.caps", "java.math", "java.time",
2725
"java.util.function", "java.util.regex", "java.util.stream"
2826
)
2927

@@ -56,7 +54,7 @@ object SafeRefs {
5654
* Once we have an updated ccException in the bootstrap compiler, we could add annotations
5755
* to library classes manually, as long as these library classes are capture checked.
5856
*/
59-
def init()(using Context): Unit =
57+
def init()(using Context): Unit = {
6058
assumeSafe("scala.Predef", except = List("print", "println", "printf"))
6159
assumeSafe("scala.runtime.coverage.Invoker")
6260
assumeSafe("scala.reflect.ClassTag")
@@ -172,21 +170,73 @@ object SafeRefs {
172170
rejectSafe("scala.runtime.LazyFloat")
173171
rejectSafe("scala.runtime.LazyDouble")
174172
rejectSafe("scala.runtime.LazyUnit")
173+
}
175174

176175
private def fail(sym: Symbol, reason: String, pos: SrcPos)(using Context) =
177176
report.error(em"Cannot refer to ${sym.sanitizedDescription}${sym.showExtendedLocation} from safe code since $reason", pos)
178177
false
179178

180179
private def checkNotRejected(sym: Symbol, pos: SrcPos)(using Context): Boolean =
181-
if !sym.exists then true
182-
else sym.getAnnotation(defn.RejectSafeAnnot) match
180+
!sym.exists || sym.is(Package) || sym.getAnnotation(defn.RejectSafeAnnot).match
183181
case Some(annot) =>
184182
val message = annot.argumentConstantString(0).getOrElse("")
185-
fail(sym, if message.nonEmpty then message else i"it is tagged @rejectSafe", pos)
183+
fail(sym, if message.nonEmpty then message else i"it is tagged @rejectSafe", pos)
186184
case _ =>
187-
sym.owner.is(Package) || checkNotRejected(sym.owner, pos)
185+
checkNotRejected(sym.owner, pos)
186+
187+
/** Check that all nodes of given tree for the following conditions.
188+
* - No reference to a symbol under a @rejectSafe annotation
189+
* - All references to static symbols are assumed safe: This means
190+
* they have been compiled in safe mode, or have an @assumeSafe
191+
* annotation or are owned by a symbol with an @assumeSafe annotation.
192+
* - No reference to a user-defined annotation which is marked @rejectSafe
193+
*/
194+
object checker extends TreeTraverser:
195+
private var checkTypes = false
196+
def traverse(tree: Tree)(using Context) =
197+
val sym = tree.symbol
198+
tree match
199+
case tree: Ident =>
200+
checkNotRejected(sym, tree.srcPos)
201+
val isStatic = tree.tpe match
202+
case NamedType(prefix, _) =>
203+
prefix.dealias match
204+
case prefix: ThisType => prefix.cls.isStatic
205+
case prefix: TermRef => prefix.symbol.isStatic
206+
case _ => sym.isStatic
207+
case _ => sym.isStatic
208+
// if sym is not static it is local, a parameter, or comes from another symbol,
209+
// which has been checked
210+
if isStatic && (checkTypes || sym.isTerm) then
211+
checkSafe(sym, tree)
212+
case tree: Select =>
213+
checkNotRejected(sym, tree.srcPos)
214+
if sym.isStatic && (checkTypes || sym.isTerm)
215+
then checkSafe(sym, tree)
216+
else traverseChildren(tree)
217+
case New(tpt) =>
218+
val saved = checkTypes
219+
checkTypes = true
220+
try traverse(tpt)
221+
finally checkTypes = saved
222+
case Inlined(call, _, _) =>
223+
traverse(call)
224+
case tree: MemberDef if !sym.is(Synthetic) =>
225+
for ann <- sym.annotations do
226+
checkSafeAnnot(ann, sym.srcPos)
227+
traverseChildren(tree)
228+
case tree: TypeApply if sym == defn.Any_asInstanceOf =>
229+
report.error(em"Cannot use asInstanceOf in safe mode", tree.srcPos)
230+
case Annotated(arg, annot) =>
231+
checkNotRejected(annot.symbol, annot.srcPos.orElse(tree.srcPos))
232+
traverseChildren(arg)
233+
case tree: Import =>
234+
// skip imports, we want to be able to wildcard import from an unsafe
235+
// object as long as all used members are @assumeSafe
236+
case _ =>
237+
traverseChildren(tree)
188238

189-
def checkSafe(tree: Tree, pt: Type)(using Context): Unit = {
239+
def checkSafe(sym: Symbol, tree: Tree)(using Context): Unit = {
190240

191241
def isSafe(sym: Symbol): Boolean =
192242
if !sym.exists then false
@@ -196,62 +246,15 @@ object SafeRefs {
196246
sym.hasAnnotation(defn.AssumeSafeAnnot)
197247
|| isSafe(if sym.is(ModuleVal) then sym.moduleClass else sym.owner)
198248

199-
val sym = tree match
200-
case tree: New => tree.tpt.tpe.classSymbol
201-
case tree: RefTree => tree.symbol
202-
203-
def checkLater =
204-
sym.isTerm && !sym.is(Method) && pt.match
205-
case pt: PathSelectionProto => pt.selector.isStatic
206-
case _: SelectionProto => true
207-
case _ => false
208-
209-
def isStatic = tree match
210-
case tree: Ident =>
211-
// Idents might refer to inherited symbols of static objects.
212-
// in this case we need to check whether the prefix is static
213-
// For Selects this is not an issue since we have already checked
214-
// the qualifier for safety. safemode-pkg-inherit.scala is a test case.
215-
tree.tpe match
216-
case NamedType(prefix, _) =>
217-
prefix.dealias match
218-
case prefix: ThisType => prefix.cls.isStatic
219-
case prefix: TermRef => prefix.symbol.isStatic
220-
case _ => sym.isStatic
221-
case _ => sym.isStatic
222-
case _ => sym.isStatic
223-
224-
if Feature.safeEnabled
225-
&& sym.exists
226-
&& !sym.is(Package)
227-
&& checkNotRejected(sym, tree.srcPos)
228-
&& !checkLater
229-
&& isStatic // if it's not static it is local, a parameter, or comes from another symbol,
230-
// which has been checked
231-
&& !isSafe(sym)
232-
then
249+
if sym.exists && !sym.is(Package) && !isSafe(sym) then
233250
fail(sym, "it is neither compiled in safe mode nor tagged with @assumedSafe", tree.srcPos)
234251
else
235-
capt.println(i"checked safe $tree, $sym, $checkLater")
252+
capt.println(i"checked safe $tree, $sym")
236253
}
237254

238255
private def checkSafeAnnot(ann: Annotation, pos: SrcPos)(using Context): Unit =
239256
val span = ann.tree.span
240257
// Skip compiler inserted annotations that have no or zero extent span.
241258
if !span.exists || span.isZeroExtent then return
242-
var errpos = ann.tree.srcPos
243-
if !pos.sourcePos.exists then errpos = pos
244-
checkNotRejected(ann.symbol, errpos)
245-
246-
def checkSafeAnnots(sym: Symbol)(using Context): Unit =
247-
if Feature.safeEnabled && !sym.is(Synthetic) then
248-
for ann <- sym.annotations do
249-
checkSafeAnnot(ann, sym.srcPos)
250-
251-
def checkSafeAnnotsInType(tree: Tree)(using Context): Unit =
252-
def checkAnnotatedType(tp: Type) = tp match
253-
case AnnotatedType(tp, ann) => checkSafeAnnot(ann, tree.srcPos)
254-
case _ =>
255-
if Feature.safeEnabled then
256-
tree.tpe.foreachPart(checkAnnotatedType(_))
259+
checkNotRejected(ann.symbol, ann.tree.srcPos)
257260
}

compiler/src/dotty/tools/dotc/config/Feature.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ object Feature:
212212
report.error(experimentalUseSite(which) + note, srcPos)
213213

214214
private def ccException(sym: Symbol)(using Context): Boolean =
215-
ccEnabled && (defn.ccExperimental.contains(sym)
215+
ccEnabledSomewhere && (defn.ccExperimental.contains(sym)
216216
|| sym.exists && defn.ccExperimental.contains(sym.owner))
217217

218218
def checkExperimentalDef(sym: Symbol, srcPos: SrcPos)(using Context) =

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,19 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
9696
def newTransformer(using Context): Transformer =
9797
new PostTyperTransformer
9898

99+
override def runOn(units: List[CompilationUnit])(using runCtx: Context): List[CompilationUnit] =
100+
if Feature.ccEnabledSomewhere then
101+
SafeRefs.init()(using ctx.withPhase(thisPhase))
102+
super.runOn(units)
103+
104+
override def run(using Context): Unit =
105+
val unit = ctx.compilationUnit
106+
if Feature.safeEnabled then
107+
// Check safe refs before PostTyper's run since that way Inline calls have not
108+
// yet been replaced with InlineCallTraces.
109+
SafeRefs.checker.traverse(unit.tpdTree)
110+
super.run
111+
99112
/**
100113
* Used to check that `changesParents` is called after `initContext`.
101114
*

compiler/src/dotty/tools/dotc/util/SourcePosition.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,3 +120,4 @@ trait SrcPos:
120120
def endPos(using ctx: Context): SourcePosition = sourcePos.endPos
121121
def focus(using ctx: Context): SourcePosition = sourcePos.focus
122122
def line(using ctx: Context): Int = sourcePos.line
123+
def orElse(other: SrcPos): SrcPos = if span.exists then this else other
Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
1-
-- Error: tests/neg-custom-args/captures/i25759.scala:4:14 -------------------------------------------------------------
1+
-- Error: tests/neg-custom-args/captures/i25759.scala:4:35 -------------------------------------------------------------
22
4 | val q = new java.util.concurrent.ConcurrentLinkedQueue[String]() // error
3-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
44
|Cannot refer to class ConcurrentLinkedQueue in package java.util.concurrent from safe code since it is neither compiled in safe mode nor tagged with @assumedSafe
5-
-- Error: tests/neg-custom-args/captures/i25759.scala:9:15 -------------------------------------------------------------
5+
-- Error: tests/neg-custom-args/captures/i25759.scala:9:25 -------------------------------------------------------------
66
9 | val xs = new java.util.ArrayList[String]() // error
7-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
7+
| ^^^^^^^^^^^^^^^^^^^
88
|Cannot refer to class ArrayList in package java.util from safe code since it is neither compiled in safe mode nor tagged with @assumedSafe
9-
-- Error: tests/neg-custom-args/captures/i25759.scala:14:14 ------------------------------------------------------------
9+
-- Error: tests/neg-custom-args/captures/i25759.scala:14:24 ------------------------------------------------------------
1010
14 | val m = new java.util.HashMap[String, String]() // error
11-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
11+
| ^^^^^^^^^^^^^^^^^
1212
|Cannot refer to class HashMap in package java.util from safe code since it is neither compiled in safe mode nor tagged with @assumedSafe
13-
-- Error: tests/neg-custom-args/captures/i25759.scala:19:15 ------------------------------------------------------------
13+
-- Error: tests/neg-custom-args/captures/i25759.scala:19:25 ------------------------------------------------------------
1414
19 | val dq = new java.util.ArrayDeque[String]() // error
15-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
15+
| ^^^^^^^^^^^^^^^^^^^^
1616
|Cannot refer to class ArrayDeque in package java.util from safe code since it is neither compiled in safe mode nor tagged with @assumedSafe

tests/neg-custom-args/captures/safemode-1.check

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@
1010
16 | Unsafe.foo() // error
1111
| ^^^^^^^^^^
1212
|Cannot refer to method foo in object Unsafe from safe code since it is neither compiled in safe mode nor tagged with @assumedSafe
13-
-- Error: tests/neg-custom-args/captures/safemode-1/safe_2.scala:18:8 --------------------------------------------------
13+
-- Error: tests/neg-custom-args/captures/safemode-1/safe_2.scala:18:16 -------------------------------------------------
1414
18 | scala.Console.out.println("!") // error
15-
| ^^^^^^^^^^^^^
15+
| ^^^^^^^^^^^^^^^^^
1616
| Cannot refer to object Console in package scala from safe code since it is tagged @rejectSafe
1717
-- Error: tests/neg-custom-args/captures/safemode-1/safe_2.scala:22:6 --------------------------------------------------
1818
22 | x.a.foo() // error

tests/neg-custom-args/captures/safemode-2.check

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
-- Error: tests/neg-custom-args/captures/safemode-2.scala:8:15 ---------------------------------------------------------
1+
-- Error: tests/neg-custom-args/captures/safemode-2.scala:8:22 ---------------------------------------------------------
22
8 | val x = caps.unsafe.unsafeErasedValue[String] // error
3-
| ^^^^^^^^^^^
3+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
44
| Cannot refer to object unsafe in package scala.caps from safe code since it is unavailable in safe mode
55
-- Error: tests/neg-custom-args/captures/safemode-2.scala:10:2 ---------------------------------------------------------
66
10 | @caps.unsafe.untrackedCaptures var y = 1 // error
Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
1-
-- Error: tests/neg-custom-args/captures/safemode-3.scala:10:21 --------------------------------------------------------
2-
10 | case x: List[Int @unchecked] => x.head // error
3-
| ^^^^^^^^^^
4-
| Cannot refer to class unchecked in package scala from safe code since it is tagged @rejectSafe
5-
-- Error: tests/neg-custom-args/captures/safemode-3.scala:18:32 --------------------------------------------------------
6-
18 | def h(x: Any) = x.asInstanceOf[String] // error
7-
| ^^^^^^^^^^^^^^^^^^^^^^
8-
| Cannot use asInstanceOf in safe mode
1+
-- Error: tests/neg-custom-args/captures/safemode-3.scala:7:21 ---------------------------------------------------------
2+
7 | case x: List[Int @unchecked] => x.head // error
3+
| ^^^^^^^^^^
4+
| Cannot refer to class unchecked in package scala from safe code since it is tagged @rejectSafe

tests/neg-custom-args/captures/safemode-3.scala

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
package test
22
import language.experimental.safe
3-
import caps.unsafe.untrackedCaptures
4-
import scala.annotation.unchecked.{uncheckedCaptures, uncheckedVariance}
5-
63

74
object Test:
85

@@ -15,4 +12,3 @@ object Test:
1512
case x: String => x.length
1613
case _ => 0
1714

18-
def h(x: Any) = x.asInstanceOf[String] // error

0 commit comments

Comments
 (0)