Skip to content

Commit ca0ef84

Browse files
oderskyodersky
authored
Merge pull request #8622 from dotty-staging/fix-#8617
Fix #8617: Check that there is no inheritance/definition shadowing
2 parents a0690e1 + 617b95c commit ca0ef84

26 files changed

+206
-64
lines changed

‎community-build/community-projects/scala-xml

‎community-build/community-projects/scalacheck

‎community-build/community-projects/scalatest

‎community-build/community-projects/stdLib213

‎compiler/src/dotty/tools/dotc/ast/Desugar.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ object desugar {
112112
val originalOwner = sym.owner
113113
def apply(tp: Type) = tp match {
114114
case tp: NamedType if tp.symbol.exists && (tp.symbol.owner eq originalOwner) =>
115-
val defctx = ctx.outersIterator.dropWhile(_.scope eq ctx.scope).next()
115+
val defctx = this.ctx.outersIterator.dropWhile(_.scope eq this.ctx.scope).next()
116116
var local = defctx.denotNamed(tp.name).suchThat(_.isParamOrAccessor).symbol
117117
if (local.exists) (defctx.owner.thisType select local).dealiasKeepAnnots
118118
else {

‎compiler/src/dotty/tools/dotc/core/Denotations.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1243,7 +1243,7 @@ object Denotations {
12431243
throw new MergeError(sym1, sym2, sym1.info, sym2.info, pre) {
12441244
override def addendum(implicit ctx: Context) =
12451245
i"""
1246-
|they are both defined in ${sym1.effectiveOwner} but have matching signatures
1246+
|they are both defined in ${this.sym1.effectiveOwner} but have matching signatures
12471247
| ${denot1.info} and
12481248
| ${denot2.info}${super.addendum}"""
12491249
}

‎compiler/src/dotty/tools/dotc/parsing/Parsers.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3784,7 +3784,7 @@ object Parsers {
37843784
i"refinement cannot have default arguments"
37853785
case tree: ValOrDefDef =>
37863786
if tree.rhs.isEmpty then ""
3787-
else "refinement in cannot have a right-hand side"
3787+
else "refinement cannot have a right-hand side"
37883788
case tree: TypeDef =>
37893789
if !tree.isClassDef then ""
37903790
else "refinement cannot be a class or trait"

‎compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ enum ErrorMessageID extends java.lang.Enum[ErrorMessageID] {
5757
CyclicReferenceInvolvingID,
5858
CyclicReferenceInvolvingImplicitID,
5959
SuperQualMustBeParentID,
60-
AmbiguousImportID,
60+
AmbiguousReferenceID,
6161
MethodDoesNotTakeParametersId,
6262
AmbiguousOverloadID,
6363
ReassignmentToValID,

‎compiler/src/dotty/tools/dotc/reporting/messages.scala

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -250,13 +250,13 @@ object messages {
250250
// these are usually easier to analyze.
251251
object reported extends TypeMap:
252252
def setVariance(v: Int) = variance = v
253-
val constraint = ctx.typerState.constraint
253+
val constraint = this.ctx.typerState.constraint
254254
def apply(tp: Type): Type = tp match
255255
case tp: TypeParamRef =>
256256
constraint.entry(tp) match
257257
case bounds: TypeBounds =>
258-
if variance < 0 then apply(ctx.typeComparer.fullUpperBound(tp))
259-
else if variance > 0 then apply(ctx.typeComparer.fullLowerBound(tp))
258+
if variance < 0 then apply(this.ctx.typeComparer.fullUpperBound(tp))
259+
else if variance > 0 then apply(this.ctx.typeComparer.fullLowerBound(tp))
260260
else tp
261261
case NoType => tp
262262
case instType => apply(instType)
@@ -1244,8 +1244,8 @@ object messages {
12441244

12451245
import typer.Typer.BindingPrec
12461246

1247-
class AmbiguousImport(name: Name, newPrec: BindingPrec, prevPrec: BindingPrec, prevCtx: Context)(implicit ctx: Context)
1248-
extends ReferenceMsg(AmbiguousImportID) {
1247+
class AmbiguousReference(name: Name, newPrec: BindingPrec, prevPrec: BindingPrec, prevCtx: Context)(implicit ctx: Context)
1248+
extends ReferenceMsg(AmbiguousReferenceID) {
12491249

12501250
/** A string which explains how something was bound; Depending on `prec` this is either
12511251
* imported by <tree>
@@ -1254,6 +1254,7 @@ object messages {
12541254
private def bindingString(prec: BindingPrec, whereFound: Context, qualifier: String = "") = {
12551255
val howVisible = prec match {
12561256
case BindingPrec.Definition => "defined"
1257+
case BindingPrec.Inheritance => "inherited"
12571258
case BindingPrec.NamedImport => "imported by name"
12581259
case BindingPrec.WildImport => "imported"
12591260
case BindingPrec.PackageClause => "found"
@@ -1266,18 +1267,21 @@ object messages {
12661267
}
12671268

12681269
def msg =
1269-
i"""|Reference to ${em"$name"} is ambiguous
1270+
i"""|Reference to ${em"$name"} is ambiguous,
12701271
|it is both ${bindingString(newPrec, ctx)}
12711272
|and ${bindingString(prevPrec, prevCtx, " subsequently")}"""
12721273

12731274
def explain =
12741275
em"""|The compiler can't decide which of the possible choices you
1275-
|are referencing with $name.
1276+
|are referencing with $name: A definition of lower precedence
1277+
|in an inner scope, or a definition with higher precedence in
1278+
|an outer scope.
12761279
|Note:
1277-
|- Definitions take precedence over imports
1278-
|- Named imports take precedence over wildcard imports
1279-
|- You may replace a name when imported using
1280-
| ${hl("import")} scala.{ $name => ${name.show + "Tick"} }
1280+
| - Definitions in an enclosing scope take precedence over inherited definitions
1281+
| - Definitions take precedence over imports
1282+
| - Named imports take precedence over wildcard imports
1283+
| - You may replace a name when imported using
1284+
| ${hl("import")} scala.{ $name => ${name.show + "Tick"} }
12811285
|"""
12821286
}
12831287

‎compiler/src/dotty/tools/dotc/transform/PCPCheckAndHeal.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ class PCPCheckAndHeal(@constructorOnly ictx: Context) extends TreeMapWithStages(
138138
tp match {
139139
case tp: TypeRef if tp.symbol.isSplice =>
140140
if (tp.isTerm)
141-
ctx.error(i"splice outside quotes", pos)
141+
this.ctx.error(i"splice outside quotes", pos)
142142
if level > 0 then getQuoteTypeTags.getTagRef(tp.prefix.asInstanceOf[TermRef])
143143
else tp
144144
case tp: TypeRef if tp.symbol == defn.QuotedTypeClass.typeParams.head =>

‎compiler/src/dotty/tools/dotc/typer/Implicits.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ object Implicits {
414414
case t: TypeParamRef =>
415415
constraint.entry(t) match {
416416
case NoType => t
417-
case bounds: TypeBounds => ctx.typeComparer.fullBounds(t)
417+
case bounds: TypeBounds => this.ctx.typeComparer.fullBounds(t)
418418
case t1 => t1
419419
}
420420
case t: TypeVar =>

‎compiler/src/dotty/tools/dotc/typer/Inferencing.scala

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ object Inferencing {
6767
def apply(tvars: Set[TypeVar], tp: Type) = tp match {
6868
case tp: TypeVar
6969
if !tp.isInstantiated &&
70-
ctx.typeComparer.bounds(tp.origin)
70+
this.ctx.typeComparer.bounds(tp.origin)
7171
.namedPartsWith(ref => params.contains(ref.symbol))
7272
.nonEmpty =>
7373
tvars + tp
@@ -172,13 +172,13 @@ object Inferencing {
172172
def traverse(tp: Type): Unit = {
173173
tp match {
174174
case param: TypeParamRef =>
175-
val constraint = ctx.typerState.constraint
175+
val constraint = this.ctx.typerState.constraint
176176
constraint.entry(param) match {
177177
case TypeBounds(lo, hi)
178178
if (hi frozen_<:< lo) =>
179-
val inst = ctx.typeComparer.approximation(param, fromBelow = true)
179+
val inst = this.ctx.typeComparer.approximation(param, fromBelow = true)
180180
typr.println(i"replace singleton $param := $inst")
181-
ctx.typerState.constraint = constraint.replace(param, inst)
181+
this.ctx.typerState.constraint = constraint.replace(param, inst)
182182
case _ =>
183183
}
184184
case _ =>
@@ -333,7 +333,7 @@ object Inferencing {
333333
def setVariance(v: Int) = variance = v
334334
def apply(vmap: VarianceMap, t: Type): VarianceMap = t match {
335335
case t: TypeVar
336-
if !t.isInstantiated && ctx.typerState.constraint.contains(t) =>
336+
if !t.isInstantiated && this.ctx.typerState.constraint.contains(t) =>
337337
val v = vmap(t)
338338
if (v == null) vmap.updated(t, variance)
339339
else if (v == variance || v == 0) vmap

‎compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,8 @@ trait TypeAssigner {
123123
range(defn.NothingType, apply(classBound(tp.cls.classInfo)))
124124
case tp: SkolemType if partsToAvoid(mutable.Set.empty, tp.info).nonEmpty =>
125125
range(defn.NothingType, apply(tp.info))
126-
case tp: TypeVar if ctx.typerState.constraint.contains(tp) =>
127-
val lo = ctx.typeComparer.instanceType(
126+
case tp: TypeVar if this.ctx.typerState.constraint.contains(tp) =>
127+
val lo = this.ctx.typeComparer.instanceType(
128128
tp.origin, fromBelow = variance > 0 || variance == 0 && tp.hasLowerBound)
129129
val lo1 = apply(lo)
130130
if (lo1 ne lo) lo1 else tp

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

Lines changed: 55 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ object Typer {
4949
* accessed by an Ident.
5050
*/
5151
enum BindingPrec {
52-
case NothingBound, PackageClause, WildImport, NamedImport, Definition
52+
case NothingBound, PackageClause, WildImport, NamedImport, Inheritance, Definition
5353

5454
def isImportPrec = this == NamedImport || this == WildImport
5555
}
@@ -135,10 +135,9 @@ class Typer extends Namer
135135
* package as a type from source is always an error.
136136
*/
137137
def qualifies(denot: Denotation): Boolean =
138-
reallyExists(denot) &&
139-
!(pt.isInstanceOf[UnapplySelectionProto] &&
140-
(denot.symbol is (Method, butNot = Accessor))) &&
141-
!(denot.symbol.is(PackageClass))
138+
reallyExists(denot)
139+
&& !(pt.isInstanceOf[UnapplySelectionProto] && denot.symbol.is(Method, butNot = Accessor))
140+
&& !denot.symbol.is(PackageClass)
142141

143142
/** Find the denotation of enclosing `name` in given context `ctx`.
144143
* @param previous A denotation that was found in a more deeply nested scope,
@@ -161,18 +160,18 @@ class Typer extends Namer
161160
* the previous (inner) definition. This models what scalac does.
162161
*/
163162
def checkNewOrShadowed(found: Type, newPrec: BindingPrec, scala2pkg: Boolean = false)(implicit ctx: Context): Type =
164-
if (!previous.exists || ctx.typeComparer.isSameRef(previous, found)) found
165-
else if ((prevCtx.scope eq ctx.scope) &&
166-
(newPrec == Definition ||
167-
newPrec == NamedImport && prevPrec == WildImport))
163+
if !previous.exists || ctx.typeComparer.isSameRef(previous, found) then
164+
found
165+
else if (prevCtx.scope eq ctx.scope)
166+
&& (newPrec == Definition || newPrec == NamedImport && prevPrec == WildImport)
167+
then
168168
// special cases: definitions beat imports, and named imports beat
169169
// wildcard imports, provided both are in contexts with same scope
170170
found
171-
else {
172-
if (!scala2pkg && !previous.isError && !found.isError)
173-
refctx.error(AmbiguousImport(name, newPrec, prevPrec, prevCtx), posd.sourcePos)
171+
else
172+
if !scala2pkg && !previous.isError && !found.isError then
173+
refctx.error(AmbiguousReference(name, newPrec, prevPrec, prevCtx), posd.sourcePos)
174174
previous
175-
}
176175

177176
/** Recurse in outer context. If final result is same as `previous`, check that it
178177
* is new or shadowed. This order of checking is necessary since an
@@ -192,7 +191,8 @@ class Typer extends Namer
192191
NoType
193192
else
194193
val pre = imp.site
195-
var denot = pre.memberBasedOnFlags(name, required, EmptyFlags).accessibleFrom(pre)(refctx)
194+
var denot = pre.memberBasedOnFlags(name, required, EmptyFlags)
195+
.accessibleFrom(pre)(using refctx)
196196
// Pass refctx so that any errors are reported in the context of the
197197
// reference instead of the context of the import scope
198198
if checkBounds && denot.exists then
@@ -290,11 +290,41 @@ class Typer extends Namer
290290
// with an error on CI which I cannot replicate locally (not even
291291
// with the exact list of files given).
292292
val isNewDefScope =
293-
if (curOwner.is(Package) && !curOwner.isRoot) curOwner ne ctx.outer.owner
294-
else ((ctx.scope ne lastCtx.scope) || (curOwner ne lastCtx.owner)) &&
295-
!isTransparentPackageObject
293+
if curOwner.is(Package) && !curOwner.isRoot then
294+
curOwner ne ctx.outer.owner
295+
else
296+
((ctx.scope ne lastCtx.scope) || (curOwner ne lastCtx.owner))
297+
&& !isTransparentPackageObject
298+
299+
// Does reference `tp` refer only to inherited symbols?
300+
def isInherited(denot: Denotation) =
301+
def isCurrent(mbr: SingleDenotation): Boolean =
302+
!mbr.symbol.exists || mbr.symbol.owner == ctx.owner
303+
denot match
304+
case denot: SingleDenotation => !isCurrent(denot)
305+
case denot => !denot.hasAltWith(isCurrent)
306+
307+
def checkNoOuterDefs(denot: Denotation, last: Context, prevCtx: Context): Unit =
308+
val outer = last.outer
309+
val owner = outer.owner
310+
if (owner eq last.owner) && (outer.scope eq last.scope) then
311+
checkNoOuterDefs(denot, outer, prevCtx)
312+
else if !owner.is(Package) then
313+
val scope = if owner.isClass then owner.info.decls else outer.scope
314+
if scope.lookup(name).exists then
315+
val symsMatch = scope.lookupAll(name).exists(denot.containsSym)
316+
if !symsMatch then
317+
refctx.errorOrMigrationWarning(
318+
AmbiguousReference(name, Definition, Inheritance, prevCtx)(using outer),
319+
posd.sourcePos)
320+
if ctx.scala2CompatMode then
321+
patch(Span(posd.span.start),
322+
if prevCtx.owner == refctx.owner.enclosingClass then "this."
323+
else s"${prevCtx.owner.name}.this.")
324+
else
325+
checkNoOuterDefs(denot, outer, prevCtx)
296326

297-
if (isNewDefScope) {
327+
if isNewDefScope then
298328
val defDenot = ctx.denotNamed(name, required)
299329
if (qualifies(defDenot)) {
300330
val found =
@@ -309,20 +339,22 @@ class Typer extends Namer
309339
curOwner
310340
effectiveOwner.thisType.select(name, defDenot)
311341
}
312-
if (!curOwner.is(Package) || isDefinedInCurrentUnit(defDenot))
342+
if !curOwner.is(Package) || isDefinedInCurrentUnit(defDenot) then
313343
result = checkNewOrShadowed(found, Definition) // no need to go further out, we found highest prec entry
314-
else {
344+
found match
345+
case found: NamedType if ctx.owner.isClass && isInherited(found.denot) =>
346+
checkNoOuterDefs(found.denot, ctx, ctx)
347+
case _ =>
348+
else
315349
if (ctx.scala2CompatMode && !foundUnderScala2.exists)
316350
foundUnderScala2 = checkNewOrShadowed(found, Definition, scala2pkg = true)
317351
if (defDenot.symbol.is(Package))
318352
result = checkNewOrShadowed(previous orElse found, PackageClause)
319353
else if (prevPrec.ordinal < PackageClause.ordinal)
320354
result = findRefRecur(found, PackageClause, ctx)(ctx.outer)
321-
}
322355
}
323-
}
324356

325-
if (result.exists) result
357+
if result.exists then result
326358
else { // find import
327359
val outer = ctx.outer
328360
val curImport = ctx.importInfo

‎compiler/src/dotty/tools/dotc/typer/VarianceChecker.scala

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ object VarianceChecker {
4444
.find(_.name.toTermName == paramName)
4545
.map(_.sourcePos)
4646
.getOrElse(tree.sourcePos)
47-
ctx.error(em"${paramVarianceStr}variant type parameter $paramName occurs in ${occursStr}variant position in ${tl.resType}", pos)
47+
this.ctx.error(em"${paramVarianceStr}variant type parameter $paramName occurs in ${occursStr}variant position in ${tl.resType}", pos)
4848
}
4949
def apply(x: Boolean, t: Type) = x && {
5050
t match {
@@ -115,10 +115,10 @@ class VarianceChecker()(implicit ctx: Context) {
115115
val required = compose(relative, this.variance)
116116
def tvar_s = s"$tvar (${varianceLabel(tvar.flags)} ${tvar.showLocated})"
117117
def base_s = s"$base in ${base.owner}" + (if (base.owner.isClass) "" else " in " + base.owner.enclosingClass)
118-
ctx.log(s"verifying $tvar_s is ${varianceLabel(required)} at $base_s")
119-
ctx.log(s"relative variance: ${varianceLabel(relative)}")
120-
ctx.log(s"current variance: ${this.variance}")
121-
ctx.log(s"owner chain: ${base.ownersIterator.toList}")
118+
this.ctx.log(s"verifying $tvar_s is ${varianceLabel(required)} at $base_s")
119+
this.ctx.log(s"relative variance: ${varianceLabel(relative)}")
120+
this.ctx.log(s"current variance: ${this.variance}")
121+
this.ctx.log(s"owner chain: ${base.ownersIterator.toList}")
122122
if (tvar.isOneOf(required)) None
123123
else Some(VarianceError(tvar, required))
124124
}

‎compiler/src/dotty/tools/dotc/util/SourceFile.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ object ScriptSourceFile {
3333
}
3434
else 0
3535
new SourceFile(file, content drop headerLength) {
36-
override val underlying = new SourceFile(file, content)
36+
override val underlying = new SourceFile(this.file, this.content)
3737
}
3838
}
3939
}

‎compiler/src/dotty/tools/io/ZipArchive.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ final class ManifestResources(val url: URL) extends ZipArchive(null) {
208208
if (!zipEntry.isDirectory) {
209209
val f = new Entry(zipEntry.getName, dir) {
210210
override def lastModified = zipEntry.getTime()
211-
override def input = resourceInputStream(path)
211+
override def input = resourceInputStream(this.path)
212212
override def sizeOption = None
213213
}
214214
dir.entries(f.name) = f

‎tests/neg/ambiref.check

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
-- [E049] Reference Error: tests/neg/ambiref.scala:8:14 ----------------------------------------------------------------
2+
8 | println(x) // error
3+
| ^
4+
| Reference to x is ambiguous,
5+
| it is both defined in object Test
6+
| and inherited subsequently in class D
7+
8+
longer explanation available when compiling with `-explain`
9+
-- [E049] Reference Error: tests/neg/ambiref.scala:10:14 ---------------------------------------------------------------
10+
10 | println(x) // error
11+
| ^
12+
| Reference to x is ambiguous,
13+
| it is both defined in object Test
14+
| and inherited subsequently in anonymous class test1.C {...}
15+
16+
longer explanation available when compiling with `-explain`
17+
-- [E049] Reference Error: tests/neg/ambiref.scala:17:14 ---------------------------------------------------------------
18+
17 | println(y) // error
19+
| ^
20+
| Reference to y is ambiguous,
21+
| it is both defined in method c
22+
| and inherited subsequently in anonymous class D {...}
23+
24+
longer explanation available when compiling with `-explain`
25+
-- [E049] Reference Error: tests/neg/ambiref.scala:25:16 ---------------------------------------------------------------
26+
25 | println(y) // error
27+
| ^
28+
| Reference to y is ambiguous,
29+
| it is both defined in method c
30+
| and inherited subsequently in class E
31+
32+
longer explanation available when compiling with `-explain`

0 commit comments

Comments
 (0)