Skip to content

Commit 4b71de8

Browse files
committed
Make lazy vals thread-safe by default
Previously this required using `@volatile` but that would not be consistent with Scala 2. The old behavior of non-volatile lazy vals can be recovered by using the newly-introduced `@threadUnsafe`.
1 parent 69630bf commit 4b71de8

File tree

21 files changed

+250
-90
lines changed

21 files changed

+250
-90
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -863,6 +863,8 @@ class Definitions {
863863
def TASTYLongSignatureAnnot(implicit ctx: Context): ClassSymbol = TASTYLongSignatureAnnotType.symbol.asClass
864864
lazy val TailrecAnnotType: TypeRef = ctx.requiredClassRef("scala.annotation.tailrec")
865865
def TailrecAnnot(implicit ctx: Context): ClassSymbol = TailrecAnnotType.symbol.asClass
866+
lazy val ThreadUnsafeAnnotType: TypeRef = ctx.requiredClassRef("scala.annotation.threadUnsafe")
867+
def ThreadUnsafeAnnot(implicit ctx: Context): ClassSymbol = ThreadUnsafeAnnotType.symbol.asClass
866868
lazy val TransientParamAnnotType: TypeRef = ctx.requiredClassRef("scala.annotation.constructorOnly")
867869
def TransientParamAnnot(implicit ctx: Context): ClassSymbol = TransientParamAnnotType.symbol.asClass
868870
lazy val CompileTimeOnlyAnnotType: TypeRef = ctx.requiredClassRef("scala.annotation.compileTimeOnly")

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

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ object AugmentScala2Traits {
2525
* These symbols would have been added between Unpickling and Mixin in the Scala2 pipeline.
2626
*
2727
* Specifically, we:
28-
* - Mark all lazy val fields as @volatile to get the proper Scala 2 semantics.
2928
* - Add trait setters for vals defined in traits.
3029
* - Expand the names of all private getters and setters as well as super accessors in the trait and make
3130
* not-private.
@@ -57,14 +56,9 @@ class AugmentScala2Traits extends MiniPhase with IdentityDenotTransformer { this
5756
info = MethodType(getter.info.resultType :: Nil, defn.UnitType))
5857

5958
for (sym <- mixin.info.decls) {
60-
if (sym.isGetter)
61-
if (sym.is(Lazy)) {
62-
if (!sym.hasAnnotation(defn.VolatileAnnot))
63-
sym.addAnnotation(Annotation(defn.VolatileAnnot, Nil))
64-
}
65-
else if (!sym.is(Deferred) && !sym.setter.exists &&
59+
if (sym.isGetter && !sym.is(LazyOrDeferred) && !sym.setter.exists &&
6660
!sym.info.resultType.isInstanceOf[ConstantType])
67-
traitSetter(sym.asTerm).enteredAfter(thisPhase)
61+
traitSetter(sym.asTerm).enteredAfter(thisPhase)
6862
if ((sym.is(PrivateAccessor) && !sym.name.is(ExpandedName) &&
6963
(sym.isGetter || sym.isSetter)) // strangely, Scala 2 fields are also methods that have Accessor set.
7064
|| sym.isSuperAccessor) // scala2 superaccessors are pickled as private, but are compiled as public expanded

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

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -74,18 +74,17 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer {
7474
else {
7575
val isField = sym.owner.isClass
7676
if (isField) {
77-
if (sym.isVolatile ||
78-
(sym.is(Module)/* || ctx.scala2Mode*/) &&
79-
// TODO assume @volatile once LazyVals uses static helper constructs instead of
80-
// ones in the companion object.
81-
!sym.is(Synthetic))
82-
// module class is user-defined.
83-
// Should be threadsafe, to mimic safety guaranteed by global object
84-
transformMemberDefVolatile(tree)
85-
else if (sym.is(Module)) // synthetic module
77+
if (sym.is(SyntheticModule))
8678
transformSyntheticModule(tree)
79+
else if (sym.isThreadUnsafe) {
80+
if (sym.is(Module)) {
81+
ctx.error(em"@threadUnsafe is only supported on lazy vals", sym.sourcePos)
82+
transformMemberDefThreadSafe(tree)
83+
}
84+
transformMemberDefThreadUnsafe(tree)
85+
}
8786
else
88-
transformMemberDefNonVolatile(tree)
87+
transformMemberDefThreadSafe(tree)
8988
}
9089
else transformLocalDef(tree)
9190
}
@@ -196,7 +195,7 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer {
196195
ref(field).becomes(nullLiteral)
197196
}
198197

199-
/** Create non-threadsafe lazy accessor equivalent to such code
198+
/** Create thread-unsafe lazy accessor equivalent to such code
200199
* ```
201200
* def methodSymbol() = {
202201
* if (!flag) {
@@ -208,7 +207,7 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer {
208207
* }
209208
* ```
210209
*/
211-
def mkNonThreadSafeDef(sym: Symbol, flag: Symbol, target: Symbol, rhs: Tree)(implicit ctx: Context): DefDef = {
210+
def mkThreadUnsafeDef(sym: Symbol, flag: Symbol, target: Symbol, rhs: Tree)(implicit ctx: Context): DefDef = {
212211
val targetRef = ref(target)
213212
val flagRef = ref(flag)
214213
val stats = targetRef.becomes(rhs) :: flagRef.becomes(Literal(Constant(true))) :: nullOut(nullableFor(sym))
@@ -220,7 +219,7 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer {
220219
DefDef(sym.asTerm, Block(List(init), targetRef.ensureApplied))
221220
}
222221

223-
/** Create non-threadsafe lazy accessor for not-nullable types equivalent to such code
222+
/** Create thread-unsafe lazy accessor for not-nullable types equivalent to such code
224223
* ```
225224
* def methodSymbol() = {
226225
* if (target eq null) {
@@ -231,7 +230,7 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer {
231230
* }
232231
* ```
233232
*/
234-
def mkDefNonThreadSafeNonNullable(sym: Symbol, target: Symbol, rhs: Tree)(implicit ctx: Context): DefDef = {
233+
def mkDefThreadUnsafeNonNullable(sym: Symbol, target: Symbol, rhs: Tree)(implicit ctx: Context): DefDef = {
235234
val targetRef = ref(target)
236235
val stats = targetRef.becomes(rhs) :: nullOut(nullableFor(sym))
237236
val init = If(
@@ -242,7 +241,7 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer {
242241
DefDef(sym.asTerm, Block(List(init), targetRef.ensureApplied))
243242
}
244243

245-
def transformMemberDefNonVolatile(x: ValOrDefDef)(implicit ctx: Context): Thicket = {
244+
def transformMemberDefThreadUnsafe(x: ValOrDefDef)(implicit ctx: Context): Thicket = {
246245
val claz = x.symbol.owner.asClass
247246
val tpe = x.tpe.widen.resultType.widen
248247
assert(!(x.symbol is Mutable))
@@ -255,13 +254,13 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer {
255254
val containerTree = ValDef(containerSymbol, defaultValue(tpe))
256255
if (x.tpe.isNotNull && tpe <:< defn.ObjectType) {
257256
// can use 'null' value instead of flag
258-
Thicket(containerTree, mkDefNonThreadSafeNonNullable(x.symbol, containerSymbol, x.rhs))
257+
Thicket(containerTree, mkDefThreadUnsafeNonNullable(x.symbol, containerSymbol, x.rhs))
259258
}
260259
else {
261260
val flagName = LazyBitMapName.fresh(x.name.asTermName)
262261
val flagSymbol = ctx.newSymbol(x.symbol.owner, flagName, containerFlags | Private, defn.BooleanType).enteredAfter(this)
263262
val flag = ValDef(flagSymbol, Literal(Constant(false)))
264-
Thicket(containerTree, flag, mkNonThreadSafeDef(x.symbol, flagSymbol, containerSymbol, x.rhs))
263+
Thicket(containerTree, flag, mkThreadUnsafeDef(x.symbol, flagSymbol, containerSymbol, x.rhs))
265264
}
266265
}
267266

@@ -365,7 +364,7 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer {
365364
DefDef(methodSymbol, loop)
366365
}
367366

368-
def transformMemberDefVolatile(x: ValOrDefDef)(implicit ctx: Context): Thicket = {
367+
def transformMemberDefThreadSafe(x: ValOrDefDef)(implicit ctx: Context): Thicket = {
369368
assert(!(x.symbol is Mutable))
370369

371370
val tpe = x.tpe.widen.resultType.widen

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ class SymUtils(val self: Symbol) extends AnyVal {
5252
def isTypeTestOrCast(implicit ctx: Context): Boolean =
5353
isTypeCast || isTypeTest
5454

55+
def isThreadUnsafe(implicit ctx: Context): Boolean = self.hasAnnotation(defn.ThreadUnsafeAnnot)
56+
5557
def isVolatile(implicit ctx: Context): Boolean = self.hasAnnotation(defn.VolatileAnnot)
5658

5759
def isAnyOverride(implicit ctx: Context): Boolean = self.is(Override) || self.is(AbsOverride)

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

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1472,20 +1472,10 @@ class Typer extends Namer
14721472
val vdef1 = assignType(cpy.ValDef(vdef)(name, tpt1, rhs1), sym)
14731473
if (sym.is(Inline, butNot = DeferredOrTermParamOrAccessor))
14741474
checkInlineConformant(rhs1, isFinal = sym.is(Final), em"right-hand side of inline $sym")
1475-
patchIfLazy(vdef1)
14761475
patchFinalVals(vdef1)
14771476
vdef1.setDefTree
14781477
}
14791478

1480-
/** Add a @volitile to lazy vals when rewriting from Scala2 */
1481-
private def patchIfLazy(vdef: ValDef)(implicit ctx: Context): Unit = {
1482-
val sym = vdef.symbol
1483-
if (sym.is(Lazy, butNot = Deferred | Module | Synthetic) && !sym.isVolatile &&
1484-
ctx.scala2Mode && ctx.settings.rewrite.value.isDefined &&
1485-
!ctx.isAfterTyper)
1486-
patch(Span(toUntyped(vdef).span.start), "@volatile ")
1487-
}
1488-
14891479
/** Adds inline to final vals with idempotent rhs
14901480
*
14911481
* duplicating scalac behavior: for final vals that have rhs as constant, we do not create a field

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

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -524,22 +524,35 @@ class TestBCode extends DottyBytecodeTest {
524524
}
525525

526526
/** Test that the size of lazy field accesors is under a certain threshold
527-
*
528-
* - Changed from 19 to 14
529527
*/
530528
@Test def lazyFields = {
531-
val source =
529+
val sourceUnsafe =
530+
"""import scala.annotation.threadUnsafe
531+
|
532+
|class Test {
533+
| @threadUnsafe lazy val test = 1
534+
|}
535+
""".stripMargin
536+
537+
val sourceSafe =
532538
"""class Test {
533539
| lazy val test = 1
534540
|}
535541
""".stripMargin
536542

537-
checkBCode(source) { dir =>
543+
checkBCode(sourceUnsafe) { dir =>
538544
val clsIn = dir.lookupName("Test.class", directory = false).input
539545
val clsNode = loadClassNode(clsIn)
540546
val method = getMethod(clsNode, "test")
541547
assertEquals(14, instructionsFromMethod(method).size)
542548
}
549+
550+
checkBCode(sourceSafe) { dir =>
551+
val clsIn = dir.lookupName("Test.class", directory = false).input
552+
val clsNode = loadClassNode(clsIn)
553+
val method = getMethod(clsNode, "test")
554+
assertEquals(94, instructionsFromMethod(method).size)
555+
}
543556
}
544557

545558
/* Test that objects compile to *final* classes. */

docs/docs/reference/changed-features/lazy-vals-spec.md renamed to docs/docs/reference/changed-features/lazy-vals-init.md

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
---
22
layout: doc-page
3-
title: Changed: Lazy Vals - More Details
3+
title: Lazy Vals initialization
44
---
55

66
Dotty implements [Version 6](https://docs.scala-lang.org/sips/improved-lazy-val-initialization.html#version-6---no-synchronization-on-this-and-concurrent-initialization-of-fields)
@@ -19,7 +19,7 @@ Given a lazy field of the form:
1919

2020
```scala
2121
class Foo {
22-
@volatile lazy val bar = <RHS>
22+
lazy val bar = <RHS>
2323
}
2424
```
2525

@@ -69,9 +69,8 @@ val. This id grows with the number of volatile lazy vals defined in the class.
6969

7070
## Note on recursive lazy vals
7171

72-
Ideally recursive lazy vals should be flagged as an error. The current behavior for `@volatile`
73-
recursive lazy vals is undefined (initialization may result in a deadlock). Non `@volatile` lazy
74-
vals behave as in Scala 2.
72+
Ideally recursive lazy vals should be flagged as an error. The current behavior for
73+
recursive lazy vals is undefined (initialization may result in a deadlock).
7574

7675
## Reference
7776

docs/docs/reference/changed-features/lazy-vals.md

Lines changed: 0 additions & 27 deletions
This file was deleted.
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
layout: doc-page
3+
title: threadUnsafe annotation
4+
---
5+
6+
A new annotation `@threadUnsafe` can be used on a field which defines a lazy
7+
val. When this annotation is used, the initialization of the lazy val will use a
8+
faster mechanism which is not thread-safe.
9+
10+
### Example
11+
12+
```scala
13+
import scala.annotation.threadUnsafe
14+
15+
class Hello {
16+
@threadUnsafe lazy val x: Int = 1
17+
}
18+
```

docs/sidebar.yml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,10 @@ sidebar:
9191
url: docs/reference/other-new-features/kind-polymorphism.html
9292
- title: Tupled Function
9393
url: docs/reference/other-new-features/tupled-function.html
94+
- title: threadUnsafe Annotation
95+
url: docs/reference/other-new-features/threadUnsafe-annotation.html
9496
- title: Other Changed Features
9597
subsection:
96-
- title: Volatile Lazy Vals
97-
url: docs/reference/changed-features/lazy-vals.html
9898
- title: Structural Types
9999
url: docs/reference/changed-features/structural-types.html
100100
- title: Operators
@@ -119,6 +119,8 @@ sidebar:
119119
url: docs/reference/changed-features/eta-expansion.html
120120
- title: Compiler Plugins
121121
url: docs/reference/changed-features/compiler-plugins.html
122+
- title: Lazy Vals initialization
123+
url: docs/reference/changed-features/lazy-vals-init.html
122124
- title: Dropped Features
123125
subsection:
124126
- title: DelayedInit
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package scala.annotation
2+
3+
/** This annotation can only be used on a field which defines a lazy val.
4+
* When this annotation is used, the initialization of the lazy val will use a
5+
* faster mechanism which is not thread-safe.
6+
*/
7+
final class threadUnsafe extends StaticAnnotation

tests/pos/lazyvals.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ trait Iterator {
1010
val leading = new Leading
1111

1212
class Trailing {
13-
@volatile lazy val it = leading.finish()
13+
lazy val it = leading.finish()
1414
}
1515
val trailing = new Trailing
1616
(leading, trailing)

tests/pos/scala2traits/dotty-subclass.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ class Sub extends T
33

44
trait DT {
55

6-
@volatile lazy val dx = 2
6+
lazy val dx = 2
77

88
}
99

tests/run/Lazies1.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
object T{ @volatile lazy val s = null}
1+
object T{ lazy val s = null}
22
object Test{
33
def main(args: Array[String]): Unit = {
44
T.s

tests/run/Lazies2.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
class T{ @volatile lazy val s = null}
2-
object T{ @volatile lazy val s = null}
1+
class T{ lazy val s = null}
2+
object T{ lazy val s = null}
33
object Test{
44
def main(args: Array[String]): Unit = {
55
T.s

tests/run/i1692.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ class LazyNullable(a: => Int) {
88
lazy val l1 = b // null out b
99

1010
private[this] val c = "C"
11-
@volatile lazy val l2 = c // null out c
11+
lazy val l2 = c // null out c
1212

1313
private[this] val d = "D"
1414
lazy val l3 = d + d // null out d (Scalac require single use?)

0 commit comments

Comments
 (0)