From b314dba5b3174fae2dbe5357c20917fbfbe891a4 Mon Sep 17 00:00:00 2001 From: Angelos Bimpoudis Date: Fri, 7 Feb 2025 10:17:33 +0100 Subject: [PATCH] Improve error reporting around overloading --- .../com/sun/tools/javac/comp/Attr.java | 50 +++++++++++-------- .../tools/javac/resources/compiler.properties | 10 +++- .../NoCompatibleMatcherFoundArity.java | 46 +++++++++++++++++ .../patterns/DeconstructionPatternErrors.out | 10 ++-- .../PrimitivePatternsSwitchErrors.out | 2 +- .../OverloadedPatternDeclarationErrors.java | 9 +++- .../OverloadedPatternDeclarationErrors.out | 7 +-- ...loadedPatternDeclarationsNonTemplated.java | 2 +- 8 files changed, 102 insertions(+), 34 deletions(-) create mode 100644 test/langtools/tools/javac/diags/examples/NoCompatibleMatcherFoundArity.java diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java index fe6892b8b51..3a30748fd70 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java @@ -175,6 +175,8 @@ protected Attr(Context context) { Feature.PATTERN_SWITCH.allowedInSource(source); allowUnconditionalPatternsInstanceOf = Feature.UNCONDITIONAL_PATTERN_IN_INSTANCEOF.allowedInSource(source); + allowPatternDeclarations = preview.isEnabled() && Feature.PATTERN_DECLARATIONS.allowedInSource(source); + sourceName = source.name; useBeforeDeclarationWarning = options.isSet("useBeforeDeclarationWarning"); @@ -204,6 +206,10 @@ protected Attr(Context context) { */ private final boolean allowUnconditionalPatternsInstanceOf; + /** Are pattern declarations allowed + */ + private final boolean allowPatternDeclarations; + /** * Switch: warn about use of variable before declaration? * RFE: 6425594 @@ -4346,24 +4352,36 @@ public void visitRecordPattern(JCRecordPattern tree) { if (site.tsym.kind == Kind.TYP) { int nestedPatternCount = tree.nested.size(); - // Resolve deconstructor call for pattern-use side - // If site refers to a record, then synthesize a MT/MethodSymbol with the signature of the implicitely declared pattern declaration - List patternDeclarations = patternDeclarationCandidatesWithArity(site, nestedPatternCount); + List candidates = candidatesWithArity(site, nestedPatternCount); - if (patternDeclarations.size() >= 1) { + if (candidates.size() >= 1) { List notionalTypes = calculateNotionalTypes(tree); - Symbol resolvedPatternDeclaration = selectBestPatternDeclarationInScope(tree.pos(), site, patternDeclarations, notionalTypes); - - if (resolvedPatternDeclaration != null && resolvedPatternDeclaration.kind != AMBIGUOUS) { - nestedPatternsTargetTypes = types.memberType(site, resolvedPatternDeclaration).getBindingTypes(); - tree.patternDeclaration = (MethodSymbol) resolvedPatternDeclaration; + Symbol resolved = + selectBestPatternDeclarationInScope(tree.pos(), site, candidates, notionalTypes); + + if (resolved != null && resolved.kind != AMBIGUOUS) { + nestedPatternsTargetTypes = types.memberType(site, resolved).getBindingTypes(); + tree.patternDeclaration = (MethodSymbol) resolved; + } else if (resolved != null && resolved.kind == AMBIGUOUS){ + log.error(tree.pos(), Errors.MatcherOverloadingAmbiguity(site.tsym)); + } else { + if (site.tsym instanceof ClassSymbol cs && cs.isRecord() || !allowPatternDeclarations) { + log.error(tree.pos(), Errors.CantApplyDeconstructionPattern(site.tsym)); + } else { + log.error(tree.pos(), Errors.NoCompatibleMatcherFound(site.tsym)); + } + } + } else { + if (site.tsym instanceof ClassSymbol cs && cs.isRecord() || !allowPatternDeclarations) { + log.error(tree.pos(), Errors.CantApplyDeconstructionPattern(site.tsym)); + } else { + log.error(tree.pos(), + Errors.NoCompatibleMatcherFoundArity(site.tsym, String.valueOf(nestedPatternCount))); } } - // TODO: error for classes with non matching arity } if (nestedPatternsTargetTypes == null) { - log.error(tree.pos(), Errors.CantApplyDeconstructionPattern(site.tsym)); nestedPatternsTargetTypes = Stream.generate(() -> types.createErrorType(tree.type)) .limit(tree.nested.size()) .collect(List.collector()); @@ -4456,14 +4474,6 @@ private Symbol selectBestPatternDeclarationInScope(DiagnosticPosition pos, } } - if (bestSoFar != null && bestSoFar.kind == AMBIGUOUS) { - log.error(pos, - Errors.MatcherOverloadingAmbiguity); - } else if (bestSoFar == null) { - log.error(pos, - Errors.NoCompatibleMatcherFound); - } - return bestSoFar; } @@ -4617,7 +4627,7 @@ private boolean isApplicable(List patternDeclarationBindingType, List patternDeclarationCandidatesWithArity(Type site, int nestedPatternCount) { + private List candidatesWithArity(Type site, int nestedPatternCount) { var matchersIt = site.tsym.members() .getSymbols(sym -> sym.isPattern() && sym.type.getBindingTypes().size() == nestedPatternCount) .iterator(); diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties index e55fd1c3ead..a4008a25017 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties @@ -467,11 +467,17 @@ compiler.err.concrete.inheritance.conflict=\ compiler.err.default.allowed.in.intf.annotation.member=\ default value only allowed in an annotation interface declaration +# 0: symbol compiler.err.matcher.overloading.ambiguity=\ - matcher overloading ambiguity + matcher overloading ambiguity for pattern {0} +# 0: symbol compiler.err.no.compatible.matcher.found=\ - no compatible matcher found + no compatible matcher found for pattern {0} + +# 0: symbol, 1: string +compiler.err.no.compatible.matcher.found.arity=\ + no compatible matcher found with arity {1} for pattern {0} # 0: symbol compiler.err.doesnt.exist=\ diff --git a/test/langtools/tools/javac/diags/examples/NoCompatibleMatcherFoundArity.java b/test/langtools/tools/javac/diags/examples/NoCompatibleMatcherFoundArity.java new file mode 100644 index 00000000000..f0e5a251583 --- /dev/null +++ b/test/langtools/tools/javac/diags/examples/NoCompatibleMatcherFoundArity.java @@ -0,0 +1,46 @@ +/* + * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +// key: compiler.err.no.compatible.matcher.found.arity +// key: compiler.misc.feature.pattern.declarations +// key: compiler.warn.preview.feature.use.plural +// options: --enable-preview -source ${jdk.version} -Xlint:preview + +public class NoCompatibleMatcherFoundArity { + private static int test(D o) { + if (o instanceof D(String data, Integer out, Float f)) { + return out; + } + return -1; + } + + public static class D { + public pattern D(Object v1, Float out) { + match D(new Object(), 10.0f); + } + + public pattern D(Float out, Integer v1) { + match D(10.0f, 42); + } + } +} \ No newline at end of file diff --git a/test/langtools/tools/javac/patterns/DeconstructionPatternErrors.out b/test/langtools/tools/javac/patterns/DeconstructionPatternErrors.out index 4d6cb938629..4569af12e5e 100644 --- a/test/langtools/tools/javac/patterns/DeconstructionPatternErrors.out +++ b/test/langtools/tools/javac/patterns/DeconstructionPatternErrors.out @@ -2,11 +2,11 @@ DeconstructionPatternErrors.java:35:37: compiler.err.illegal.start.of.type DeconstructionPatternErrors.java:37:28: compiler.err.illegal.start.of.type DeconstructionPatternErrors.java:39:42: compiler.err.expected: ';' DeconstructionPatternErrors.java:39:43: compiler.err.not.stmt -DeconstructionPatternErrors.java:15:26: compiler.err.no.compatible.matcher.found +DeconstructionPatternErrors.java:15:26: compiler.err.cant.apply.deconstruction.pattern: DeconstructionPatternErrors.P3 DeconstructionPatternErrors.java:16:29: compiler.err.instanceof.reifiable.not.safe: java.lang.Object, java.util.ArrayList -DeconstructionPatternErrors.java:17:26: compiler.err.no.compatible.matcher.found -DeconstructionPatternErrors.java:18:26: compiler.err.no.compatible.matcher.found -DeconstructionPatternErrors.java:19:26: compiler.err.no.compatible.matcher.found +DeconstructionPatternErrors.java:17:26: compiler.err.cant.apply.deconstruction.pattern: DeconstructionPatternErrors.P5 +DeconstructionPatternErrors.java:18:26: compiler.err.cant.apply.deconstruction.pattern: DeconstructionPatternErrors.P +DeconstructionPatternErrors.java:19:26: compiler.err.cant.apply.deconstruction.pattern: DeconstructionPatternErrors.P5 DeconstructionPatternErrors.java:20:26: compiler.err.cant.apply.deconstruction.pattern: DeconstructionPatternErrors.P2 DeconstructionPatternErrors.java:21:26: compiler.err.cant.apply.deconstruction.pattern: DeconstructionPatternErrors.P2 DeconstructionPatternErrors.java:22:26: compiler.err.cant.apply.deconstruction.pattern: DeconstructionPatternErrors.P @@ -15,7 +15,7 @@ DeconstructionPatternErrors.java:24:26: compiler.err.cant.apply.deconstruction.p DeconstructionPatternErrors.java:24:36: compiler.err.cant.resolve.location: kindname.class, Unresolvable, , , (compiler.misc.location: kindname.class, DeconstructionPatternErrors, null) DeconstructionPatternErrors.java:25:13: compiler.err.instanceof.reifiable.not.safe: java.lang.Object, DeconstructionPatternErrors.GenRecord DeconstructionPatternErrors.java:26:29: compiler.err.instanceof.reifiable.not.safe: java.lang.Object, DeconstructionPatternErrors.GenRecord -DeconstructionPatternErrors.java:27:26: compiler.err.no.compatible.matcher.found +DeconstructionPatternErrors.java:27:26: compiler.err.cant.apply.deconstruction.pattern: DeconstructionPatternErrors.GenRecord DeconstructionPatternErrors.java:27:13: compiler.err.instanceof.reifiable.not.safe: java.lang.Object, DeconstructionPatternErrors.GenRecord DeconstructionPatternErrors.java:28:40: compiler.err.match.binding.exists DeconstructionPatternErrors.java:29:56: compiler.err.already.defined: kindname.variable, v1, kindname.method, meth() diff --git a/test/langtools/tools/javac/patterns/PrimitivePatternsSwitchErrors.out b/test/langtools/tools/javac/patterns/PrimitivePatternsSwitchErrors.out index 6a38cd39b60..b5100d7a8c5 100644 --- a/test/langtools/tools/javac/patterns/PrimitivePatternsSwitchErrors.out +++ b/test/langtools/tools/javac/patterns/PrimitivePatternsSwitchErrors.out @@ -1,6 +1,6 @@ PrimitivePatternsSwitchErrors.java:15:18: compiler.err.pattern.dominated PrimitivePatternsSwitchErrors.java:24:18: compiler.err.pattern.dominated -PrimitivePatternsSwitchErrors.java:31:18: compiler.err.no.compatible.matcher.found +PrimitivePatternsSwitchErrors.java:31:18: compiler.err.cant.apply.deconstruction.pattern: PrimitivePatternsSwitchErrors.R_int PrimitivePatternsSwitchErrors.java:70:18: compiler.err.pattern.dominated PrimitivePatternsSwitchErrors.java:78:18: compiler.err.pattern.dominated PrimitivePatternsSwitchErrors.java:84:18: compiler.err.prob.found.req: (compiler.misc.possible.loss.of.precision: long, byte) diff --git a/test/langtools/tools/javac/patterns/declarations/OverloadedPatternDeclarationErrors.java b/test/langtools/tools/javac/patterns/declarations/OverloadedPatternDeclarationErrors.java index b3e928b36c4..45ba166698e 100644 --- a/test/langtools/tools/javac/patterns/declarations/OverloadedPatternDeclarationErrors.java +++ b/test/langtools/tools/javac/patterns/declarations/OverloadedPatternDeclarationErrors.java @@ -10,14 +10,19 @@ static class I1 implements I {} static class I2 implements I {} private static int test(D o) { - if (o instanceof D(String data, Integer out)) { // no compatible matcher found + if (o instanceof D(String data, Integer out)) { // matching arity exists, no applicable overload found return out; } return -1; } private static void test2(D o) { - if (o instanceof D(I data)) { // ambiguity + if (o instanceof D(I data)) { // matching arity exists, multiple applicable exist, ambiguous + } + } + + private static void test3(D o) { + if (o instanceof D(I data, Integer i1, Integer i2)) { // no matching arity matcher exists } } diff --git a/test/langtools/tools/javac/patterns/declarations/OverloadedPatternDeclarationErrors.out b/test/langtools/tools/javac/patterns/declarations/OverloadedPatternDeclarationErrors.out index 34141eb3629..079d05ccb15 100644 --- a/test/langtools/tools/javac/patterns/declarations/OverloadedPatternDeclarationErrors.out +++ b/test/langtools/tools/javac/patterns/declarations/OverloadedPatternDeclarationErrors.out @@ -1,5 +1,6 @@ -OverloadedPatternDeclarationErrors.java:13:26: compiler.err.no.compatible.matcher.found -OverloadedPatternDeclarationErrors.java:20:26: compiler.err.matcher.overloading.ambiguity +OverloadedPatternDeclarationErrors.java:13:26: compiler.err.no.compatible.matcher.found: OverloadedPatternDeclarationErrors.D +OverloadedPatternDeclarationErrors.java:20:26: compiler.err.matcher.overloading.ambiguity: OverloadedPatternDeclarationErrors.D +OverloadedPatternDeclarationErrors.java:25:26: compiler.err.no.compatible.matcher.found.arity: OverloadedPatternDeclarationErrors.D, 3 - compiler.note.preview.filename: OverloadedPatternDeclarationErrors.java, DEFAULT - compiler.note.preview.recompile -2 errors \ No newline at end of file +3 errors \ No newline at end of file diff --git a/test/langtools/tools/javac/patterns/declarations/OverloadedPatternDeclarationsNonTemplated.java b/test/langtools/tools/javac/patterns/declarations/OverloadedPatternDeclarationsNonTemplated.java index 7bb9dacf81f..b4e93c357d0 100644 --- a/test/langtools/tools/javac/patterns/declarations/OverloadedPatternDeclarationsNonTemplated.java +++ b/test/langtools/tools/javac/patterns/declarations/OverloadedPatternDeclarationsNonTemplated.java @@ -207,7 +207,7 @@ pattern R(EF ef, Object o) { compileAndRun(base, source, """ - Test.java:8:17: compiler.err.matcher.overloading.ambiguity + Test.java:8:17: compiler.err.matcher.overloading.ambiguity: test.Test.R - compiler.note.preview.filename: Test.java, DEFAULT - compiler.note.preview.recompile 1 error