From 64a8865d222f2285f72347292eab57e708439fc5 Mon Sep 17 00:00:00 2001 From: Yilin Wei Date: Wed, 31 Jan 2024 21:22:33 +0000 Subject: [PATCH 1/6] Add regression test for generic records (#19578) --- tests/pos-java16+/java-records/FromScala.scala | 4 ++++ tests/pos-java16+/java-records/R4.java | 1 + 2 files changed, 5 insertions(+) create mode 100644 tests/pos-java16+/java-records/R4.java diff --git a/tests/pos-java16+/java-records/FromScala.scala b/tests/pos-java16+/java-records/FromScala.scala index 67747e658432..bab8eec99195 100644 --- a/tests/pos-java16+/java-records/FromScala.scala +++ b/tests/pos-java16+/java-records/FromScala.scala @@ -41,3 +41,7 @@ object C: val l2: Long = r3.l(43L, 44L) // supertype val isRecord: java.lang.Record = r3 + + def useR4: Unit = + val r4 = R4(1) + val i: Int = r4.t diff --git a/tests/pos-java16+/java-records/R4.java b/tests/pos-java16+/java-records/R4.java new file mode 100644 index 000000000000..b8a415480afd --- /dev/null +++ b/tests/pos-java16+/java-records/R4.java @@ -0,0 +1 @@ +public record R4(T t) {} From f7eb589c1a331cbe74256124bd5d0a43efde11a3 Mon Sep 17 00:00:00 2001 From: Yilin Wei Date: Thu, 1 Feb 2024 18:44:43 +0000 Subject: [PATCH 2/6] Fix records with type parameters (#19578). This fixes a whole host of subtle issues. - The type parameter was not stamped correctly on the constructor causing the original error - The parsed record was not stamped with `JavaDefined`, which meant the duplicate constructors in the case of overrides were not removed. --- compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala | 4 ++-- compiler/src/dotty/tools/dotc/typer/Namer.scala | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala b/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala index f7ef86ee5cde..5bc2e2210874 100644 --- a/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala @@ -860,7 +860,7 @@ object JavaParsers { // generate the canonical constructor val canonicalConstructor = - DefDef(nme.CONSTRUCTOR, joinParams(tparams, List(header)), TypeTree(), EmptyTree) + DefDef(nme.CONSTRUCTOR, joinParams(Nil, List(header)), TypeTree(), EmptyTree) .withMods(Modifiers(Flags.JavaDefined | Flags.Synthetic, mods.privateWithin)) // return the trees @@ -872,7 +872,7 @@ object JavaParsers { tparams = tparams, needsDummyConstr = true ) - ).withMods(mods) + ).withMods(mods.withFlags(Flags.JavaDefined | Flags.Final)) } addCompanionObject(statics, recordTypeDef) end recordDecl diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 819b43fcec2c..9abb8e7c89e4 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -879,6 +879,10 @@ class Namer { typer: Typer => * with a user-defined method in the same scope with a matching type. */ private def invalidateIfClashingSynthetic(denot: SymDenotation): Unit = + + def isJavaRecord(owner: Symbol) = + owner.is(JavaDefined) && owner.derivesFrom(defn.JavaRecordClass) + def isCaseClassOrCompanion(owner: Symbol) = owner.isClass && { if (owner.is(Module)) owner.linkedClass.is(CaseClass) @@ -904,7 +908,7 @@ class Namer { typer: Typer => || // remove synthetic constructor of a java Record if it clashes with a non-synthetic constructor (denot.isConstructor - && denot.owner.is(JavaDefined) && denot.owner.derivesFrom(defn.JavaRecordClass) + && isJavaRecord(denot.owner) && denot.owner.unforcedDecls.lookupAll(denot.name).exists(c => c != denot.symbol && c.info.matches(denot.info)) ) ) From 781c25fbd9515e48d3e288992359a8fd016d451c Mon Sep 17 00:00:00 2001 From: Yilin Wei Date: Thu, 1 Feb 2024 18:56:30 +0000 Subject: [PATCH 3/6] Add arity 0 regression test for records. --- tests/pos-java16+/java-records/FromScala.scala | 4 ++++ tests/pos-java16+/java-records/R0.java | 1 + 2 files changed, 5 insertions(+) create mode 100644 tests/pos-java16+/java-records/R0.java diff --git a/tests/pos-java16+/java-records/FromScala.scala b/tests/pos-java16+/java-records/FromScala.scala index bab8eec99195..8654f88f1835 100644 --- a/tests/pos-java16+/java-records/FromScala.scala +++ b/tests/pos-java16+/java-records/FromScala.scala @@ -1,4 +1,8 @@ object C: + + def useR0: Unit = + val r = R0() + def useR1: Unit = // constructor signature val r = R1(123, "hello") diff --git a/tests/pos-java16+/java-records/R0.java b/tests/pos-java16+/java-records/R0.java new file mode 100644 index 000000000000..ad43a2b22673 --- /dev/null +++ b/tests/pos-java16+/java-records/R0.java @@ -0,0 +1 @@ +public record R0() {} From 47511ae643517a170a1688239613d0dfee7b146f Mon Sep 17 00:00:00 2001 From: Yilin Wei Date: Thu, 1 Feb 2024 21:44:32 +0000 Subject: [PATCH 4/6] Fixes record accessors incorrect signature (#19386). We change the proxy method to take in a single argument. This also exposed a second issue where the overriden methods were not invalidated correctly in the `Namer`. --- compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala | 2 +- compiler/src/dotty/tools/dotc/typer/Namer.scala | 6 +++--- compiler/src/dotty/tools/dotc/typer/Typer.scala | 4 ++-- tests/pos-java16+/i19386/FromScala.scala | 2 ++ tests/pos-java16+/i19386/R1.java | 1 + 5 files changed, 9 insertions(+), 6 deletions(-) create mode 100644 tests/pos-java16+/i19386/FromScala.scala create mode 100644 tests/pos-java16+/i19386/R1.java diff --git a/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala b/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala index 5bc2e2210874..e98ff6c9d66d 100644 --- a/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala @@ -854,7 +854,7 @@ object JavaParsers { val accessors = (for (name, (tpt, annots)) <- fieldsByName yield - DefDef(name, Nil, tpt, unimplementedExpr) + DefDef(name, List(Nil), tpt, unimplementedExpr) .withMods(Modifiers(Flags.JavaDefined | Flags.Method | Flags.Synthetic)) ).toList diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 9abb8e7c89e4..1f9f06874818 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -906,9 +906,9 @@ class Namer { typer: Typer => && (definesMember || inheritsConcreteMember) ) || - // remove synthetic constructor of a java Record if it clashes with a non-synthetic constructor - (denot.isConstructor - && isJavaRecord(denot.owner) + // remove synthetic constructor or method of a java Record if it clashes with a non-synthetic constructor + (isJavaRecord(denot.owner) + && (denot.isConstructor || definesMember) && denot.owner.unforcedDecls.lookupAll(denot.name).exists(c => c != denot.symbol && c.info.matches(denot.info)) ) ) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index c0b5ea19ec23..580438bad7c4 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2548,10 +2548,10 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer def canBeInvalidated(sym: Symbol): Boolean = sym.is(Synthetic) && (desugar.isRetractableCaseClassMethodName(sym.name) || - (sym.isConstructor && sym.owner.derivesFrom(defn.JavaRecordClass))) + sym.owner.derivesFrom(defn.JavaRecordClass)) if !sym.info.exists then - // it's a discarded method (synthetic case class method or synthetic java record constructor), drop it + // it's a discarded method (synthetic case class method or synthetic java record constructor or overriden member), drop it assert(canBeInvalidated(sym)) sym.owner.info.decls.openForMutations.unlink(sym) return EmptyTree diff --git a/tests/pos-java16+/i19386/FromScala.scala b/tests/pos-java16+/i19386/FromScala.scala new file mode 100644 index 000000000000..82b5835753a1 --- /dev/null +++ b/tests/pos-java16+/i19386/FromScala.scala @@ -0,0 +1,2 @@ +def test(r: R1): Unit = + val i: Int = r.i() diff --git a/tests/pos-java16+/i19386/R1.java b/tests/pos-java16+/i19386/R1.java new file mode 100644 index 000000000000..40837040d659 --- /dev/null +++ b/tests/pos-java16+/i19386/R1.java @@ -0,0 +1 @@ +public record R1(int i) {} From c532dd7de06624a54394eb2f4fe63f847c18ed44 Mon Sep 17 00:00:00 2001 From: Yilin Wei Date: Fri, 2 Feb 2024 10:28:16 +0000 Subject: [PATCH 5/6] Add extra predicate check in `canBeInvalidated` --- compiler/src/dotty/tools/dotc/typer/Typer.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 580438bad7c4..44ab5441c7c9 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2548,7 +2548,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer def canBeInvalidated(sym: Symbol): Boolean = sym.is(Synthetic) && (desugar.isRetractableCaseClassMethodName(sym.name) || - sym.owner.derivesFrom(defn.JavaRecordClass)) + (sym.owner.is(JavaDefined) && sym.owner.derivesFrom(defn.JavaRecordClass))) if !sym.info.exists then // it's a discarded method (synthetic case class method or synthetic java record constructor or overriden member), drop it From 5be6faca38ff808b01e6bb410c1a922def44e9a7 Mon Sep 17 00:00:00 2001 From: Yilin Wei Date: Fri, 2 Feb 2024 16:33:19 +0000 Subject: [PATCH 6/6] Remove redundant `definesMember` check. --- compiler/src/dotty/tools/dotc/typer/Namer.scala | 2 +- compiler/src/dotty/tools/dotc/typer/Typer.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 1f9f06874818..1e351067debf 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -908,7 +908,7 @@ class Namer { typer: Typer => || // remove synthetic constructor or method of a java Record if it clashes with a non-synthetic constructor (isJavaRecord(denot.owner) - && (denot.isConstructor || definesMember) + && denot.is(Method) && denot.owner.unforcedDecls.lookupAll(denot.name).exists(c => c != denot.symbol && c.info.matches(denot.info)) ) ) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 44ab5441c7c9..d51f51ea335a 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2548,7 +2548,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer def canBeInvalidated(sym: Symbol): Boolean = sym.is(Synthetic) && (desugar.isRetractableCaseClassMethodName(sym.name) || - (sym.owner.is(JavaDefined) && sym.owner.derivesFrom(defn.JavaRecordClass))) + (sym.owner.is(JavaDefined) && sym.owner.derivesFrom(defn.JavaRecordClass) && sym.is(Method))) if !sym.info.exists then // it's a discarded method (synthetic case class method or synthetic java record constructor or overriden member), drop it