From b93ffa5cc46533cb8a74d19fbc46e0287cedecdd Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 15 Jul 2016 13:06:22 +0200 Subject: [PATCH 1/3] Fix #1386: Reduce double def errors Use additional disambiguation criteria before raising a double def error. See for context: #1240. Review by @darkdimius --- src/dotty/tools/dotc/core/Denotations.scala | 30 ++++++++++++++++----- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/src/dotty/tools/dotc/core/Denotations.scala b/src/dotty/tools/dotc/core/Denotations.scala index 5ce8cbcd8b5f..e23e66b95aeb 100644 --- a/src/dotty/tools/dotc/core/Denotations.scala +++ b/src/dotty/tools/dotc/core/Denotations.scala @@ -292,24 +292,41 @@ object Denotations { val sym1 = denot1.symbol val sym2 = denot2.symbol - if (isDoubleDef(sym1, sym2)) doubleDefError(denot1, denot2, pre) - val sym2Accessible = sym2.isAccessibleFrom(pre) + /** Does `sym1` come before `sym2` in the linearization of `pre`? */ def precedes(sym1: Symbol, sym2: Symbol) = { def precedesIn(bcs: List[ClassSymbol]): Boolean = bcs match { case bc :: bcs1 => (sym1 eq bc) || !(sym2 eq bc) && precedesIn(bcs1) case Nil => true } - sym1.derivesFrom(sym2) || - !sym2.derivesFrom(sym1) && precedesIn(pre.baseClasses) + (sym1 ne sym2) && + (sym1.derivesFrom(sym2) || + !sym2.derivesFrom(sym1) && precedesIn(pre.baseClasses)) } - /** Preference according to partial pre-order (isConcrete, precedes) */ + /** Establish a partial order "preference" order between symbols. + * Give preference to `sym1` over `sym2` if one of the following + * conditions holds, in decreasing order of weight: + * 1. sym1 is concrete and sym2 is abstract + * 2. The owner of sym1 comes before the owner of sym2 in the linearization + * of the type of the prefix `pre`. + * 3. The access boundary of sym2 is properly contained in the access + * boundary of sym1. For protected access, we count the enclosing + * package as access boundary. + * 4. sym1 a method but sym2 is not. + * The aim of these criteria is to give some disambiguation on access which + * - does not depend on textual order or other arbitrary choices + * - minimizes raising of doubleDef errors + */ def preferSym(sym1: Symbol, sym2: Symbol) = sym1.eq(sym2) || sym1.isAsConcrete(sym2) && - (!sym2.isAsConcrete(sym1) || precedes(sym1.owner, sym2.owner)) + (!sym2.isAsConcrete(sym1) || + precedes(sym1.owner, sym2.owner) || + sym2.accessBoundary(sym2.enclosingPackageClass) + .isProperlyContainedIn(sym1.accessBoundary(sym1.enclosingPackageClass)) || + sym1.is(Method) && !sym2.is(Method)) /** Sym preference provided types also override */ def prefer(sym1: Symbol, sym2: Symbol, info1: Type, info2: Type) = @@ -321,6 +338,7 @@ object Denotations { if (sym1Accessible && prefer(sym1, sym2, info1, info2)) denot1 else if (sym1Accessible && sym2.exists && !sym2Accessible) denot1 else if (sym2Accessible && sym1.exists && !sym1Accessible) denot2 + else if (isDoubleDef(sym1, sym2)) doubleDefError(denot1, denot2, pre) else { val sym = if (!sym1.exists) sym2 From 7ec75f35e192a795945e889bf383ef02521e4b69 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 15 Jul 2016 14:28:11 +0200 Subject: [PATCH 2/3] Refine disambiguation logic and add test case. --- src/dotty/tools/dotc/core/Denotations.scala | 17 ++++++++++++++--- tests/run/i1386.scala | 4 ++++ 2 files changed, 18 insertions(+), 3 deletions(-) create mode 100644 tests/run/i1386.scala diff --git a/src/dotty/tools/dotc/core/Denotations.scala b/src/dotty/tools/dotc/core/Denotations.scala index e23e66b95aeb..132f04aa9998 100644 --- a/src/dotty/tools/dotc/core/Denotations.scala +++ b/src/dotty/tools/dotc/core/Denotations.scala @@ -305,6 +305,14 @@ object Denotations { !sym2.derivesFrom(sym1) && precedesIn(pre.baseClasses)) } + /** Similar to SymDenotation#accessBoundary, but without the special cases. */ + def accessBoundary(sym: Symbol) = + if (sym.is(Private)) sym.owner + else sym.privateWithin.orElse( + if (sym.is(Protected)) sym.owner.enclosingPackageClass + else defn.RootClass + ) + /** Establish a partial order "preference" order between symbols. * Give preference to `sym1` over `sym2` if one of the following * conditions holds, in decreasing order of weight: @@ -324,8 +332,7 @@ object Denotations { sym1.isAsConcrete(sym2) && (!sym2.isAsConcrete(sym1) || precedes(sym1.owner, sym2.owner) || - sym2.accessBoundary(sym2.enclosingPackageClass) - .isProperlyContainedIn(sym1.accessBoundary(sym1.enclosingPackageClass)) || + accessBoundary(sym2).isProperlyContainedIn(accessBoundary(sym1)) || sym1.is(Method) && !sym2.is(Method)) /** Sym preference provided types also override */ @@ -338,7 +345,11 @@ object Denotations { if (sym1Accessible && prefer(sym1, sym2, info1, info2)) denot1 else if (sym1Accessible && sym2.exists && !sym2Accessible) denot1 else if (sym2Accessible && sym1.exists && !sym1Accessible) denot2 - else if (isDoubleDef(sym1, sym2)) doubleDefError(denot1, denot2, pre) + else if (isDoubleDef(sym1, sym2)) { + if (preferSym(sym1, sym2)) denot1 + else if (preferSym(sym2, sym1)) denot2 + else doubleDefError(denot1, denot2, pre) + } else { val sym = if (!sym1.exists) sym2 diff --git a/tests/run/i1386.scala b/tests/run/i1386.scala new file mode 100644 index 000000000000..e5f4332d26c6 --- /dev/null +++ b/tests/run/i1386.scala @@ -0,0 +1,4 @@ +object Test { + def main(args: Array[String]) = + assert(new java.util.HashMap[Int, Int]().size == 0) +} From 5c4496a2fc710d5f969fe9891b92e22ecec34057 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 15 Jul 2016 14:48:53 +0200 Subject: [PATCH 3/3] Refactor code into nested method mergeDenot is already large as it is. --- src/dotty/tools/dotc/core/Denotations.scala | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/dotty/tools/dotc/core/Denotations.scala b/src/dotty/tools/dotc/core/Denotations.scala index 132f04aa9998..a41ebbdbaf87 100644 --- a/src/dotty/tools/dotc/core/Denotations.scala +++ b/src/dotty/tools/dotc/core/Denotations.scala @@ -339,17 +339,18 @@ object Denotations { def prefer(sym1: Symbol, sym2: Symbol, info1: Type, info2: Type) = preferSym(sym1, sym2) && info1.overrides(info2) + def handleDoubleDef = + if (preferSym(sym1, sym2)) denot1 + else if (preferSym(sym2, sym1)) denot2 + else doubleDefError(denot1, denot2, pre) + if (sym2Accessible && prefer(sym2, sym1, info2, info1)) denot2 else { val sym1Accessible = sym1.isAccessibleFrom(pre) if (sym1Accessible && prefer(sym1, sym2, info1, info2)) denot1 else if (sym1Accessible && sym2.exists && !sym2Accessible) denot1 else if (sym2Accessible && sym1.exists && !sym1Accessible) denot2 - else if (isDoubleDef(sym1, sym2)) { - if (preferSym(sym1, sym2)) denot1 - else if (preferSym(sym2, sym1)) denot2 - else doubleDefError(denot1, denot2, pre) - } + else if (isDoubleDef(sym1, sym2)) handleDoubleDef else { val sym = if (!sym1.exists) sym2