From b574ac0727ab704d0cbb037047021e791e0c70d1 Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Fri, 3 Apr 2015 14:59:22 +0200 Subject: [PATCH 1/5] Fix #453, patternMatcher should use == --- src/dotty/tools/dotc/transform/PatternMatcher.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/transform/PatternMatcher.scala b/src/dotty/tools/dotc/transform/PatternMatcher.scala index 33e52e0680e3..1336d39e4aef 100644 --- a/src/dotty/tools/dotc/transform/PatternMatcher.scala +++ b/src/dotty/tools/dotc/transform/PatternMatcher.scala @@ -141,7 +141,8 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans } // NOTE: checker must be the target of the ==, that's the patmat semantics for ya - def _equals(checker: Tree, binder: Symbol): Tree = checker.select(nme.equals_).appliedTo(ref(binder)) + def _equals(checker: Tree, binder: Symbol): Tree = + tpd.applyOverloaded(checker, nme.EQ, List(ref(binder)), List.empty, defn.BooleanType) // the force is needed mainly to deal with the GADT typing hack (we can't detect it otherwise as tp nor pt need contain an abstract type, we're just casting wildly) def _asInstanceOf(b: Symbol, tp: Type): Tree = ref(b).ensureConforms(tp) // andType here breaks t1048 From a2a12a263467df11fb7ba8a1cc18ec9984530d94 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 9 Apr 2015 12:43:29 +0200 Subject: [PATCH 2/5] Rename -YnoDoubleBindings to -Yno-double-bindings This aligns with the "-" instead of CamelCase convention for the other command line options. Also, enable -Yno-double-bindings for dotc_core. --- src/dotty/tools/dotc/ast/TreeTypeMap.scala | 2 +- src/dotty/tools/dotc/config/ScalaSettings.scala | 2 +- test/dotc/tests.scala | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/dotty/tools/dotc/ast/TreeTypeMap.scala b/src/dotty/tools/dotc/ast/TreeTypeMap.scala index d2ec3ea105a3..ec66fcdb8296 100644 --- a/src/dotty/tools/dotc/ast/TreeTypeMap.scala +++ b/src/dotty/tools/dotc/ast/TreeTypeMap.scala @@ -30,7 +30,7 @@ import dotty.tools.dotc.transform.SymUtils._ * have two substitutons S1 = [outer#1 := outer#3], S2 = [inner#2 := inner#4] where * hashtags precede symbol ids. If we do S1 first, we get outer#2.inner#3. If we then * do S2 we get outer#2.inner#4. But that means that the named type outer#2.inner - * gets two different denotations in the same period. Hence, if -YnoDoubleBindings is + * gets two different denotations in the same period. Hence, if -Yno-double-bindings is * set, we would get a data race assertion error. */ final class TreeTypeMap( diff --git a/src/dotty/tools/dotc/config/ScalaSettings.scala b/src/dotty/tools/dotc/config/ScalaSettings.scala index 1c1f3e49490f..0d4034db28c5 100644 --- a/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -170,7 +170,7 @@ class ScalaSettings extends Settings.SettingGroup { val Ytyperdebug = BooleanSetting("-Ytyper-debug", "Trace all type assignments.") val Ypatmatdebug = BooleanSetting("-Ypatmat-debug", "Trace pattern matching translation.") val Yexplainlowlevel = BooleanSetting("-Yexplain-lowlevel", "When explaining type errors, show types at a lower level.") - val YnoDoubleBindings = BooleanSetting("-YnoDoubleBindings", "Assert no namedtype is bound twice (should be enabled only if program is error-free).") + val YnoDoubleBindings = BooleanSetting("-Yno-double-bindings", "Assert no namedtype is bound twice (should be enabled only if program is error-free).") val optimise = BooleanSetting("-optimise", "Generates faster bytecode by applying optimisations to the program") withAbbreviation "-optimize" diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index d1b5538dc399..bd092a482b3e 100644 --- a/test/dotc/tests.scala +++ b/test/dotc/tests.scala @@ -25,7 +25,7 @@ class tests extends CompilerTest { val testPickling = List("-Xprint-types", "-Ytest-pickler", "-Ystop-after:pickler") val failedOther = List("-Ystop-before:collectEntryPoints") // some non-obvious reason. need to look deeper - val twice = List("#runs", "2", "-YnoDoubleBindings") + val twice = List("#runs", "2", "-Yno-double-bindings") val staleSymbolError: List[String] = List() val allowDeepSubtypes = defaultOptions diff List("-Yno-deep-subtypes") @@ -130,8 +130,8 @@ class tests extends CompilerTest { @Test def dotc = compileDir(dotcDir + "tools/dotc", failedOther)(allowDeepSubtypes ++ twice) // see dotc_core @Test def dotc_ast = compileDir(dotcDir + "tools/dotc/ast", failedOther ++ twice) //similar to dotc_core_pickling but for another anon class. Still during firstTransform - @Test def dotc_config = compileDir(dotcDir + "tools/dotc/config", twice) - @Test def dotc_core = compileDir(dotcDir + "tools/dotc/core", failedOther)(allowDeepSubtypes) // !!!twice gives "data race?" error in InterceptedMethods + @Test def dotc_config = compileDir(dotcDir + "tools/dotc/config") + @Test def dotc_core = compileDir(dotcDir + "tools/dotc/core", failedOther)("-Yno-double-bindings" :: allowDeepSubtypes)// twice omitted to make tests run faster // error: error while loading ConstraintHandling$$anon$1$, // class file 'target/scala-2.11/dotty_2.11-0.1-SNAPSHOT.jar(dotty/tools/dotc/core/ConstraintHandling$$anon$1.class)' // has location not matching its contents: contains class $anon From 673842f94e68beb6f8bfa8136b37578464b0e55b Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 9 Apr 2015 12:48:33 +0200 Subject: [PATCH 3/5] 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. --- src/dotty/tools/dotc/ast/tpd.scala | 41 ++++++++++++++++++- .../dotc/transform/InterceptedMethods.scala | 6 +-- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/src/dotty/tools/dotc/ast/tpd.scala b/src/dotty/tools/dotc/ast/tpd.scala index 9e799a8d3a85..0f4585a53f77 100644 --- a/src/dotty/tools/dotc/ast/tpd.scala +++ b/src/dotty/tools/dotc/ast/tpd.scala @@ -588,63 +588,97 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { tree } + /** A select node with the given selector name and a computed type */ def select(name: Name)(implicit ctx: Context): Select = Select(tree, name) + /** A select node with the given type */ def select(tp: NamedType)(implicit ctx: Context): Select = untpd.Select(tree, tp.name).withType(tp) + /** A select node that selects the given symbol. Note: Need to make sure this + * is in fact the symbol you would get when you select with the symbol's name, + * otherwise a data race may occur which would be flagged by -Yno-double-bindings. + */ def select(sym: Symbol)(implicit ctx: Context): Select = untpd.Select(tree, sym.name).withType( TermRef.withSigAndDenot(tree.tpe, sym.name.asTermName, sym.signature, sym.denot.asSeenFrom(tree.tpe))) - def selectWithSig(name: Name, sig: Signature)(implicit ctx: Context) = + /** A select node with the given selector name and signature and a computed type */ + def selectWithSig(name: Name, sig: Signature)(implicit ctx: Context): Tree = untpd.SelectWithSig(tree, name, sig) .withType(TermRef.withSig(tree.tpe, name.asTermName, sig)) + /** A select node with selector name and signature taken from `sym`. + * Note: Use this method instead of select(sym) if the referenced symbol + * might be overridden in the type of the qualifier prefix. See note + * on select(sym: Symbol). + */ + def selectWithSig(sym: Symbol)(implicit ctx: Context): Tree = + selectWithSig(sym.name, sym.signature) + + /** A unary apply node with given argument: `tree(arg)` */ def appliedTo(arg: Tree)(implicit ctx: Context): Tree = appliedToArgs(arg :: Nil) + /** An apply node with given arguments: `tree(arg, args0, ..., argsN)` */ def appliedTo(arg: Tree, args: Tree*)(implicit ctx: Context): Tree = appliedToArgs(arg :: args.toList) + /** An apply node with given argument list `tree(args(0), ..., args(args.length - 1))` */ def appliedToArgs(args: List[Tree])(implicit ctx: Context): Apply = Apply(tree, args) + /** The current tree applied to given argument lists: + * `tree (argss(0)) ... (argss(argss.length -1))` + */ def appliedToArgss(argss: List[List[Tree]])(implicit ctx: Context): Tree = ((tree: Tree) /: argss)(Apply(_, _)) + /** The current tree applied to (): `tree()` */ def appliedToNone(implicit ctx: Context): Apply = appliedToArgs(Nil) + /** The current tree applied to given type argument: `tree[targ]` */ def appliedToType(targ: Type)(implicit ctx: Context): Tree = appliedToTypes(targ :: Nil) + /** The current tree applied to given type arguments: `tree[targ0, ..., targN]` */ def appliedToTypes(targs: List[Type])(implicit ctx: Context): Tree = appliedToTypeTrees(targs map (TypeTree(_))) + /** The current tree applied to given type argument list: `tree[targs(0), ..., targs(targs.length - 1)]` */ def appliedToTypeTrees(targs: List[Tree])(implicit ctx: Context): Tree = if (targs.isEmpty) tree else TypeApply(tree, targs) + /** Apply to `()` unless tree's widened type is parameterless */ def ensureApplied(implicit ctx: Context): Tree = if (tree.tpe.widen.isParameterless) tree else tree.appliedToNone + /** `tree.isInstanceOf[tp]` */ def isInstance(tp: Type)(implicit ctx: Context): Tree = tree.select(defn.Any_isInstanceOf).appliedToType(tp) + /** tree.asInstanceOf[`tp`] */ def asInstance(tp: Type)(implicit ctx: Context): Tree = { assert(tp.isValueType, i"bad cast: $tree.asInstanceOf[$tp]") tree.select(defn.Any_asInstanceOf).appliedToType(tp) } + /** `tree.asInstanceOf[tp]` unless tree's type already conforms to `tp` */ def ensureConforms(tp: Type)(implicit ctx: Context): Tree = if (tree.tpe <:< tp) tree else asInstance(tp) + /** `this && that`, for boolean trees `this`, `that` */ def and(that: Tree)(implicit ctx: Context): Tree = tree.select(defn.Boolean_&&).appliedTo(that) + /** `this || that`, for boolean trees `this`, `that` */ def or(that: Tree)(implicit ctx: Context): Tree = tree.select(defn.Boolean_||).appliedTo(that) + /** The translation of `tree = rhs`. + * This is either the tree as an assignment, to a setter call. + */ def becomes(rhs: Tree)(implicit ctx: Context): Tree = if (tree.symbol is Method) { val setr = tree match { @@ -660,13 +694,15 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { // --- Higher order traversal methods ------------------------------- - def foreachSubTree(f: Tree => Unit)(implicit ctx: Context): Unit = { //TODO should go in tpd. + /** Apply `f` to each subtree of this tree */ + def foreachSubTree(f: Tree => Unit)(implicit ctx: Context): Unit = { val traverser = new TreeTraverser { def traverse(tree: Tree)(implicit ctx: Context) = foldOver(f(tree), tree) } traverser.traverse(tree) } + /** Is there a subtree of this tree that satisfies predicate `p`? */ def existsSubTree(p: Tree => Boolean)(implicit ctx: Context): Boolean = { val acc = new TreeAccumulator[Boolean] { def apply(x: Boolean, t: Tree)(implicit ctx: Context) = x || p(t) || foldOver(x, t) @@ -674,6 +710,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { acc(false, tree) } + /** All subtrees of this tree that satisfy predicate `p`. */ def filterSubTrees(f: Tree => Boolean)(implicit ctx: Context): List[Tree] = { val buf = new mutable.ListBuffer[Tree] foreachSubTree { tree => if (f(tree)) buf += tree } diff --git a/src/dotty/tools/dotc/transform/InterceptedMethods.scala b/src/dotty/tools/dotc/transform/InterceptedMethods.scala index 87a5c1a4ca23..458527c46eba 100644 --- a/src/dotty/tools/dotc/transform/InterceptedMethods.scala +++ b/src/dotty/tools/dotc/transform/InterceptedMethods.scala @@ -111,9 +111,9 @@ class InterceptedMethods extends MiniPhaseTransform { thisTransform => poundPoundValue(qual) } else if (Any_comparisons contains tree.fun.symbol.asTerm) { if (tree.fun.symbol eq defn.Any_==) { - qual.select(defn.Any_equals).appliedToArgs(tree.args) + qual.selectWithSig(defn.Any_equals).appliedToArgs(tree.args) } else if (tree.fun.symbol eq defn.Any_!=) { - qual.select(defn.Any_equals).appliedToArgs(tree.args).select(defn.Boolean_!) + qual.selectWithSig(defn.Any_equals).appliedToArgs(tree.args).select(defn.Boolean_!) } else unknown } /* else if (isPrimitiveValueClass(qual.tpe.typeSymbol)) { // todo: this is needed to support value classes @@ -130,7 +130,7 @@ class InterceptedMethods extends MiniPhaseTransform { thisTransform => // we get a primitive form of _getClass trying to target a boxed value // so we need replace that method name with Object_getClass to get correct behavior. // See SI-5568. - qual.select(defn.Any_getClass).appliedToNone + qual.selectWithSig(defn.Any_getClass).appliedToNone } else { unknown } From 651b8ddbf518327c059ee053acf586a35903c87f Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 9 Apr 2015 13:30:34 +0200 Subject: [PATCH 4/5] Make -Yno-double-bindings the default for all tests. --- test/dotc/tests.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index bd092a482b3e..095c5053bab1 100644 --- a/test/dotc/tests.scala +++ b/test/dotc/tests.scala @@ -14,21 +14,21 @@ class tests extends CompilerTest { "-pagewidth", "160") implicit val defaultOptions = noCheckOptions ++ List( - "-Yno-deep-subtypes", + "-Yno-deep-subtypes", "-Yno-double-bindings", "-Ycheck:tailrec,resolveSuper,mixin,restoreScopes", "-d", "./out/" ) val doEmitBytecode = List("-Ystop-before:terminal") val failedbyName = List("-Ystop-before:collectEntryPoints") // #288 - val failedUnderscore = List("-Ystop-before:collectEntryPoints") // #289 val testPickling = List("-Xprint-types", "-Ytest-pickler", "-Ystop-after:pickler") val failedOther = List("-Ystop-before:collectEntryPoints") // some non-obvious reason. need to look deeper - val twice = List("#runs", "2", "-Yno-double-bindings") + val twice = List("#runs", "2") val staleSymbolError: List[String] = List() val allowDeepSubtypes = defaultOptions diff List("-Yno-deep-subtypes") + val allowDoubleBindings = defaultOptions diff List("-Yno-double-bindings") val posDir = "./tests/pos/" val posSpecialDir = "./tests/pos-special/" From d7f6e377899c2450c0919332e184efad9593d8f8 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 9 Apr 2015 14:01:03 +0200 Subject: [PATCH 5/5] Disabled -Yno-double-bindings for a neg test. The test introduced a double definition, which led to a double binding. With -Yno-double-bindings this cauases an assertion violation instead of a reported error. --- test/dotc/tests.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index 095c5053bab1..c3655253680d 100644 --- a/test/dotc/tests.scala +++ b/test/dotc/tests.scala @@ -88,7 +88,7 @@ class tests extends CompilerTest { @Test def neg_typedapply() = compileFile(negDir, "typedapply", xerrors = 4) @Test def neg_typedidents() = compileFile(negDir, "typedIdents", xerrors = 2) @Test def neg_assignments() = compileFile(negDir, "assignments", xerrors = 3) - @Test def neg_typers() = compileFile(negDir, "typers", xerrors = 12) + @Test def neg_typers() = compileFile(negDir, "typers", xerrors = 12)(allowDoubleBindings) @Test def neg_privates() = compileFile(negDir, "privates", xerrors = 2) @Test def neg_rootImports = compileFile(negDir, "rootImplicits", xerrors = 2) @Test def neg_templateParents() = compileFile(negDir, "templateParents", xerrors = 3)