Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[21] JEP 443: Unnamed Patterns and Variables (Preview) #893

Closed
Tracked by #890
mpalat opened this issue Mar 23, 2023 · 11 comments · Fixed by #1517
Closed
Tracked by #890

[21] JEP 443: Unnamed Patterns and Variables (Preview) #893

mpalat opened this issue Mar 23, 2023 · 11 comments · Fixed by #1517
Assignees
Milestone

Comments

@mpalat
Copy link
Contributor

mpalat commented Mar 23, 2023

Essentially the underscore _

ref: https://openjdk.org/jeps/443

"Summary
Enhance the Java language with unnamed patterns, which match a record component without stating the component's name or type, and unnamed variables, which can be initialized but not used. Both are denoted by an underscore character, _. This is a preview language feature."

JLS Draft changes at: https://cr.openjdk.org/~abimpoudis/unnamed/latest/

@mpalat mpalat added this to the BETA_JAVA21 milestone Mar 23, 2023
@mpalat mpalat changed the title JEP 443: Unnamed Patterns and Variables (Preview) [21] JEP 443: Unnamed Patterns and Variables (Preview) Mar 23, 2023
@mpalat
Copy link
Contributor Author

mpalat commented Jul 20, 2023

The relevant portion :
A declaration commonly includes an identifier (3.8) that can be used in a name to refer to the declared entity. The identifier is constrained to avoid certain contextual keywords when the entity being introduced is a class, interface, or type parameter.

If a declaration does not include an identifier, but instead includes the reserved keyword _ (underscore), then the entity cannot be referred to by name. The following kinds of entity may be declared using an underscore:

A local variable, one of the following:

A local variable declared by a local variable declaration statement in a block (14.4.2)

A local variable declared by a for statement or a try-with-resources statement (14.14, 14.20.3)

A local variable declared by a pattern (14.30.1)

An exception parameter of an exception handler declared in a catch clause of a try statement (14.20)

A formal parameter of a lambda expression (15.27.1)

A local variable, exception parameter, or lambda parameter that is declared using an underscore is called an unnamed local variable, unnamed exception parameter, or unnamed lambda parameter, respectively.

@mpalat
Copy link
Contributor Author

mpalat commented Aug 21, 2023

retargeting out of BETA_JAVA21
(that said, may incorporate if time permits)

@datho7561
Copy link
Contributor

If no one else is currently working on this, I'm interested in taking a look at it.

@robstryker
Copy link
Contributor

robstryker commented Oct 17, 2023

First things I've noticed:

  1. The file eclipse.jdt.core/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/messages.properties has an error message for error code 443 as follows: 443 = '_' should not be used as an identifier, since it is a reserved keyword from source level 1.8 on - This error string likely needs to be changed to recognize it is a valid identifier post java-21.

  2. Parser.java needs a parsingJava21Plus flag. There is already a parsingJava18Plus flag. It can probably be discovered via this.parsingJava21Plus = this.options.sourceLevel >= ClassFileConstants.JDK21;

  3. The visual problem displayed when using an underscore as a variable name is also in Parser.java and is created where you find this code inside Parser's pushIdentifier method: problemReporter().illegalUseOfUnderscoreAsAnIdentifier((int) (position >>> 32), (int) position, this.parsingJava9Plus);

This is likely the code to begin looking at. I suggest setting a breakpoint here.

My initial reaction here is to impl as follows:

	if (this.parsingJava8Plus && identifier.length == 1 && identifier[0] == '_' && !this.processingLambdaParameterList) {
		if( !this.parsingJava21Plus ) {
			problemReporter().illegalUseOfUnderscoreAsAnIdentifier((int) (position >>> 32), (int) position, this.parsingJava9Plus);
		} else {
			// TODO check if we are in an approved situation? If not, show the problem
			boolean approved = false; // TODO impl this
			if( !approved ) {
				problemReporter().illegalUseOfUnderscoreAsAnIdentifier((int) (position >>> 32), (int) position, this.parsingJava9Plus);
			}
		}

But of course some of this might need to be done earlier in the pushIdentifier method call because it appears we are pushing an identifier to the stack...? I am really clueless here.

In the above check for boolean approved = ..., I would assume we check if we're in the middle of parsing a for-loop declaration, a try/catch declaration, a try-with-resources declaration, or a catch statement. If we are, I assume we are approved and do not need to show the error?

This is as far as I've gotten thus far.

@rgrunber rgrunber moved this to 📋 Backlog in IDE Cloudaptors Oct 18, 2023
@datho7561 datho7561 moved this from 📋 Backlog to 🏗 In progress in IDE Cloudaptors Oct 18, 2023
@rgrunber
Copy link
Contributor

Just as an update. @datho7561 & @robstryker have a basic idea of how the grammar works and were able to implement support for cases like : catch (Exception _). Now, I think they just need to do the same for all other supported cases.

Also, it looks like there may be some work to do on the bytecode generation side since it's likely the unnamed variables shouldn't be added to any table tracking locals.

@datho7561
Copy link
Contributor

I looked into what would be needed for bytecode generation. I experimented with generating bytecode using javac (Termurin 21) and ecj (with the parser changes allowing for unnamed variables in catch statements). For the following case:

try {
    throw new Exception();
} catch (Exception e1) {
    try {
        throw new Exception();
    } catch (Exception e2) {
        System.out.println("oh no");
    }
}

javac moves e1 and e2 off the stack using astore, but when e1 and e2 are replaced with _, it uses pop instead.

ecj already uses the pop instruction in both cases.

This leads me to believe that nothing needs to changed about how the bytecode is generated.

@rgrunber
Copy link
Contributor

As an update for anyone following, we've implemented all cases of this. I think the case of "_" with no type at all is tricky in some places. We're preparing a PR that breaks things down as-per #1487 (comment) , as well as adding a bunch of tests for this.

@datho7561
Copy link
Contributor

datho7561 commented Oct 26, 2023

we've implemented all cases of this

we're not quite there yet

okay I pushed the code for the last case we weren't handling

@mpalat
Copy link
Contributor Author

mpalat commented Oct 31, 2023

@datho7561 Wanted to check where you are wrt this implementation? I am adding a pr which I had for tests for this. Also adding myself as an additional assignee (hope you are fine)

@rgrunber rgrunber linked a pull request Nov 1, 2023 that will close this issue
3 tasks
@mpalat mpalat self-assigned this Nov 1, 2023
@mpalat
Copy link
Contributor Author

mpalat commented Nov 1, 2023

Thanks @rgrunber for linking the pr 1517 here.. will take a look

@datho7561 datho7561 moved this from 🏗 In progress to 👀 In review in IDE Cloudaptors Nov 16, 2023
kriegaex added a commit to eclipse-aspectj/aspectj that referenced this issue Dec 10, 2023
TODO: Activate after
eclipse-jdt/eclipse.jdt.core#893 is done.
Signed-off-by: Alexander Kriegisch <[email protected]>
@kriegaex
Copy link
Contributor

JDK 21 has been out for several months and Eclipse 2023-12 was announced to (fully?) support JDK 21. I am surprised that this was not part of the release. Is this going to be done anytime soon? I delayed the AspectJ release for JDK 21 for several months, waiting for JDT Core and ECJ to catch up with JDK 21. Now after the 2023-12 release, I thought I could also release AspectJ, but obviously I cannot or have to add a disclaimer about which preview features are still unsupported.

This is no bashing, I know that probably you do not have much resources and understand. But I want to suggest to delay any JDK 22 features until all JDK 21 features are fully implemented. Or is the policy rather to always implement the final features for each JDK release, and preview features should are best effort, maybe done and maybe not? Please help me to understand this. Thank you.

kriegaex added a commit to eclipse-aspectj/aspectj that referenced this issue Dec 11, 2023
TODO: Activate after
eclipse-jdt/eclipse.jdt.core#893 is done.
Signed-off-by: Alexander Kriegisch <[email protected]>
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in IDE Cloudaptors Jan 29, 2024
robstryker pushed a commit to robstryker/eclipse.jdt.core that referenced this issue Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants