Skip to content

Commit

Permalink
8304671: javac regression: Compilation with --release 8 fails on unde…
Browse files Browse the repository at this point in the history 
…rscore in enum identifiers

Reviewed-by: vromero, darcy
  • Loading branch information
@lahodaj
lahodaj committed Mar 23, 2023
1 parent e2cfcfb commit 63d4afb
Show file tree
Hide file tree
Showing 2 changed files with 188 additions and 9 deletions.
16 changes: 9 additions & 7 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
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1999, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1999, 2023, 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
Expand Down Expand Up @@ -4303,10 +4303,13 @@ List<JCTree> enumBody(Name enumName) {
return defs.toList();
}

@SuppressWarnings("fallthrough")
private EnumeratorEstimate estimateEnumeratorOrMember(Name enumName) {
// if we are seeing a record declaration inside of an enum we want the same error message as expected for a
// let's say an interface declaration inside an enum
if (token.kind == TokenKind.IDENTIFIER && token.name() != enumName &&
boolean ident = token.kind == TokenKind.IDENTIFIER ||
token.kind == TokenKind.UNDERSCORE;
if (ident && token.name() != enumName &&
(!allowRecords || !isRecordStart())) {
Token next = S.token(1);
switch (next.kind) {
Expand All @@ -4315,12 +4318,11 @@ private EnumeratorEstimate estimateEnumeratorOrMember(Name enumName) {
}
}
switch (token.kind) {
case IDENTIFIER: case MONKEYS_AT: case LT:
if (token.kind == IDENTIFIER) {
if (allowRecords && isRecordStart()) {
return EnumeratorEstimate.MEMBER;
}
case IDENTIFIER:
if (allowRecords && isRecordStart()) {
return EnumeratorEstimate.MEMBER;
}
case MONKEYS_AT: case LT: case UNDERSCORE:
return EnumeratorEstimate.UNKNOWN;
default:
return EnumeratorEstimate.MEMBER;
Expand Down
181 changes: 179 additions & 2 deletions test/langtools/tools/javac/parser/JavacParserTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2011, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2011, 2023, 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
Expand All @@ -23,7 +23,7 @@

/*
* @test
* @bug 7073631 7159445 7156633 8028235 8065753 8205418 8205913 8228451 8237041 8253584 8246774 8256411 8256149 8259050 8266436 8267221 8271928 8275097 8293897 8295401
* @bug 7073631 7159445 7156633 8028235 8065753 8205418 8205913 8228451 8237041 8253584 8246774 8256411 8256149 8259050 8266436 8267221 8271928 8275097 8293897 8295401 8304671
* @summary tests error and diagnostics positions
* @author Jan Lahoda
* @modules jdk.compiler/com.sun.tools.javac.api
Expand Down Expand Up @@ -2061,6 +2061,183 @@ public Void visitModule(ModuleTree node, Void p) {
});
}

@Test //JDK-8304671
void testEnumConstantUnderscore() throws IOException {
record TestCase(String code, String release, String ast, String errors) {}
TestCase[] testCases = new TestCase[] {
new TestCase("""
package t;
enum Test {
_
}
""",
"8",
"""
package t;
\n\
enum Test {
/*public static final*/ _ /* = new Test() */ /*enum*/ ;
} """,
"""
- compiler.warn.option.obsolete.source: 8
- compiler.warn.option.obsolete.target: 8
- compiler.warn.option.obsolete.suppression
Test.java:3:5: compiler.warn.underscore.as.identifier
"""),
new TestCase("""
package t;
enum Test {
_
}
""",
System.getProperty("java.specification.version"),
"""
package t;
\n\
enum Test {
/*public static final*/ _ /* = new Test() */ /*enum*/ ;
} """,
"""
Test.java:3:5: compiler.err.underscore.as.identifier
"""),
new TestCase("""
package t;
enum Test {
_;
}
""",
"8",
"""
package t;
\n\
enum Test {
/*public static final*/ _ /* = new Test() */ /*enum*/ ;
} """,
"""
- compiler.warn.option.obsolete.source: 8
- compiler.warn.option.obsolete.target: 8
- compiler.warn.option.obsolete.suppression
Test.java:3:5: compiler.warn.underscore.as.identifier
"""),
new TestCase("""
package t;
enum Test {
_;
}
""",
System.getProperty("java.specification.version"),
"""
package t;
\n\
enum Test {
/*public static final*/ _ /* = new Test() */ /*enum*/ ;
} """,
"""
Test.java:3:5: compiler.err.underscore.as.identifier
"""),
new TestCase("""
package t;
enum Test {
A;
void t() {}
_;
}
""",
"8",
"""
package t;
\n\
enum Test {
/*public static final*/ A /* = new Test() */ /*enum*/ ,
/*public static final*/ _ /* = new Test() */ /*enum*/ ;
\n\
void t() {
}
} """,
"""
- compiler.warn.option.obsolete.source: 8
- compiler.warn.option.obsolete.target: 8
- compiler.warn.option.obsolete.suppression
Test.java:5:5: compiler.err.enum.constant.not.expected
Test.java:5:5: compiler.warn.underscore.as.identifier
"""),
new TestCase("""
package t;
enum Test {
A;
void t() {}
_;
}
""",
System.getProperty("java.specification.version"),
"""
package t;
\n\
enum Test {
/*public static final*/ A /* = new Test() */ /*enum*/ ,
/*public static final*/ _ /* = new Test() */ /*enum*/ ;
\n\
void t() {
}
} """,
"""
Test.java:5:5: compiler.err.enum.constant.not.expected
"""),
new TestCase("""
package t;
enum Test {
_ {},
A;
}
""",
"8",
"""
package t;
\n\
enum Test {
/*public static final*/ _ /* = new Test() */ /*enum*/ {
},
/*public static final*/ A /* = new Test() */ /*enum*/ ;
} """,
"""
- compiler.warn.option.obsolete.source: 8
- compiler.warn.option.obsolete.target: 8
- compiler.warn.option.obsolete.suppression
Test.java:3:5: compiler.warn.underscore.as.identifier
"""),
new TestCase("""
package t;
enum Test {
_ {},
A;
}
""",
System.getProperty("java.specification.version"),
"""
package t;
\n\
enum Test {
/*public static final*/ _ /* = new Test() */ /*enum*/ {
},
/*public static final*/ A /* = new Test() */ /*enum*/ ;
} """,
"""
Test.java:3:5: compiler.err.underscore.as.identifier
"""),
};
for (TestCase testCase : testCases) {
StringWriter out = new StringWriter();
JavacTaskImpl ct = (JavacTaskImpl) tool.getTask(out, fm, null,
List.of("-XDrawDiagnostics", "--release", testCase.release),
null, Arrays.asList(new MyFileObject(testCase.code)));
String ast = ct.parse().iterator().next().toString().replaceAll("\\R", "\n");
assertEquals("Unexpected AST, got:\n" + ast, testCase.ast, ast);
assertEquals("Unexpected errors, got:\n" + out.toString(),
out.toString().replaceAll("\\R", "\n"),
testCase.errors);
}
}

void run(String[] args) throws Exception {
int passed = 0, failed = 0;
final Pattern p = (args != null && args.length > 0)
Expand Down

5 comments on commit 63d4afb

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

@MBaesken
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk20u

@openjdk
Copy link

@openjdk openjdk bot commented on 63d4afb Mar 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MBaesken the backport was successfully created on the branch MBaesken-backport-63d4afbe in my personal fork of openjdk/jdk20u. To create a pull request with this backport targeting openjdk/jdk20u:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 63d4afbe from the openjdk/jdk repository.

The commit being backported was authored by Jan Lahoda on 23 Mar 2023 and was reviewed by Vicente Romero and Joe Darcy.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk20u:

$ git fetch https://github.com/openjdk-bots/jdk20u.git MBaesken-backport-63d4afbe:MBaesken-backport-63d4afbe
$ git checkout MBaesken-backport-63d4afbe
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk20u.git MBaesken-backport-63d4afbe

@MBaesken
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk17u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on 63d4afb Mar 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MBaesken Could not automatically backport 63d4afbe to openjdk/jdk17u-dev due to conflicts in the following files:

  • src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java
  • test/langtools/tools/javac/parser/JavacParserTest.java

Please fetch the appropriate branch/commit and manually resolve these conflicts by using the following commands in your personal fork of openjdk/jdk17u-dev. Note: these commands are just some suggestions and you can use other equivalent commands you know.

# Fetch the up-to-date version of the target branch
$ git fetch --no-tags https://git.openjdk.org/jdk17u-dev.git master:master

# Check out the target branch and create your own branch to backport
$ git checkout master
$ git checkout -b MBaesken-backport-63d4afbe

# Fetch the commit you want to backport
$ git fetch --no-tags https://git.openjdk.org/jdk.git 63d4afbeb17df4eff0f65041926373ee62a8a33a

# Backport the commit
$ git cherry-pick --no-commit 63d4afbeb17df4eff0f65041926373ee62a8a33a
# Resolve conflicts now

# Commit the files you have modified
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport 63d4afbeb17df4eff0f65041926373ee62a8a33a'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jdk17u-dev with the title Backport 63d4afbeb17df4eff0f65041926373ee62a8a33a.

Please sign in to comment.