Skip to content

Commit 673842f

Browse files
committed
Fixed selection in InterceptedMethods that caused a data race
Also, added comments to the tpd select methods that explain how the data race could arise and how to avoid it.
1 parent a2a12a2 commit 673842f

File tree

2 files changed

+42
-5
lines changed

2 files changed

+42
-5
lines changed

src/dotty/tools/dotc/ast/tpd.scala

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -588,63 +588,97 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
588588
tree
589589
}
590590

591+
/** A select node with the given selector name and a computed type */
591592
def select(name: Name)(implicit ctx: Context): Select =
592593
Select(tree, name)
593594

595+
/** A select node with the given type */
594596
def select(tp: NamedType)(implicit ctx: Context): Select =
595597
untpd.Select(tree, tp.name).withType(tp)
596598

599+
/** A select node that selects the given symbol. Note: Need to make sure this
600+
* is in fact the symbol you would get when you select with the symbol's name,
601+
* otherwise a data race may occur which would be flagged by -Yno-double-bindings.
602+
*/
597603
def select(sym: Symbol)(implicit ctx: Context): Select =
598604
untpd.Select(tree, sym.name).withType(
599605
TermRef.withSigAndDenot(tree.tpe, sym.name.asTermName, sym.signature, sym.denot.asSeenFrom(tree.tpe)))
600606

601-
def selectWithSig(name: Name, sig: Signature)(implicit ctx: Context) =
607+
/** A select node with the given selector name and signature and a computed type */
608+
def selectWithSig(name: Name, sig: Signature)(implicit ctx: Context): Tree =
602609
untpd.SelectWithSig(tree, name, sig)
603610
.withType(TermRef.withSig(tree.tpe, name.asTermName, sig))
604611

612+
/** A select node with selector name and signature taken from `sym`.
613+
* Note: Use this method instead of select(sym) if the referenced symbol
614+
* might be overridden in the type of the qualifier prefix. See note
615+
* on select(sym: Symbol).
616+
*/
617+
def selectWithSig(sym: Symbol)(implicit ctx: Context): Tree =
618+
selectWithSig(sym.name, sym.signature)
619+
620+
/** A unary apply node with given argument: `tree(arg)` */
605621
def appliedTo(arg: Tree)(implicit ctx: Context): Tree =
606622
appliedToArgs(arg :: Nil)
607623

624+
/** An apply node with given arguments: `tree(arg, args0, ..., argsN)` */
608625
def appliedTo(arg: Tree, args: Tree*)(implicit ctx: Context): Tree =
609626
appliedToArgs(arg :: args.toList)
610627

628+
/** An apply node with given argument list `tree(args(0), ..., args(args.length - 1))` */
611629
def appliedToArgs(args: List[Tree])(implicit ctx: Context): Apply =
612630
Apply(tree, args)
613631

632+
/** The current tree applied to given argument lists:
633+
* `tree (argss(0)) ... (argss(argss.length -1))`
634+
*/
614635
def appliedToArgss(argss: List[List[Tree]])(implicit ctx: Context): Tree =
615636
((tree: Tree) /: argss)(Apply(_, _))
616637

638+
/** The current tree applied to (): `tree()` */
617639
def appliedToNone(implicit ctx: Context): Apply = appliedToArgs(Nil)
618640

641+
/** The current tree applied to given type argument: `tree[targ]` */
619642
def appliedToType(targ: Type)(implicit ctx: Context): Tree =
620643
appliedToTypes(targ :: Nil)
621644

645+
/** The current tree applied to given type arguments: `tree[targ0, ..., targN]` */
622646
def appliedToTypes(targs: List[Type])(implicit ctx: Context): Tree =
623647
appliedToTypeTrees(targs map (TypeTree(_)))
624648

649+
/** The current tree applied to given type argument list: `tree[targs(0), ..., targs(targs.length - 1)]` */
625650
def appliedToTypeTrees(targs: List[Tree])(implicit ctx: Context): Tree =
626651
if (targs.isEmpty) tree else TypeApply(tree, targs)
627652

653+
/** Apply to `()` unless tree's widened type is parameterless */
628654
def ensureApplied(implicit ctx: Context): Tree =
629655
if (tree.tpe.widen.isParameterless) tree else tree.appliedToNone
630656

657+
/** `tree.isInstanceOf[tp]` */
631658
def isInstance(tp: Type)(implicit ctx: Context): Tree =
632659
tree.select(defn.Any_isInstanceOf).appliedToType(tp)
633660

661+
/** tree.asInstanceOf[`tp`] */
634662
def asInstance(tp: Type)(implicit ctx: Context): Tree = {
635663
assert(tp.isValueType, i"bad cast: $tree.asInstanceOf[$tp]")
636664
tree.select(defn.Any_asInstanceOf).appliedToType(tp)
637665
}
638666

667+
/** `tree.asInstanceOf[tp]` unless tree's type already conforms to `tp` */
639668
def ensureConforms(tp: Type)(implicit ctx: Context): Tree =
640669
if (tree.tpe <:< tp) tree else asInstance(tp)
641670

671+
/** `this && that`, for boolean trees `this`, `that` */
642672
def and(that: Tree)(implicit ctx: Context): Tree =
643673
tree.select(defn.Boolean_&&).appliedTo(that)
644674

675+
/** `this || that`, for boolean trees `this`, `that` */
645676
def or(that: Tree)(implicit ctx: Context): Tree =
646677
tree.select(defn.Boolean_||).appliedTo(that)
647678

679+
/** The translation of `tree = rhs`.
680+
* This is either the tree as an assignment, to a setter call.
681+
*/
648682
def becomes(rhs: Tree)(implicit ctx: Context): Tree =
649683
if (tree.symbol is Method) {
650684
val setr = tree match {
@@ -660,20 +694,23 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
660694

661695
// --- Higher order traversal methods -------------------------------
662696

663-
def foreachSubTree(f: Tree => Unit)(implicit ctx: Context): Unit = { //TODO should go in tpd.
697+
/** Apply `f` to each subtree of this tree */
698+
def foreachSubTree(f: Tree => Unit)(implicit ctx: Context): Unit = {
664699
val traverser = new TreeTraverser {
665700
def traverse(tree: Tree)(implicit ctx: Context) = foldOver(f(tree), tree)
666701
}
667702
traverser.traverse(tree)
668703
}
669704

705+
/** Is there a subtree of this tree that satisfies predicate `p`? */
670706
def existsSubTree(p: Tree => Boolean)(implicit ctx: Context): Boolean = {
671707
val acc = new TreeAccumulator[Boolean] {
672708
def apply(x: Boolean, t: Tree)(implicit ctx: Context) = x || p(t) || foldOver(x, t)
673709
}
674710
acc(false, tree)
675711
}
676712

713+
/** All subtrees of this tree that satisfy predicate `p`. */
677714
def filterSubTrees(f: Tree => Boolean)(implicit ctx: Context): List[Tree] = {
678715
val buf = new mutable.ListBuffer[Tree]
679716
foreachSubTree { tree => if (f(tree)) buf += tree }

src/dotty/tools/dotc/transform/InterceptedMethods.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,9 @@ class InterceptedMethods extends MiniPhaseTransform { thisTransform =>
111111
poundPoundValue(qual)
112112
} else if (Any_comparisons contains tree.fun.symbol.asTerm) {
113113
if (tree.fun.symbol eq defn.Any_==) {
114-
qual.select(defn.Any_equals).appliedToArgs(tree.args)
114+
qual.selectWithSig(defn.Any_equals).appliedToArgs(tree.args)
115115
} else if (tree.fun.symbol eq defn.Any_!=) {
116-
qual.select(defn.Any_equals).appliedToArgs(tree.args).select(defn.Boolean_!)
116+
qual.selectWithSig(defn.Any_equals).appliedToArgs(tree.args).select(defn.Boolean_!)
117117
} else unknown
118118
} /* else if (isPrimitiveValueClass(qual.tpe.typeSymbol)) {
119119
// todo: this is needed to support value classes
@@ -130,7 +130,7 @@ class InterceptedMethods extends MiniPhaseTransform { thisTransform =>
130130
// we get a primitive form of _getClass trying to target a boxed value
131131
// so we need replace that method name with Object_getClass to get correct behavior.
132132
// See SI-5568.
133-
qual.select(defn.Any_getClass).appliedToNone
133+
qual.selectWithSig(defn.Any_getClass).appliedToNone
134134
} else {
135135
unknown
136136
}

0 commit comments

Comments
 (0)