From a387ef8ca1463855cf4f4a06bfcc13072b212f62 Mon Sep 17 00:00:00 2001 From: Jan Chyb Date: Mon, 10 Mar 2025 10:19:34 +0100 Subject: [PATCH 1/2] Fail compilation if 2 top-level private defs/vals are in the same package --- compiler/src/dotty/tools/dotc/typer/Namer.scala | 12 ++++++------ tests/neg/i22721.check | 4 ++++ tests/neg/i22721/test1.scala | 4 ++++ tests/neg/i22721/test2.scala | 4 ++++ 4 files changed, 18 insertions(+), 6 deletions(-) create mode 100644 tests/neg/i22721.check create mode 100644 tests/neg/i22721/test1.scala create mode 100644 tests/neg/i22721/test2.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 0442c72af6f0..89dc4cf53472 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -134,7 +134,7 @@ class Namer { typer: Typer => * The logic here is very subtle and fragile due to the fact that * we are not allowed to force anything. */ - def checkNoConflict(name: Name, isPrivate: Boolean, span: Span)(using Context): Name = + def checkNoConflict(name: Name, span: Span)(using Context): Name = val owner = ctx.owner var conflictsDetected = false @@ -169,7 +169,7 @@ class Namer { typer: Typer => def preExisting = ctx.effectiveScope.lookup(name) if (!owner.isClass || name.isTypeName) && preExisting.exists then conflict(preExisting) - else if owner.isPackageObject && !isPrivate && name != nme.CONSTRUCTOR then + else if owner.isPackageObject && name != nme.CONSTRUCTOR then checkNoConflictIn(owner.owner) for pkgObj <- pkgObjs(owner.owner) if pkgObj != owner do checkNoConflictIn(pkgObj) @@ -249,7 +249,7 @@ class Namer { typer: Typer => var flags = checkFlags(tree.mods.flags) if ctx.settings.YcompileScala2Library.value then flags |= Scala2x - val name = checkNoConflict(tree.name, flags.is(Private), tree.span).asTypeName + val name = checkNoConflict(tree.name, tree.span).asTypeName val cls = createOrRefine[ClassSymbol](tree, name, flags, ctx.owner, cls => adjustIfModule(new ClassCompleter(cls, tree)(ctx), tree), @@ -258,7 +258,7 @@ class Namer { typer: Typer => cls case tree: MemberDef => var flags = checkFlags(tree.mods.flags) - val name = checkNoConflict(tree.name, flags.is(Private), tree.span) + val name = checkNoConflict(tree.name, tree.span) tree match case tree: ValOrDefDef => if tree.isInstanceOf[ValDef] && !flags.is(Param) && name.endsWith("_=") then @@ -1264,7 +1264,7 @@ class Namer { typer: Typer => val hasDefaults = sym.hasDefaultParams // compute here to ensure HasDefaultParams and NoDefaultParams flags are set val forwarder = if mbr.isType then - val forwarderName = checkNoConflict(alias.toTypeName, isPrivate = false, span) + val forwarderName = checkNoConflict(alias.toTypeName, span) var target = pathType.select(sym) if target.typeParams.nonEmpty then target = target.etaExpand @@ -1320,7 +1320,7 @@ class Namer { typer: Typer => (EmptyFlags, mbrInfo) var mbrFlags = MandatoryExportTermFlags | maybeStable | (sym.flags & RetainedExportTermFlags) if pathMethod.exists then mbrFlags |= ExtensionMethod - val forwarderName = checkNoConflict(alias, isPrivate = false, span) + val forwarderName = checkNoConflict(alias, span) newSymbol(cls, forwarderName, mbrFlags, mbrInfo, coord = span) forwarder.info = avoidPrivateLeaks(forwarder) diff --git a/tests/neg/i22721.check b/tests/neg/i22721.check new file mode 100644 index 000000000000..0797b0a3c2e0 --- /dev/null +++ b/tests/neg/i22721.check @@ -0,0 +1,4 @@ +-- [E161] Naming Error: tests/neg/i22721/test2.scala:3:4 --------------------------------------------------------------- +3 |val foobar: Int = 0 // error + |^^^^^^^^^^^^^^^^^^^ + |foobar is already defined as value foobar in tests/neg/i22721/test1.scala diff --git a/tests/neg/i22721/test1.scala b/tests/neg/i22721/test1.scala new file mode 100644 index 000000000000..b3f1b1fba690 --- /dev/null +++ b/tests/neg/i22721/test1.scala @@ -0,0 +1,4 @@ +package example + +private def foobar: String = "foo" +object test1 { def x = foobar } diff --git a/tests/neg/i22721/test2.scala b/tests/neg/i22721/test2.scala new file mode 100644 index 000000000000..c70a42894dcd --- /dev/null +++ b/tests/neg/i22721/test2.scala @@ -0,0 +1,4 @@ +package example + +private def foobar: Int = 0 // error +object test2 { def x = foobar } From db2eb1925dd13c5982f39e154e4c37f607f657b3 Mon Sep 17 00:00:00 2001 From: Jan Chyb Date: Mon, 10 Mar 2025 12:10:33 +0100 Subject: [PATCH 2/2] Adjust test case --- tests/neg/i22721.check | 10 ++++++---- tests/neg/toplevel-overload/moredefs_1.scala | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/neg/i22721.check b/tests/neg/i22721.check index 0797b0a3c2e0..44b0749f3262 100644 --- a/tests/neg/i22721.check +++ b/tests/neg/i22721.check @@ -1,4 +1,6 @@ --- [E161] Naming Error: tests/neg/i22721/test2.scala:3:4 --------------------------------------------------------------- -3 |val foobar: Int = 0 // error - |^^^^^^^^^^^^^^^^^^^ - |foobar is already defined as value foobar in tests/neg/i22721/test1.scala +-- [E161] Naming Error: tests/neg/i22721/test2.scala:3:12 -------------------------------------------------------------- +3 |private def foobar: Int = 0 // error + |^^^^^^^^^^^^^^^^^^^^^^^^^^^ + |foobar is already defined as method foobar in tests/neg/i22721/test1.scala + | + |Note that overloaded methods must all be defined in the same group of toplevel definitions diff --git a/tests/neg/toplevel-overload/moredefs_1.scala b/tests/neg/toplevel-overload/moredefs_1.scala index 5ba8cfc52078..74cdd3ac1bb6 100644 --- a/tests/neg/toplevel-overload/moredefs_1.scala +++ b/tests/neg/toplevel-overload/moredefs_1.scala @@ -2,4 +2,4 @@ trait B def f(x: B) = s"B" // error: has already been compiled -private def g(): Unit = () // OK, since it is private \ No newline at end of file +private def g(): Unit = () // error: has already been compiled (def is visible in package) \ No newline at end of file