Skip to content

Avoid creating constructors where not warranted #23178

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions compiler/src/dotty/tools/backend/jvm/Primitives.scala
Original file line number Diff line number Diff line change
Expand Up @@ -150,13 +150,12 @@ object Primitives {
case object XOR extends LogicalOp

/** Signals the beginning of a series of concatenations.
* On the JVM platform, it should create a new StringBuffer
*/
* On the JVM platform, it should create a new StringBuilder.
*/
case object StartConcat extends Primitive

/**
* type: (buf) => STR
* jvm : It should turn the StringBuffer into a String.
/** type: (buf) => STR
* jvm : It should turn the StringBuilder into a String.
*/
case object EndConcat extends Primitive

Expand Down
28 changes: 11 additions & 17 deletions compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala
Original file line number Diff line number Diff line change
Expand Up @@ -369,27 +369,21 @@ class ClassfileParser(
* Updates the read pointer of 'in'. */
def parseParents: List[Type] = {
val superType =
if (classRoot.symbol == defn.ComparableClass ||
classRoot.symbol == defn.JavaCloneableClass ||
classRoot.symbol == defn.JavaSerializableClass) {
// Treat these interfaces as universal traits
in.nextChar
val superClass = in.nextChar
// Treat these interfaces as universal traits
if classRoot.symbol == defn.ComparableClass
|| classRoot.symbol == defn.JavaCloneableClass
|| classRoot.symbol == defn.JavaSerializableClass
then
defn.AnyType
}
else
pool.getSuperClass(in.nextChar).typeRef
pool.getSuperClass(superClass).typeRef
val ifaceCount = in.nextChar
var ifaces = for (i <- (0 until ifaceCount).toList) yield pool.getSuperClass(in.nextChar).typeRef
// Dotty deviation: was
// var ifaces = for (i <- List.range(0, ifaceCount)) ...
// This does not typecheck because the type parameter of List is now lower-bounded by Int | Char.
// Consequently, no best implicit for the "Integral" evidence parameter of "range"
// is found. Previously, this worked because of weak conformance, which has been dropped.

val ifaces = List.fill(ifaceCount.toInt):
pool.getSuperClass(in.nextChar).typeRef
superType :: ifaces
}


val result = unpickleOrParseInnerClasses()
if (!result.isDefined) {
var classInfo: Type = TempClassInfoType(parseParents, instanceScope, classRoot.symbol)
Expand All @@ -408,8 +402,8 @@ class ClassfileParser(
moduleRoot.setPrivateWithin(privateWithin)
moduleRoot.sourceModule.setPrivateWithin(privateWithin)

for (i <- 0 until in.nextChar) parseMember(method = false)
for (i <- 0 until in.nextChar) parseMember(method = true)
for (_ <- 0 until in.nextChar) parseMember(method = false)
for (_ <- 0 until in.nextChar) parseMember(method = true)

classRoot.registerCompanion(moduleRoot.symbol)
moduleRoot.registerCompanion(classRoot.symbol)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,22 @@ object Scala2Unpickler {
denot.info = PolyType.fromParams(denot.owner.typeParams, denot.info)
}

def ensureConstructor(cls: ClassSymbol, clsDenot: ClassDenotation, scope: Scope)(using Context): Unit = {
if (scope.lookup(nme.CONSTRUCTOR) == NoSymbol) {
val constr = newDefaultConstructor(cls)
def ensureConstructor(cls: ClassSymbol, clsDenot: ClassDenotation, scope: Scope)(using Context): Unit =
doEnsureConstructor(cls, clsDenot, scope, fromScala2 = true)

private def doEnsureConstructor(cls: ClassSymbol, clsDenot: ClassDenotation, scope: Scope, fromScala2: Boolean)
(using Context): Unit =
if scope.lookup(nme.CONSTRUCTOR) == NoSymbol then
val constr =
if fromScala2 || cls.isAllOf(Trait | JavaDefined) then newDefaultConstructor(cls)
else newConstructor(cls, Private, paramNames = Nil, paramTypes = Nil)
// Scala 2 traits have a constructor iff they have initialization code
// In dotc we represent that as !StableRealizable, which is also owner.is(NoInits)
if clsDenot.flagsUNSAFE.is(Trait) then
constr.setFlag(StableRealizable)
clsDenot.setFlag(NoInits)
addConstructorTypeParams(constr)
cls.enter(constr, scope)
}
}

def setClassInfo(denot: ClassDenotation, info: Type, fromScala2: Boolean, selfInfo: Type = NoType)(using Context): Unit = {
val cls = denot.classSymbol
Expand Down Expand Up @@ -108,7 +112,7 @@ object Scala2Unpickler {
if (tsym.exists) tsym.setFlag(TypeParam)
else denot.enter(tparam, decls)
}
if (!denot.flagsUNSAFE.isAllOf(JavaModule)) ensureConstructor(cls, denot, decls)
if (!denot.flagsUNSAFE.isAllOf(JavaModule)) doEnsureConstructor(cls, denot, decls, fromScala2)

val scalacCompanion = denot.classSymbol.scalacLinkedClass

Expand Down
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/printing/Formatting.scala
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ object Formatting {
given Show[Class[?]] = ShowAny
given Show[Throwable] = ShowAny
given Show[StringBuffer] = ShowAny
given Show[StringBuilder] = ShowAny
given Show[CompilationUnit] = ShowAny
given Show[Phases.Phase] = ShowAny
given Show[TyperState] = ShowAny
Expand Down
6 changes: 3 additions & 3 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3142,18 +3142,18 @@ extends ReferenceMsg(CannotBeAccessedID):
case _ =>
i"none of the overloaded alternatives named $name can"
val where = if (ctx.owner.exists) i" from ${ctx.owner.enclosingClass}" else ""
val whyNot = new StringBuffer
val whyNot = new StringBuilder
for alt <- alts do
val cls = alt.owner.enclosingSubClass
val owner = if cls.exists then cls else alt.owner
val location: String =
if alt.is(Protected) then
if alt.privateWithin.exists && alt.privateWithin != owner then
if owner.is(Final) then alt.privateWithin.showLocated
else alt.privateWithin.showLocated + ", or " + owner.showLocated + " or one of its subclasses"
else s"${alt.privateWithin.showLocated}, or ${owner.showLocated} or one of its subclasses"
else
if owner.is(Final) then owner.showLocated
else owner.showLocated + " or one of its subclasses"
else s"${owner.showLocated} or one of its subclasses"
else
alt.privateWithin.orElse(owner).showLocated
val accessMod = if alt.is(Protected) then "protected" else "private"
Expand Down
16 changes: 16 additions & 0 deletions tests/neg/i15144.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//> using options --release 11
// test: -jvm 17+
// Must be tested where release target < current JVM.
// Maybe a bug that ct.sym is not used if release == JVM version.

import java.time.Instant

class C:
def f: Instant = new Instant // error
def g: Instant = Instant() // error
def p: P = new P // error

class P private ()

@main def test() = println:
C().f
Loading