From 123a951e2763a1fc56f15fb661395141d178efae Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Tue, 20 Jun 2023 16:44:36 +0200 Subject: [PATCH 1/4] Revert "Revert "Include top-level symbols from same file in outer ambiguity error"" This reverts commit ccf2f81b023cbc4d82b207b95ebd3ba65bbe6259. --- .../src/dotty/tools/dotc/typer/Typer.scala | 19 ++++++++++++---- tests/neg/ambiref.check | 16 ++++++++++++++ tests/neg/ambiref.scala | 22 ++++++++++++++++++- tests/pos-special/fatal-warnings/i9260.scala | 2 +- tests/run/protectedacc.scala | 2 +- 5 files changed, 54 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 458e60ddfa38..e42c0eec165f 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -408,11 +408,16 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer // Does reference `tp` refer only to inherited symbols? def isInherited(denot: Denotation) = def isCurrent(mbr: SingleDenotation): Boolean = - !mbr.symbol.exists || mbr.symbol.owner == ctx.owner + !mbr.symbol.exists || mbr.symbol.owner == ctx.owner || ctx.owner.is(Package) denot match case denot: SingleDenotation => !isCurrent(denot) case denot => !denot.hasAltWith(isCurrent) + /* It is an error if an identifier x is available as an inherited member in an inner scope + * and the same name x is defined in an outer scope in the same source file, unless + * the inherited member (has an overloaded alternative that) coincides with + * (an overloaded alternative of) the definition x. + */ def checkNoOuterDefs(denot: Denotation, last: Context, prevCtx: Context): Unit = def sameTermOrType(d1: SingleDenotation, d2: Denotation) = d2.containsSym(d1.symbol) || d2.hasUniqueSym && { @@ -429,9 +434,15 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer val owner = outer.owner if (owner eq last.owner) && (outer.scope eq last.scope) then checkNoOuterDefs(denot, outer, prevCtx) - else if !owner.is(Package) then - val scope = if owner.isClass then owner.info.decls else outer.scope - val competing = scope.denotsNamed(name).filterWithFlags(required, excluded) + else if !owner.isRoot then + val found = + if owner.is(Package) then + owner.denot.asClass.membersNamed(name) + .filterWithPredicate(d => !d.symbol.is(Package) && d.symbol.source == denot.symbol.source) + else + val scope = if owner.isClass then owner.info.decls else outer.scope + scope.denotsNamed(name) + val competing = found.filterWithFlags(required, excluded | Synthetic) if competing.exists then val symsMatch = competing .filterWithPredicate(sd => sameTermOrType(sd, denot)) diff --git a/tests/neg/ambiref.check b/tests/neg/ambiref.check index 5d701b3b3b71..32b4078f1346 100644 --- a/tests/neg/ambiref.check +++ b/tests/neg/ambiref.check @@ -30,3 +30,19 @@ | and inherited subsequently in class E | | longer explanation available when compiling with `-explain` +-- [E049] Reference Error: tests/neg/ambiref.scala:43:10 --------------------------------------------------------------- +43 | println(global) // error + | ^^^^^^ + | Reference to global is ambiguous. + | It is both defined in package + | and inherited subsequently in object D + | + | longer explanation available when compiling with `-explain` +-- [E049] Reference Error: tests/neg/ambiref.scala:49:16 --------------------------------------------------------------- +49 | def t = new T { } // error + | ^ + | Reference to T is ambiguous. + | It is both defined in package p + | and inherited subsequently in class C + | + | longer explanation available when compiling with `-explain` diff --git a/tests/neg/ambiref.scala b/tests/neg/ambiref.scala index e7a5d5efbd7e..bb48997cd465 100644 --- a/tests/neg/ambiref.scala +++ b/tests/neg/ambiref.scala @@ -40,4 +40,24 @@ val global = 0 class C: val global = 1 object D extends C: - println(global) // OK, since global is defined in package \ No newline at end of file + println(global) // error + +package p: + class T + trait P { trait T } + class C extends P: + def t = new T { } // error + +package scala: + trait P { trait Option[+A] } + class C extends P: + def t = new Option[String] { } // OK, competing scala.Option is not defined in the same compilation unit + +object test5: + class Mu // generates a synthetic companion object with an apply method + trait A { + val Mu = 1 + } + trait B extends A { + def t = Mu // don't warn about synthetic companion + } diff --git a/tests/pos-special/fatal-warnings/i9260.scala b/tests/pos-special/fatal-warnings/i9260.scala index df548f393eea..0392c1c96fa8 100644 --- a/tests/pos-special/fatal-warnings/i9260.scala +++ b/tests/pos-special/fatal-warnings/i9260.scala @@ -10,7 +10,7 @@ end AstImpl object untpd extends AstImpl[Null]: - def DefDef(ast: Ast): DefDef = ast match + def DefDef(ast: this.Ast): DefDef = ast match case ast: DefDef => ast end untpd diff --git a/tests/run/protectedacc.scala b/tests/run/protectedacc.scala index a08e7201fd15..85aa3438faa3 100644 --- a/tests/run/protectedacc.scala +++ b/tests/run/protectedacc.scala @@ -134,7 +134,7 @@ package p { abstract class X[T] extends PolyA[T] { - trait Inner extends B { + trait Inner extends this.B { def self: T; def self2: Node; def getB: Inner; From 0b52243e1207556906983166543e2c23006b6069 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Mon, 8 May 2023 22:36:17 -0700 Subject: [PATCH 2/4] Check only existing conflicting def with source --- compiler/src/dotty/tools/dotc/typer/Typer.scala | 6 ++++-- tests/pos/17433/A_1.scala | 3 +++ tests/pos/17433/B_2.scala | 5 +++++ 3 files changed, 12 insertions(+), 2 deletions(-) create mode 100644 tests/pos/17433/A_1.scala create mode 100644 tests/pos/17433/B_2.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index e42c0eec165f..e07c32a5a6ee 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -438,7 +438,9 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer val found = if owner.is(Package) then owner.denot.asClass.membersNamed(name) - .filterWithPredicate(d => !d.symbol.is(Package) && d.symbol.source == denot.symbol.source) + .filterWithPredicate(d => !d.symbol.is(Package) + && denot.symbol.source.exists + && d.symbol.source == denot.symbol.source) else val scope = if owner.isClass then owner.info.decls else outer.scope scope.denotsNamed(name) @@ -479,7 +481,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer result = checkNewOrShadowed(found, Definition) // no need to go further out, we found highest prec entry found match case found: NamedType - if curOwner.isClass && isInherited(found.denot) && !ctx.compilationUnit.isJava => + if curOwner.isClass && found.denot.exists && isInherited(found.denot) && !ctx.compilationUnit.isJava => checkNoOuterDefs(found.denot, ctx, ctx) case _ => else diff --git a/tests/pos/17433/A_1.scala b/tests/pos/17433/A_1.scala new file mode 100644 index 000000000000..575ef4b244e9 --- /dev/null +++ b/tests/pos/17433/A_1.scala @@ -0,0 +1,3 @@ +package p + +object Value diff --git a/tests/pos/17433/B_2.scala b/tests/pos/17433/B_2.scala new file mode 100644 index 000000000000..9a45c6e50d2e --- /dev/null +++ b/tests/pos/17433/B_2.scala @@ -0,0 +1,5 @@ +package p + +object B extends Enumeration { + val A = Value +} From 062d9ca2bf97301b486a3d7f6580563f70abe112 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 9 May 2023 11:18:39 -0700 Subject: [PATCH 3/4] Improve source check of conflicting definition An inherited member cannot shadow a definition in scope that was defined in the current compilation unit. Aliases are given a pass of sorts. Either the member or the definition may be overloaded, which complicates both detection and mitigation. Tests are for the status quo. --- compiler/src/dotty/tools/dotc/typer/Typer.scala | 6 +++--- tests/neg/i17433.check | 8 ++++++++ tests/neg/i17433.scala | 12 ++++++++++++ tests/neg/i17433c.scala | 12 ++++++++++++ tests/neg/i17433d.scala | 13 +++++++++++++ tests/pos/{17433 => i17433a}/A_1.scala | 0 tests/pos/{17433 => i17433a}/B_2.scala | 0 tests/pos/i17433b/p.scala | 7 +++++++ tests/pos/i17433b/q.scala | 6 ++++++ tests/pos/i17433c.scala | 12 ++++++++++++ 10 files changed, 73 insertions(+), 3 deletions(-) create mode 100644 tests/neg/i17433.check create mode 100644 tests/neg/i17433.scala create mode 100644 tests/neg/i17433c.scala create mode 100644 tests/neg/i17433d.scala rename tests/pos/{17433 => i17433a}/A_1.scala (100%) rename tests/pos/{17433 => i17433a}/B_2.scala (100%) create mode 100644 tests/pos/i17433b/p.scala create mode 100644 tests/pos/i17433b/q.scala create mode 100644 tests/pos/i17433c.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index e07c32a5a6ee..6bc696ddb601 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -439,8 +439,8 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer if owner.is(Package) then owner.denot.asClass.membersNamed(name) .filterWithPredicate(d => !d.symbol.is(Package) - && denot.symbol.source.exists - && d.symbol.source == denot.symbol.source) + && d.symbol.source.exists + && isDefinedInCurrentUnit(d)) else val scope = if owner.isClass then owner.info.decls else outer.scope scope.denotsNamed(name) @@ -481,7 +481,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer result = checkNewOrShadowed(found, Definition) // no need to go further out, we found highest prec entry found match case found: NamedType - if curOwner.isClass && found.denot.exists && isInherited(found.denot) && !ctx.compilationUnit.isJava => + if curOwner.isClass && isInherited(found.denot) && !ctx.compilationUnit.isJava => checkNoOuterDefs(found.denot, ctx, ctx) case _ => else diff --git a/tests/neg/i17433.check b/tests/neg/i17433.check new file mode 100644 index 000000000000..7b2bac13b767 --- /dev/null +++ b/tests/neg/i17433.check @@ -0,0 +1,8 @@ +-- [E049] Reference Error: tests/neg/i17433.scala:9:10 ----------------------------------------------------------------- +9 | def g = f(42) // error + | ^ + | Reference to f is ambiguous. + | It is both defined in package + | and inherited subsequently in class D + | + | longer explanation available when compiling with `-explain` diff --git a/tests/neg/i17433.scala b/tests/neg/i17433.scala new file mode 100644 index 000000000000..e500c8226f0f --- /dev/null +++ b/tests/neg/i17433.scala @@ -0,0 +1,12 @@ + +class C: + def f(i: Int) = i + 1 + def f(s: String) = s + "_1" + +def f = 42 + +class D extends C: + def g = f(42) // error + +@main def test() = println: + D().g diff --git a/tests/neg/i17433c.scala b/tests/neg/i17433c.scala new file mode 100644 index 000000000000..ec387a6a6444 --- /dev/null +++ b/tests/neg/i17433c.scala @@ -0,0 +1,12 @@ + +package p: + trait T: + def t(i: Int) = i + 1 + def t(s: String) = s + "_1" + + package object q extends T + + package q: + + class C extends T: + def c = t(42) // error diff --git a/tests/neg/i17433d.scala b/tests/neg/i17433d.scala new file mode 100644 index 000000000000..3cef36e52cac --- /dev/null +++ b/tests/neg/i17433d.scala @@ -0,0 +1,13 @@ + +package p: + trait T: + def t(i: Int) = i + 1 + + package object q extends T: + override def t(i: Int) = i + 2 + def t(s: String) = s + "_2" + + package q: + + class C extends T: + def c = t(42) // error diff --git a/tests/pos/17433/A_1.scala b/tests/pos/i17433a/A_1.scala similarity index 100% rename from tests/pos/17433/A_1.scala rename to tests/pos/i17433a/A_1.scala diff --git a/tests/pos/17433/B_2.scala b/tests/pos/i17433a/B_2.scala similarity index 100% rename from tests/pos/17433/B_2.scala rename to tests/pos/i17433a/B_2.scala diff --git a/tests/pos/i17433b/p.scala b/tests/pos/i17433b/p.scala new file mode 100644 index 000000000000..42313d79f79b --- /dev/null +++ b/tests/pos/i17433b/p.scala @@ -0,0 +1,7 @@ + +package p: + trait T: + def t(i: Int) = i + 1 + def t(s: String) = s + "_1" + + package object q extends T diff --git a/tests/pos/i17433b/q.scala b/tests/pos/i17433b/q.scala new file mode 100644 index 000000000000..3923854ae2f9 --- /dev/null +++ b/tests/pos/i17433b/q.scala @@ -0,0 +1,6 @@ + +package p +package q + + class C extends T: + def c = t(42) diff --git a/tests/pos/i17433c.scala b/tests/pos/i17433c.scala new file mode 100644 index 000000000000..6717375c041d --- /dev/null +++ b/tests/pos/i17433c.scala @@ -0,0 +1,12 @@ + +package p: + trait T: + def t(i: Int) = i + 1 + //def t(s: String) = s + "_1" // it won't try to reconcile the overloaded member + + package object q extends T + + package q: + + class C extends T: + def c = t(42) // should error because this.t is not q.t, but is currently reconciled From 5740455fdcf83bb3eca6f6f10ee981ecd956a466 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Wed, 5 Jul 2023 09:49:02 +0200 Subject: [PATCH 4/4] Package object members are not conflicting with inherited --- .../src/dotty/tools/dotc/typer/Typer.scala | 30 +++++++++------- tests/neg/t12186.scala | 36 +++++++++++++++++++ tests/neg/t12186b/A.scala | 4 +++ tests/neg/t12186b/B.scala | 31 ++++++++++++++++ tests/pos/i17433c.scala | 3 +- tests/{neg => pos}/i17433d.scala | 2 +- .../{neg/i17433c.scala => pos/i17433e.scala} | 2 +- 7 files changed, 91 insertions(+), 17 deletions(-) create mode 100644 tests/neg/t12186.scala create mode 100644 tests/neg/t12186b/A.scala create mode 100644 tests/neg/t12186b/B.scala rename tests/{neg => pos}/i17433d.scala (86%) rename tests/{neg/i17433c.scala => pos/i17433e.scala} (83%) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 6bc696ddb601..3bace406b19e 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -418,7 +418,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer * the inherited member (has an overloaded alternative that) coincides with * (an overloaded alternative of) the definition x. */ - def checkNoOuterDefs(denot: Denotation, last: Context, prevCtx: Context): Unit = + def checkNoOuterDefs(denot: Denotation, ctx: Context, origCtx: Context): Unit = def sameTermOrType(d1: SingleDenotation, d2: Denotation) = d2.containsSym(d1.symbol) || d2.hasUniqueSym && { val sym1 = d1.symbol @@ -430,19 +430,23 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer else (sym1.isAliasType || sym2.isAliasType) && d1.info =:= d2.info } - val outer = last.outer - val owner = outer.owner - if (owner eq last.owner) && (outer.scope eq last.scope) then - checkNoOuterDefs(denot, outer, prevCtx) - else if !owner.isRoot then + val outerCtx = ctx.outer + val outerOwner = outerCtx.owner + if (outerOwner eq ctx.owner) && (outerCtx.scope eq ctx.scope) then + checkNoOuterDefs(denot, outerCtx, origCtx) + else if !outerOwner.isRoot then val found = - if owner.is(Package) then - owner.denot.asClass.membersNamed(name) + if outerOwner.is(Package) then + def notInPackageObject(sym: Symbol) = + sym.owner == outerOwner || // sym.owner.isPackageObject is false if sym is defined in a parent of the package object + sym.owner.isPackageObject && sym.owner.name.endsWith(str.TOPLEVEL_SUFFIX) // top-level definitions + outerOwner.denot.asClass.membersNamed(name) .filterWithPredicate(d => !d.symbol.is(Package) + && notInPackageObject(d.symbol) && d.symbol.source.exists && isDefinedInCurrentUnit(d)) else - val scope = if owner.isClass then owner.info.decls else outer.scope + val scope = if outerOwner.isClass then outerOwner.info.decls else outerCtx.scope scope.denotsNamed(name) val competing = found.filterWithFlags(required, excluded | Synthetic) if competing.exists then @@ -451,14 +455,14 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer .exists if !symsMatch && !suppressErrors then report.errorOrMigrationWarning( - AmbiguousReference(name, Definition, Inheritance, prevCtx)(using outer), + AmbiguousReference(name, Definition, Inheritance, origCtx)(using outerCtx), pos, from = `3.0`) if migrateTo3 then patch(Span(pos.span.start), - if prevCtx.owner == refctx.owner.enclosingClass then "this." - else s"${prevCtx.owner.name}.this.") + if origCtx.owner == refctx.owner.enclosingClass then "this." + else s"${origCtx.owner.name}.this.") else - checkNoOuterDefs(denot, outer, prevCtx) + checkNoOuterDefs(denot, outerCtx, origCtx) if isNewDefScope then val defDenot = ctx.denotNamed(name, required, excluded) diff --git a/tests/neg/t12186.scala b/tests/neg/t12186.scala new file mode 100644 index 000000000000..040ef568f82b --- /dev/null +++ b/tests/neg/t12186.scala @@ -0,0 +1,36 @@ +package object p extends p.U { + def b: Int = 0 + trait Y +} + +package p { + trait U { + def a: Int = 0 + trait X + } + + object c + def c1 = 0 // top-level def + trait Z + trait T { + def a = 1 + def b = 1 + def c = 1 + def c1 = 1 + + trait X + trait Y + trait Z + } + + trait RR extends T { + def m1 = a // ok + def m2 = b // ok + def m3 = c // error + def m4 = c1 // error + + def n1: X // ok + def n2: Y // ok + def n3: Z // error + } +} diff --git a/tests/neg/t12186b/A.scala b/tests/neg/t12186b/A.scala new file mode 100644 index 000000000000..e1b46e235727 --- /dev/null +++ b/tests/neg/t12186b/A.scala @@ -0,0 +1,4 @@ +package object p extends p.U { + def b: Int = 0 + trait Y +} diff --git a/tests/neg/t12186b/B.scala b/tests/neg/t12186b/B.scala new file mode 100644 index 000000000000..84637fdea7d4 --- /dev/null +++ b/tests/neg/t12186b/B.scala @@ -0,0 +1,31 @@ +package p { + trait U { + def a: Int = 0 + trait X + } + + object c + def c1 = 0 // top-level def + trait Z + trait T { + def a = 1 + def b = 1 + def c = 1 + def c1 = 1 + + trait X + trait Y + trait Z + } + + trait RR extends T { + def m1 = a // ok + def m2 = b // ok + def m3 = c // error + def m4 = c1 // error + + def n1: X // ok + def n2: Y // ok + def n3: Z // error + } +} diff --git a/tests/pos/i17433c.scala b/tests/pos/i17433c.scala index 6717375c041d..c091c4637e5a 100644 --- a/tests/pos/i17433c.scala +++ b/tests/pos/i17433c.scala @@ -2,11 +2,10 @@ package p: trait T: def t(i: Int) = i + 1 - //def t(s: String) = s + "_1" // it won't try to reconcile the overloaded member package object q extends T package q: class C extends T: - def c = t(42) // should error because this.t is not q.t, but is currently reconciled + def c = t(42) // OK diff --git a/tests/neg/i17433d.scala b/tests/pos/i17433d.scala similarity index 86% rename from tests/neg/i17433d.scala rename to tests/pos/i17433d.scala index 3cef36e52cac..45179966d015 100644 --- a/tests/neg/i17433d.scala +++ b/tests/pos/i17433d.scala @@ -10,4 +10,4 @@ package p: package q: class C extends T: - def c = t(42) // error + def c = t(42) // OK diff --git a/tests/neg/i17433c.scala b/tests/pos/i17433e.scala similarity index 83% rename from tests/neg/i17433c.scala rename to tests/pos/i17433e.scala index ec387a6a6444..135eb61a3b81 100644 --- a/tests/neg/i17433c.scala +++ b/tests/pos/i17433e.scala @@ -9,4 +9,4 @@ package p: package q: class C extends T: - def c = t(42) // error + def c = t(42) // OK