Skip to content

Commit

Permalink
8238838: spurious error message for compact constructors with throws …
Browse files Browse the repository at this point in the history 
…clause

Reviewed-by: mcimadamore
  • Loading branch information
Vicente Romero committed Feb 13, 2020
1 parent f844943 commit 4c707c1
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 11 deletions.
16 changes: 9 additions & 7 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1086,15 +1086,15 @@ public void visitMethodDef(JCMethodDecl tree) {
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));
}
}

// 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
Expand All @@ -1104,7 +1104,7 @@ public void visitMethodDef(JCMethodDecl tree) {
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;
}
Expand Down Expand Up @@ -1178,18 +1178,20 @@ public void visitMethodDef(JCMethodDecl tree) {
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)));
}
}
}
Expand Down Expand Up @@ -2227,7 +2229,7 @@ public void visitReturn(JCReturn tree) {
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.
Expand Down
11 changes: 11 additions & 0 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4077,6 +4077,17 @@ else if (annosAfterParams.nonEmpty())
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();
Expand Down
5 changes: 3 additions & 2 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3472,7 +3472,7 @@ compiler.misc.accessor.method.must.not.be.static=\
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})
Expand All @@ -3486,8 +3486,9 @@ compiler.misc.compact=\
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
Expand Down
14 changes: 12 additions & 2 deletions test/langtools/tools/javac/records/RecordCompilationTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,16 +309,26 @@ public void testConstructorRedeclaration() {
"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; }}");
Expand Down

0 comments on commit 4c707c1

Please sign in to comment.