OpenJDK / jdk / jdk
changeset 58051:39b90cc7f331
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; }}");