diff options
author | Jason Zaugg <jzaugg@gmail.com> | 2014-01-12 18:12:29 +0100 |
---|---|---|
committer | Jason Zaugg <jzaugg@gmail.com> | 2014-01-14 09:00:05 +0100 |
commit | 2524fdde3edc7b668fdb4bf68e990141d3ec18d6 (patch) | |
tree | 61a99c3191b26f58d1404e1463ae4a90a807ae14 /src/compiler | |
parent | e089cafb5fd02e2457bafde3252da3a771d3180e (diff) | |
download | scala-2524fdde3edc7b668fdb4bf68e990141d3ec18d6.tar.gz scala-2524fdde3edc7b668fdb4bf68e990141d3ec18d6.tar.bz2 scala-2524fdde3edc7b668fdb4bf68e990141d3ec18d6.zip |
SI-8143 Regressions with override checks, private members
These regressed in e609f1f20b, which excluded all private methods from
overriding checks. We should only exclude private[this] members on the
low end of a pair, as was done before that commit, and, we must also
exclude private members on the high side.
Why? Warning: reverse engineered intuition follows.
We need to report an error when if a private method in a subclass
has matches a less-private method in the super class and report an
error, lest the user be fooled into thinking it might be invoked
virtually. On the other hand, adding a private method to a super
class shouldn't invalidate the choice names of public members in
its superclasses.
I've removed the test case added by that commit and will lodge a
reworked version of it that Paul provided as a new issue. That shows
a bug with qualified private + inheritance.
In addition, the expectation of `neg/accesses.check` is reverted
to its 2.10.3 version, which I believe is correct. When it was
changed in e609f1f20b it sprouted a variation, `neg/accesses-2`,
which has now changed behaviour. The intent of that test will
be captured in the aforementioned issue covering qualified private
inheritance.
Diffstat (limited to 'src/compiler')
3 files changed, 11 insertions, 9 deletions
diff --git a/src/compiler/scala/tools/nsc/transform/Erasure.scala b/src/compiler/scala/tools/nsc/transform/Erasure.scala index 6732900ef2..54b5b399ac 100644 --- a/src/compiler/scala/tools/nsc/transform/Erasure.scala +++ b/src/compiler/scala/tools/nsc/transform/Erasure.scala @@ -760,7 +760,7 @@ abstract class Erasure extends AddInterfaces || super.exclude(sym) || !sym.hasTypeAt(currentRun.refchecksPhase.id) ) - override def matches(sym1: Symbol, sym2: Symbol) = true + override def matches(lo: Symbol, high: Symbol) = true } def isErasureDoubleDef(pair: SymbolPair) = { import pair._ diff --git a/src/compiler/scala/tools/nsc/transform/OverridingPairs.scala b/src/compiler/scala/tools/nsc/transform/OverridingPairs.scala index 4222c4d8c8..870eafbf20 100644 --- a/src/compiler/scala/tools/nsc/transform/OverridingPairs.scala +++ b/src/compiler/scala/tools/nsc/transform/OverridingPairs.scala @@ -24,15 +24,16 @@ abstract class OverridingPairs extends SymbolPairs { /** Symbols to exclude: Here these are constructors and private/artifact symbols, * including bridges. But it may be refined in subclasses. */ - override protected def exclude(sym: Symbol) = (sym hasFlag PRIVATE | ARTIFACT) || sym.isConstructor + override protected def exclude(sym: Symbol) = sym.isPrivateLocal || sym.isArtifact || sym.isConstructor /** Types always match. Term symbols match if their member types * relative to `self` match. */ - override protected def matches(sym1: Symbol, sym2: Symbol) = sym1.isType || ( - (sym1.owner != sym2.owner) - && !exclude(sym2) - && relatively.matches(sym1, sym2) - ) + override protected def matches(lo: Symbol, high: Symbol) = lo.isType || ( + (lo.owner != high.owner) // don't try to form pairs from overloaded members + && !high.isPrivate // private or private[this] members never are overriden + && !exclude(lo) // this admits private, as one can't have a private member that matches a less-private member. + && relatively.matches(lo, high) + ) // TODO we don't call exclude(high), should we? } } diff --git a/src/compiler/scala/tools/nsc/typechecker/Checkable.scala b/src/compiler/scala/tools/nsc/typechecker/Checkable.scala index 94f8f509fc..b899cd8994 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Checkable.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Checkable.scala @@ -186,7 +186,7 @@ trait Checkable { * additional conditions holds: * - either A or B is effectively final * - neither A nor B is a trait (i.e. both are actual classes, not eligible for mixin) - * - both A and B are sealed, and every possible pairing of their children is irreconcilable + * - both A and B are sealed/final, and every possible pairing of their children is irreconcilable * * TODO: the last two conditions of the last possibility (that the symbols are not of * classes being compiled in the current run) are because this currently runs too early, @@ -198,8 +198,9 @@ trait Checkable { isEffectivelyFinal(sym1) // initialization important || isEffectivelyFinal(sym2) || !sym1.isTrait && !sym2.isTrait - || sym1.isSealed && sym2.isSealed && allChildrenAreIrreconcilable(sym1, sym2) && !currentRun.compiles(sym1) && !currentRun.compiles(sym2) + || isSealedOrFinal(sym1) && isSealedOrFinal(sym2) && allChildrenAreIrreconcilable(sym1, sym2) && !currentRun.compiles(sym1) && !currentRun.compiles(sym2) ) + private def isSealedOrFinal(sym: Symbol) = sym.isSealed || sym.isFinal private def isEffectivelyFinal(sym: Symbol): Boolean = ( // initialization important sym.initialize.isEffectivelyFinal || ( |