RFR: 8315532: Compiler Implementation for Unnamed Variables and Patterns

[Jan Lahoda]

On Sat, 9 Sep 2023 00:02:20 GMT, Aggelos Biboudis <abimpoudis at openjdk.org> wrote:

> This PR finalizes the feature of unnamed variables and patterns.
> 
> ---------
> ### Progress
> - [ ] Change must be properly reviewed (1 review required, with at least 1 [Reviewer](https://openjdk.org/bylaws#reviewer))
> - [x] Change must not contain extraneous whitespace
> - [x] Commit message must refer to an issue
> - [ ] Change requires CSR request [JDK-8315851](https://bugs.openjdk.org/browse/JDK-8315851) to be approved
> 
> 
> 
> ### Reviewing
> <details><summary>Using <code>git</code></summary>
> 
> Checkout this PR locally: \
> `$ git fetch https://git.openjdk.org/jdk.git pull/15649/head:pull/15649` \
> `$ git checkout pull/15649`
> 
> Update a local copy of the PR: \
> `$ git checkout pull/15649` \
> `$ git pull https://git.openjdk.org/jdk.git pull/15649/head`
> 
> </details>
> <details><summary>Using Skara CLI tools</summary>
> 
> Checkout this PR locally: \
> `$ git pr checkout 15649`
> 
> View PR using the GUI difftool: \
> `$ git pr show -t 15649`
> 
> </details>
> <details><summary>Using diff file</summary>
> 
> Download this PR as a diff file: \
> <a href="https://git.openjdk.org/jdk/pull/15649.diff">https://git.openjdk.org/jdk/pull/15649.diff</a>
> 
> </details>
> 
> 
> ### Webrev
> [Link to Webrev Comment](https://git.openjdk.org/jdk/pull/15649#issuecomment-1733906010)

Overall, looks reasonable to me. A few comments/suggestions for your consideration inline.

test/langtools/tools/javac/T8314423.java line 5:

> 3:  * @bug 8314423
> 4:  * @summary Multiple patterns without unnamed variables
> 5:  * @compile T8314423.java

I think I would keep the existing `@compile` as well, with `--release 21`, to verify the source level checks are working.

test/langtools/tools/javac/diags/examples/UnderscoreAsIdentifierError.java line 26:

> 24: // key: compiler.err.underscore.as.identifier
> 25: // key: compiler.warn.source.no.system.modules.path
> 26: // options: -source 20

Nit: either `-Xlint:-options -source 21`, or `--release 21`, and drop the `warn.source.no.system.modules.path`.

test/langtools/tools/javac/diags/examples/UnderscoreInLambdaExpression.java line 24:

> 22:  */
> 23: 
> 24: // options: -Xlint:preview

`-Xlint:preview` is probably unnecessary here.

test/langtools/tools/javac/diags/examples/UseOfUnderscoreNotAllowedWithBrackets.java line 25:

> 23: 
> 24: // key: compiler.err.use.of.underscore.not.allowed.with.brackets
> 25: // options: -Xlint:preview

The `-Xlint:preview` is probably unnecessary here.

test/langtools/tools/javac/lambda/IdentifierTest.java line 7:

> 5:  * @summary Test generation of warnings when '_' is used an identifier
> 6:  * @compile/fail/ref=IdentifierTest8.out --release 8 -Werror -XDrawDiagnostics -Xlint:-options IdentifierTest.java
> 7:  * @compile/fail/ref=IdentifierTest9.out -XDrawDiagnostics IdentifierTest.java

Note here we are using `ref=IdentifierTest9.out`, but there's no `--release 9` (or `--release 21`). So, it is the same as the next line. I think it would be useful if one of these lines used `--release 21` - even if the outputs are the same.

test/langtools/tools/javac/lambda/IdentifierTest9.out line 33:

> 31: IdentifierTest.java:152:17: compiler.err.use.of.underscore.not.allowed
> 32: IdentifierTest.java:158:16: compiler.err.use.of.underscore.not.allowed.with.brackets
> 33: IdentifierTest.java:160:25: compiler.err.use.of.underscore.not.allowed

Note the errors are like:

/home/jlahoda/src/jdk/jdk2/test/langtools/tools/javac/lambda/IdentifierTest.java:160: error: as of release 21, the underscore keyword '_' is only allowed to declare
        for(String _s : _ ) {
                        ^
  unnamed patterns, local variables, exception parameters or lambda parameters


At least the release version should be updated, but to not let the message to be broken into two lines like this. Maybe produce a top-level diagnostic along the line of "underscore not allowed here" with sub-diagnostic with the details (which then can (maybe?) span multiple lines).

Also, the wording sounds to me like if there was a restriction in JDK 22, while underscore is actually permitted on more places than before.

So, maybe something like:


underscore not allowed here
as of release 9, ''_'' is a keyword, and may not be used as an identifier
as of release 21, ''_'' can be used as a name in the declaration of unnamed patterns, local variables, exception parameters or lambda parameters

?
Just an idea.

-------------

PR Review: https://git.openjdk.org/jdk/pull/15649#pullrequestreview-1642635025
PR Review Comment: https://git.openjdk.org/jdk/pull/15649#discussion_r1336168133
PR Review Comment: https://git.openjdk.org/jdk/pull/15649#discussion_r1336173607
PR Review Comment: https://git.openjdk.org/jdk/pull/15649#discussion_r1336171867
PR Review Comment: https://git.openjdk.org/jdk/pull/15649#discussion_r1336169144
PR Review Comment: https://git.openjdk.org/jdk/pull/15649#discussion_r1336171079
PR Review Comment: https://git.openjdk.org/jdk/pull/15649#discussion_r1336165928