OpenJDK / jdk / jdk

changeset 58051:39b90cc7f331

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
8238838: spurious error message for compact constructors with throws clause Reviewed-by: mcimadamore
author vromero
date Thu, 13 Feb 2020 14:13:08 -0500
parents 7ec9c6a6df4f
children b9ff1541faf9
files src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties test/langtools/tools/javac/records/RecordCompilationTests.java
diffstat 4 files changed, 35 insertions(+), 11 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java	Thu Feb 13 10:54:07 2020 -0800
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java	Thu Feb 13 14:13:08 2020 -0500
@@ -1086,7 +1086,7 @@
                                             TreeInfo.name(app.meth) == names._super) &&
                                     checkFirstConstructorStat(app, tree, false)) {
                                 log.error(tree, Errors.InvalidCanonicalConstructorInRecord(
-                                        Fragments.Canonical, tree.sym,
+                                        Fragments.Canonical, tree.sym.name,
                                         Fragments.CanonicalMustNotContainExplicitConstructorInvocation));
                             }
                         }
@@ -1094,7 +1094,7 @@
                         // also we want to check that no type variables have been defined
                         if (!tree.typarams.isEmpty()) {
                             log.error(tree, Errors.InvalidCanonicalConstructorInRecord(
-                                    Fragments.Canonical, tree.sym, Fragments.CanonicalMustNotDeclareTypeVariables));
+                                    Fragments.Canonical, tree.sym.name, Fragments.CanonicalMustNotDeclareTypeVariables));
                         }
 
                         /* and now we need to check that the constructor's arguments are exactly the same as those of the
@@ -1104,7 +1104,7 @@
                         for (JCVariableDecl param: tree.params) {
                             if (!types.isSameType(param.type, recordComponentTypes.head)) {
                                 log.error(param, Errors.InvalidCanonicalConstructorInRecord(
-                                        Fragments.Canonical, tree.sym, Fragments.TypeMustBeIdenticalToCorrespondingRecordComponentType));
+                                        Fragments.Canonical, tree.sym.name, Fragments.TypeMustBeIdenticalToCorrespondingRecordComponentType));
                             }
                             recordComponentTypes = recordComponentTypes.tail;
                         }
@@ -1178,18 +1178,20 @@
                         List<Name> initParamNames = tree.sym.params.map(p -> p.name);
                         if (!initParamNames.equals(recordComponentNames)) {
                             log.error(tree, Errors.InvalidCanonicalConstructorInRecord(
-                                    Fragments.Canonical, env.enclClass.sym, Fragments.CanonicalWithNameMismatch));
+                                    Fragments.Canonical, env.enclClass.sym.name, Fragments.CanonicalWithNameMismatch));
                         }
                         if (!tree.sym.isPublic()) {
                             log.error(tree, Errors.InvalidCanonicalConstructorInRecord(
                                     TreeInfo.isCompactConstructor(tree) ? Fragments.Compact : Fragments.Canonical,
-                                    env.enclClass.sym, Fragments.CanonicalConstructorMustBePublic));
+                                    env.enclClass.sym.name, Fragments.CanonicalConstructorMustBePublic));
                         }
                         if (tree.sym.type.asMethodType().thrown != null && !tree.sym.type.asMethodType().thrown.isEmpty()) {
                             log.error(tree,
                                     Errors.InvalidCanonicalConstructorInRecord(
                                             TreeInfo.isCompactConstructor(tree) ? Fragments.Compact : Fragments.Canonical,
-                                            env.enclClass.sym, Fragments.ThrowsClauseNotAllowedForCanonicalConstructor));
+                                            env.enclClass.sym.name,
+                                            Fragments.ThrowsClauseNotAllowedForCanonicalConstructor(
+                                                    TreeInfo.isCompactConstructor(tree) ? Fragments.Compact : Fragments.Canonical)));
                         }
                     }
                 }
@@ -2227,7 +2229,7 @@
                 env.enclMethod != null &&
                 TreeInfo.isCompactConstructor(env.enclMethod)) {
             log.error(env.enclMethod,
-                    Errors.InvalidCanonicalConstructorInRecord(Fragments.Compact, env.enclMethod.sym, Fragments.CanonicalCantHaveReturnStatement));
+                    Errors.InvalidCanonicalConstructorInRecord(Fragments.Compact, env.enclMethod.sym.name, Fragments.CanonicalCantHaveReturnStatement));
         } else {
             // Attribute return expression, if it exists, and check that
             // it conforms to result type of enclosing method.
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java	Thu Feb 13 10:54:07 2020 -0800
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java	Thu Feb 13 14:13:08 2020 -0500
@@ -4077,6 +4077,17 @@
                     return List.of(methodDeclaratorRest(
                         pos, mods, null, names.init, typarams,
                         isInterface, true, isRecord, dc));
+                } else if (isRecord && type.hasTag(IDENT) && token.kind == THROWS) {
+                    // trying to define a compact constructor with a throws clause
+                    log.error(DiagnosticFlag.SYNTAX, token.pos,
+                            Errors.InvalidCanonicalConstructorInRecord(
+                                    Fragments.Compact,
+                                    className,
+                                    Fragments.ThrowsClauseNotAllowedForCanonicalConstructor(Fragments.Compact)));
+                    skip(false, true, false, false);
+                    return List.of(methodDeclaratorRest(
+                            pos, mods, null, names.init, typarams,
+                            isInterface, true, isRecord, dc));
                 } else {
                     pos = token.pos;
                     Name name = ident();
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties	Thu Feb 13 10:54:07 2020 -0800
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties	Thu Feb 13 14:13:08 2020 -0500
@@ -3472,7 +3472,7 @@
     accessor method must not be static
 
 # canonical constructors
-# 0: fragment, 1: symbol, 2: fragment
+# 0: fragment, 1: name, 2: fragment
 compiler.err.invalid.canonical.constructor.in.record=\
     invalid {0} constructor in record {1}\n\
     ({2})
@@ -3486,8 +3486,9 @@
 compiler.misc.canonical.constructor.must.be.public=\
     canonical constructor must be public
 
+# 0: fragment
 compiler.misc.throws.clause.not.allowed.for.canonical.constructor=\
-    throws clause not allowed for canonical constructor
+    throws clause not allowed for {0} constructor
 
 compiler.misc.canonical.with.name.mismatch=\
     invalid parameter names in canonical constructor
--- a/test/langtools/tools/javac/records/RecordCompilationTests.java	Thu Feb 13 10:54:07 2020 -0800
+++ b/test/langtools/tools/javac/records/RecordCompilationTests.java	Thu Feb 13 14:13:08 2020 -0500
@@ -309,16 +309,26 @@
                         "record R(List<String> list) { # }",
                 "R(List list) { this.list = list; }");
 
-        // ctor should not add checked exceptions
+        // canonical ctor should not throw checked exceptions
         assertFail("compiler.err.invalid.canonical.constructor.in.record",
                    "record R() { # }",
                    "public R() throws Exception { }");
 
-        // not even checked exceptions
+        // same for compact
+        assertFail("compiler.err.invalid.canonical.constructor.in.record",
+                "record R() { # }",
+                "public R throws Exception { }");
+
+        // not even unchecked exceptions
         assertFail("compiler.err.invalid.canonical.constructor.in.record",
                 "record R() { # }",
                  "public R() throws IllegalArgumentException { }");
 
+        // ditto
+        assertFail("compiler.err.invalid.canonical.constructor.in.record",
+                "record R() { # }",
+                "public R throws IllegalArgumentException { }");
+
         // If types match, names must match
         assertFail("compiler.err.invalid.canonical.constructor.in.record",
                    "record R(int x, int y) { public R(int y, int x) { this.x = this.y = 0; }}");
© 2007, 2021 Oracle and/or its affiliates
Terms of Use · Privacy · Trademarks