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

8287885: Local classes cause ClassLoader error if the type names are similar but not same #12678

Closed
wants to merge 2 commits into from

Conversation

archiecobbs
Copy link
Contributor

@archiecobbs archiecobbs commented Feb 20, 2023

The compiler doesn't really support case-insensitive filesystems. However, we can still avoid name clashes for local classes because their names are synthesized with indexed names like$1, $2, etc.

So previously, a class like this:

public class Test {

    void method1() {
        enum ABC { A, B, C; };
    }

    void method2() {
        enum Abc { A, B, C; };
    } 
}

would generate these classfiles:

Test.class
Test$1ABC.class
Test$1Abc.class

the latter two of which clash on case-insensitive filesystems. After this patch, these non-clashing classfiles are generated instead:

Test.class
Test$1ABC.class
Test$2Abc.class

The only thing slightly wonky about this patch is that clearLocalClassNameIndexes() since local classes ABC and Abc share the same index, clearing either one clears both. But this is harmless, because these indexes are cleared in a batch, during a final "cleanup" step after processing the containing class.

To avoid any locale weirdness, we only collapse ASCII letters A-Z/a-z. So this is clearly a limited, tactical fix.


Progress

  • Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue

Issue

  • JDK-8287885: Local classes cause ClassLoader error if the type names are similar but not same

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/12678/head:pull/12678
$ git checkout pull/12678

Update a local copy of the PR:
$ git checkout pull/12678
$ git pull https://git.openjdk.org/jdk pull/12678/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 12678

View PR using the GUI difftool:
$ git pr show -t 12678

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/12678.diff

@bridgekeeper
Copy link

bridgekeeper bot commented Feb 20, 2023

👋 Welcome back archiecobbs! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk openjdk bot added the rfr Pull request is ready for review label Feb 20, 2023
@openjdk
Copy link

openjdk bot commented Feb 20, 2023

@archiecobbs The following label will be automatically applied to this pull request:

  • compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

@openjdk openjdk bot added the compiler compiler-dev@openjdk.org label Feb 20, 2023
@mlbridge
Copy link

mlbridge bot commented Feb 20, 2023

Webrevs

@archiecobbs
Copy link
Contributor Author

Withdrawing this PR - too much of a hack and it doesn't really solve the underlying problem.

To be discussed further on compiler-dev.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler compiler-dev@openjdk.org rfr Pull request is ready for review
1 participant