From 1272b52d190a8eb90d2579d47007311ca31ee6d2 Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Thu, 12 Jun 2025 14:01:05 -0500 Subject: [PATCH] Fix various starting position bugs relating to implicitly typed variables. --- .../com/sun/tools/javac/comp/Attr.java | 7 +- .../sun/tools/javac/parser/JavacParser.java | 27 +++--- .../com/sun/tools/javac/tree/JCTree.java | 27 ++++-- .../com/sun/tools/javac/tree/Pretty.java | 5 +- .../com/sun/tools/javac/tree/TreeCopier.java | 2 +- .../com/sun/tools/javac/tree/TreeInfo.java | 13 +-- .../com/sun/tools/javac/tree/TreeMaker.java | 5 +- .../javac/parser/DeclarationEndPositions.java | 92 +++++++++++++++---- .../tools/javac/patterns/PrettyTest.java | 10 +- test/langtools/tools/javac/tree/VarTree.java | 6 ++ .../tools/javac/tree/VarWarnPosition.java | 3 + .../tools/javac/tree/VarWarnPosition.out | 4 +- 12 files changed, 141 insertions(+), 60 deletions(-) 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 9c9672151c7..3e61de2ded0 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 @@ -1264,14 +1264,14 @@ public class Attr extends JCTree.Visitor { if (tree.init == null) { //cannot use 'var' without initializer log.error(tree, Errors.CantInferLocalVarType(tree.name, Fragments.LocalMissingInit)); - tree.vartype = make.Erroneous(); + tree.vartype = make.at(tree.pos()).Erroneous(); } else { Fragment msg = canInferLocalVarType(tree); if (msg != null) { //cannot use 'var' with initializer which require an explicit target //(e.g. lambda, method reference, array initializer). log.error(tree, Errors.CantInferLocalVarType(tree.name, msg)); - tree.vartype = make.Erroneous(); + tree.vartype = make.at(tree.pos()).Erroneous(); } } } @@ -5717,6 +5717,9 @@ public class Attr extends JCTree.Visitor { private void setSyntheticVariableType(JCVariableDecl tree, Type type) { if (type.isErroneous()) { tree.vartype = make.at(tree.pos()).Erroneous(); + } else if (tree.declaredUsingVar()) { + Assert.check(tree.typePos != Position.NOPOS); + tree.vartype = make.at(tree.typePos).Type(type); } else { tree.vartype = make.at(tree.pos()).Type(type); } diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java index 7ebcd0c844b..bb29bcbfbcb 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java @@ -1005,10 +1005,12 @@ public class JavacParser implements Parser { pattern = toP(F.at(token.pos).AnyPattern()); } else { + int varTypePos = Position.NOPOS; if (parsedType == null) { boolean var = token.kind == IDENTIFIER && token.name() == names.var; e = unannotatedType(allowVar, TYPE | NOLAMBDA); if (var) { + varTypePos = e.pos; e = null; } } else { @@ -1046,9 +1048,10 @@ public class JavacParser implements Parser { if (Feature.UNNAMED_VARIABLES.allowedInSource(source) && name == names.underscore) { name = names.empty; } - JCVariableDecl var = toP(F.at(varPos).VarDef(mods, name, e, null)); + JCVariableDecl var = toP(F.at(varPos).VarDef(mods, name, e, null, + varTypePos != Position.NOPOS ? JCVariableDecl.DeclKind.VAR : JCVariableDecl.DeclKind.EXPLICIT, + varTypePos)); if (e == null) { - var.startPos = pos; if (var.name == names.underscore && !allowVar) { log.error(DiagnosticFlag.SYNTAX, varPos, Errors.UseOfUnderscoreNotAllowed); } @@ -2190,7 +2193,8 @@ public class JavacParser implements Parser { if (param.vartype != null && restrictedTypeName(param.vartype, true) != null) { checkSourceLevel(param.pos, Feature.VAR_SYNTAX_IMPLICIT_LAMBDAS); - param.startPos = TreeInfo.getStartPos(param.vartype); + param.declKind = JCVariableDecl.DeclKind.VAR; + param.typePos = TreeInfo.getStartPos(param.vartype); param.vartype = null; } } @@ -3830,7 +3834,7 @@ public class JavacParser implements Parser { syntaxError(token.pos, Errors.Expected(EQ)); } - int startPos = Position.NOPOS; + int varTypePos = Position.NOPOS; JCTree elemType = TreeInfo.innermostType(type, true); if (elemType.hasTag(IDENT)) { Name typeName = ((JCIdent) elemType).name; @@ -3842,19 +3846,17 @@ public class JavacParser implements Parser { reportSyntaxError(elemType.pos, Errors.RestrictedTypeNotAllowedArray(typeName)); } else { declaredUsingVar = true; + varTypePos = elemType.pos; if (compound) //error - 'var' in compound local var decl reportSyntaxError(elemType.pos, Errors.RestrictedTypeNotAllowedCompound(typeName)); - startPos = TreeInfo.getStartPos(mods); - if (startPos == Position.NOPOS) - startPos = TreeInfo.getStartPos(type); //implicit type type = null; } } } - JCVariableDecl result = toP(F.at(pos).VarDef(mods, name, type, init, declaredUsingVar)); - result.startPos = startPos; + JCVariableDecl result = toP(F.at(pos).VarDef(mods, name, type, init, + declaredUsingVar ? JCVariableDecl.DeclKind.VAR : JCVariableDecl.DeclKind.EXPLICIT, varTypePos)); return attach(result, dc); } @@ -3968,8 +3970,11 @@ public class JavacParser implements Parser { name = names.empty; } - return toP(F.at(pos).VarDef(mods, name, type, null, - type != null && type.hasTag(IDENT) && ((JCIdent)type).name == names.var)); + boolean declaredUsingVar = type != null && type.hasTag(IDENT) && ((JCIdent)type).name == names.var; + JCVariableDecl.DeclKind declKind = declaredUsingVar ? JCVariableDecl.DeclKind.VAR : + type != null ? JCVariableDecl.DeclKind.EXPLICIT : JCVariableDecl.DeclKind.IMPLICIT; + int typePos = type != null ? type.pos : pos; + return toP(F.at(pos).VarDef(mods, name, type, null, declKind, typePos)); } /** Resources = Resource { ";" Resources } diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java index debc10c9ff8..0436f68b9e3 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java @@ -1002,6 +1002,13 @@ public abstract class JCTree implements Tree, Cloneable, DiagnosticPosition { * A variable definition. */ public static class JCVariableDecl extends JCStatement implements VariableTree { + + public enum DeclKind { + EXPLICIT, // "SomeType name" + IMPLICIT, // "name" + VAR, // "var name" + } + /** variable modifiers */ public JCModifiers mods; /** variable name */ @@ -1014,17 +1021,17 @@ public abstract class JCTree implements Tree, Cloneable, DiagnosticPosition { public JCExpression init; /** symbol */ public VarSymbol sym; - /** explicit start pos */ - public int startPos = Position.NOPOS; - /** declared using `var` */ - private boolean declaredUsingVar; + /** how the variable's type was declared */ + public DeclKind declKind; + /** a source code position to use for "vartype" when null (can happen if declKind != EXPLICIT) */ + public int typePos; protected JCVariableDecl(JCModifiers mods, Name name, JCExpression vartype, JCExpression init, VarSymbol sym) { - this(mods, name, vartype, init, sym, false); + this(mods, name, vartype, init, sym, DeclKind.EXPLICIT, Position.NOPOS); } protected JCVariableDecl(JCModifiers mods, @@ -1032,19 +1039,21 @@ public abstract class JCTree implements Tree, Cloneable, DiagnosticPosition { JCExpression vartype, JCExpression init, VarSymbol sym, - boolean declaredUsingVar) { + DeclKind declKind, + int typePos) { this.mods = mods; this.name = name; this.vartype = vartype; this.init = init; this.sym = sym; - this.declaredUsingVar = declaredUsingVar; + this.declKind = declKind; + this.typePos = typePos; } protected JCVariableDecl(JCModifiers mods, JCExpression nameexpr, JCExpression vartype) { - this(mods, null, vartype, null, null, false); + this(mods, null, vartype, null, null, DeclKind.EXPLICIT, Position.NOPOS); this.nameexpr = nameexpr; if (nameexpr.hasTag(Tag.IDENT)) { this.name = ((JCIdent)nameexpr).name; @@ -1059,7 +1068,7 @@ public abstract class JCTree implements Tree, Cloneable, DiagnosticPosition { } public boolean declaredUsingVar() { - return declaredUsingVar; + return declKind == DeclKind.VAR; } @Override diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/Pretty.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/Pretty.java index 919b2325ef6..d953663a6d7 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/Pretty.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/Pretty.java @@ -724,7 +724,10 @@ public class Pretty extends JCTree.Visitor { print("... "); print(tree.name); } else { - printExpr(tree.vartype); + if (tree.vartype == null && tree.declaredUsingVar()) + print("var"); + else + printExpr(tree.vartype); print(' '); if (tree.name.isEmpty()) { print('_'); diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeCopier.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeCopier.java index 9c3ed3bbcd2..9efc6a9d895 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeCopier.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeCopier.java @@ -551,7 +551,7 @@ public class TreeCopier

implements TreeVisitor { JCExpression vartype = copy(t.vartype, p); if (t.nameexpr == null) { JCExpression init = copy(t.init, p); - return M.at(t.pos).VarDef(mods, t.name, vartype, init); + return M.at(t.pos).VarDef(mods, t.name, vartype, init, t.declKind, t.typePos); } else { JCExpression nameexpr = copy(t.nameexpr, p); return M.at(t.pos).ReceiverVarDef(mods, nameexpr, vartype); diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java index ae946c5b6d6..a66dcaad851 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java @@ -610,17 +610,14 @@ public class TreeInfo { } case VARDEF: { JCVariableDecl node = (JCVariableDecl)tree; - if (node.startPos != Position.NOPOS) { - return node.startPos; - } else if (node.mods.pos != Position.NOPOS) { + if (node.mods.pos != Position.NOPOS) { return node.mods.pos; - } else if (node.vartype == null || node.vartype.pos == Position.NOPOS) { - //if there's no type (partially typed lambda parameter) - //simply return node position - return node.pos; - } else { + } else if (node.vartype != null) { return getStartPos(node.vartype); + } else if (node.typePos != Position.NOPOS) { + return node.typePos; } + break; } case BINDINGPATTERN: { JCBindingPattern node = (JCBindingPattern)tree; diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeMaker.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeMaker.java index 0d0852cb91b..0e71df99bdc 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeMaker.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeMaker.java @@ -237,8 +237,9 @@ public class TreeMaker implements JCTree.Factory { return tree; } - public JCVariableDecl VarDef(JCModifiers mods, Name name, JCExpression vartype, JCExpression init, boolean declaredUsingVar) { - JCVariableDecl tree = new JCVariableDecl(mods, name, vartype, init, null, declaredUsingVar); + public JCVariableDecl VarDef(JCModifiers mods, Name name, JCExpression vartype, JCExpression init, + JCVariableDecl.DeclKind declKind, int typePos) { + JCVariableDecl tree = new JCVariableDecl(mods, name, vartype, init, null, declKind, typePos); tree.pos = pos; return tree; } diff --git a/test/langtools/tools/javac/parser/DeclarationEndPositions.java b/test/langtools/tools/javac/parser/DeclarationEndPositions.java index 473d6bc2712..492dccfe2e8 100644 --- a/test/langtools/tools/javac/parser/DeclarationEndPositions.java +++ b/test/langtools/tools/javac/parser/DeclarationEndPositions.java @@ -49,7 +49,7 @@ import javax.tools.ToolProvider; public class DeclarationEndPositions { - public static void checkEndPosition(Class nodeType, String input, String marker) throws IOException { + public static void checkPositions(Class nodeType, String input, String markers) throws IOException { // Create source var source = new SimpleJavaFileObject(URI.create("file://T.java"), JavaFileObject.Kind.SOURCE) { @@ -71,11 +71,26 @@ public class DeclarationEndPositions { public Void scan(Tree node, Void aVoid) { if (nodeType.isInstance(node)) { JCTree tree = (JCTree)node; - int actual = TreeInfo.getEndPos(tree, unit.endPositions); - int expected = marker.indexOf('^') + 1; - if (actual != expected) { + + // Verify declaration start and end positions + int start = tree.getStartPosition(); + if (markers.charAt(start) != '<') { throw new AssertionError(String.format( - "wrong end pos %d != %d for \"%s\" @ %d", actual, expected, input, tree.pos)); + "wrong %s pos %d for \"%s\" in \"%s\"", "start", start, tree, input)); + } + int end = TreeInfo.getEndPos(tree, unit.endPositions); + if (markers.charAt(end - 1) != '>') { + throw new AssertionError(String.format( + "wrong %s pos %d for \"%s\" in \"%s\"", "end", end, tree, input)); + } + + // For variable declarations using "var", verify the "var" position + if (tree instanceof JCVariableDecl varDecl && varDecl.declaredUsingVar()) { + int vpos = varDecl.typePos; + if (!input.substring(vpos).startsWith("var")) { + throw new AssertionError(String.format( + "wrong %s pos %d for \"%s\" in \"%s\"", "var", vpos, tree, input)); + } } } return super.scan(node, aVoid); @@ -86,34 +101,71 @@ public class DeclarationEndPositions { public static void main(String... args) throws Exception { // JCModuleDecl - checkEndPosition(JCModuleDecl.class, + checkPositions(JCModuleDecl.class, "/* comment */ module fred { /* comment */ } /* comment */", - " ^ "); + " <---------------------------> "); // JCPackageDecl - checkEndPosition(JCPackageDecl.class, + checkPositions(JCPackageDecl.class, "/* comment */ package fred; /* comment */", - " ^ "); + " <-----------> "); // JCClassDecl - checkEndPosition(JCClassDecl.class, + checkPositions(JCClassDecl.class, "/* comment */ class Fred { /* comment */ } /* comment */", - " ^ "); + " <--------------------------> "); // JCMethodDecl - checkEndPosition(JCMethodDecl.class, + checkPositions(JCMethodDecl.class, "/* comment */ class Fred { void m() { /* comment */ } } /* comment */", - " ^ "); + " <------------------------> "); // JCVariableDecl - checkEndPosition(JCVariableDecl.class, + checkPositions(JCVariableDecl.class, "/* comment */ class Fred { int x; } /* comment */", - " ^ "); - checkEndPosition(JCVariableDecl.class, + " <----> "); + checkPositions(JCVariableDecl.class, "/* comment */ class Fred { int x = 123; } /* comment */", - " ^ "); - checkEndPosition(JCVariableDecl.class, - "/* comment */ class A { try {} catch (Error err) {} } /* comment */", - " ^ "); + " <----------> "); + checkPositions(JCVariableDecl.class, + "/* comment */ class Fred { final int x = 123; } /* comment */", + " <----------------> "); + checkPositions(JCVariableDecl.class, + "/* comment */ class Fred { final int x = 123, y = 456; } /* comment */", + " <---------------->--------> "); + checkPositions(JCVariableDecl.class, + "/* comment */ class A { void m() { try {} catch (Error err) {} } } /* comment */", + " <-------> "); + + // JCVariableDecl with "var" declarations + checkPositions(JCVariableDecl.class, + "class A { void m() { var foo; } }", + " <------> "); + checkPositions(JCVariableDecl.class, + "class A { void m() { var foo = 42; } }", + " <-----------> "); + checkPositions(JCVariableDecl.class, + "class A { void m() { final var foo = 42; } }", + " <-----------------> "); + + checkPositions(JCVariableDecl.class, + "class A { void m() { java.util.function.Consumer = foo -> { } } }", + " <-> "); + checkPositions(JCVariableDecl.class, + "class A { void m() { java.util.function.Consumer = (foo) -> { } } }", + " <-> "); + checkPositions(JCVariableDecl.class, + "class A { void m() { java.util.function.Consumer = (var foo) -> { } } }", + " <-----> "); + checkPositions(JCVariableDecl.class, + "class A { void m() { java.util.function.Consumer = (final var foo) -> { } } }", + " <-----------> "); + + checkPositions(JCVariableDecl.class, + "class A { record R(int x) { } void m() { switch (null) { case R(var x) -> {} default -> {} } } }", + " <---> <---> "); + checkPositions(JCVariableDecl.class, + "class A { record R(int x) { } void m() { switch (null) { case R(final var x) -> {} default -> {} } } }", + " <---> <---------> "); } } diff --git a/test/langtools/tools/javac/patterns/PrettyTest.java b/test/langtools/tools/javac/patterns/PrettyTest.java index 5425127e91b..5f9c74b760e 100644 --- a/test/langtools/tools/javac/patterns/PrettyTest.java +++ b/test/langtools/tools/javac/patterns/PrettyTest.java @@ -72,12 +72,12 @@ public class PrettyTest { boolean _ = true; b = o instanceof String s; b = o instanceof R(String s); - b = o instanceof R(/*missing*/ s); - b = o instanceof R2(R(/*missing*/ s), String t); - b = o instanceof R2(R(/*missing*/ s), /*missing*/ t); + b = o instanceof R(var s); + b = o instanceof R2(R(var s), String t); + b = o instanceof R2(R(var s), var t); b = o instanceof R(String _); - b = o instanceof R2(R(/*missing*/ _), /*missing*/ _); - b = o instanceof R2(R(_), /*missing*/ t); + b = o instanceof R2(R(var _), var _); + b = o instanceof R2(R(_), var t); } \n\ class R { diff --git a/test/langtools/tools/javac/tree/VarTree.java b/test/langtools/tools/javac/tree/VarTree.java index 5ce7ec07541..2c70d5eb317 100644 --- a/test/langtools/tools/javac/tree/VarTree.java +++ b/test/langtools/tools/javac/tree/VarTree.java @@ -67,6 +67,12 @@ public class VarTree { "java.lang.String testVar"); test.run("java.util.function.Consumer c = (|var testVar|) -> {};", "java.lang.String testVar"); + test.run("java.util.function.Consumer c = (|final var testVar|) -> {};", + "final java.lang.String testVar"); + test.run("record Rec(int x) { }; switch (null) { case Rec(|var testVar|) -> {} default -> {} };", + "int testVar"); + test.run("record Rec(int x) { }; switch (null) { case Rec(|final var testVar|) -> {} default -> {} };", + "final int testVar"); } void run(String code, String expected) throws IOException { diff --git a/test/langtools/tools/javac/tree/VarWarnPosition.java b/test/langtools/tools/javac/tree/VarWarnPosition.java index 7dae84f3542..f1d5f4346d0 100644 --- a/test/langtools/tools/javac/tree/VarWarnPosition.java +++ b/test/langtools/tools/javac/tree/VarWarnPosition.java @@ -22,6 +22,9 @@ public class VarWarnPosition { // Test 3 Consumer c2 = (var d) -> { }; + + // Test 4 + Consumer c3 = (final var d) -> { }; } } diff --git a/test/langtools/tools/javac/tree/VarWarnPosition.out b/test/langtools/tools/javac/tree/VarWarnPosition.out index 2200c8b4ae0..87a4ca3a1fd 100644 --- a/test/langtools/tools/javac/tree/VarWarnPosition.out +++ b/test/langtools/tools/javac/tree/VarWarnPosition.out @@ -3,4 +3,6 @@ VarWarnPosition.java:21:18: compiler.warn.has.been.deprecated: Depr, compiler.mi VarWarnPosition.java:21:28: compiler.warn.has.been.deprecated: Depr, compiler.misc.unnamed.package VarWarnPosition.java:24:18: compiler.warn.has.been.deprecated: Depr, compiler.misc.unnamed.package VarWarnPosition.java:24:30: compiler.warn.has.been.deprecated: Depr, compiler.misc.unnamed.package -5 warnings +VarWarnPosition.java:27:18: compiler.warn.has.been.deprecated: Depr, compiler.misc.unnamed.package +VarWarnPosition.java:27:36: compiler.warn.has.been.deprecated: Depr, compiler.misc.unnamed.package +7 warnings