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

Generate reference to classes holding constants used in the code #2341

Merged
merged 3 commits into from
Apr 15, 2024

Conversation

stephan-herrmann
Copy link
Contributor

fixes #1382

Emit a class constant into the constant pool for every class from which a constant is used.

fixes eclipse-jdt#1382

Emit a class constant into the constant pool for every class from which
a constant is used.
@stephan-herrmann
Copy link
Contributor Author

@iloveeclipse results from API-Tools signal bytecode changes, indeed. This could cause quite a landslide of comparator errors in the SDK build. Please suggest how / when to proceed.

As mentioned in the issue #1382 this will eliminate one small difference wrt javac.

@iloveeclipse
Copy link
Member

@stephan-herrmann :

  1. we can merge this and build SDK now to get the list of (most likely all) affected bundles.
  2. After that one should create ticket for unstable SDK build at https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/issues and check if our expectation why the unstable build is unstable matches the class file diffs (there should be this change in constant pool only).
  3. After that for every bundle that is reported to be "unstable" one has to "touch" it, set a PR and merge.
    The work of touching bundles is boring, but can be automated via https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/wiki/Platform-Releng-Faq#how-to-fix-unstable-build
  4. After that we can start next SDK build to validate everything is touched and stable.

So if you are confident in this PR, feel free to merge and we can continue with points above.

@iloveeclipse
Copy link
Member

Actually you can touch affected JDT bundles already that make this PR red as of now.

Switch to correct syntax for Class constant

Also find constants within expressions as field initializers
@iloveeclipse
Copy link
Member

@stephan-herrmann : if you plan to merge it today, please do it before 00:00 CET so the next build can pick it and we can start looking into "touching" bundles tomorrow without extra build.

@stephan-herrmann
Copy link
Contributor Author

Actually you can touch affected JDT bundles already that make this PR red as of now.

done

if you plan to merge it today, please do it before 00:00 CET so the next build can pick it and we can start looking into "touching" bundles tomorrow without extra build.

At that time I was already offline, and in fact I thought better not to mess up a Monday build (as those are the ones retained longer than other days, right?) - but then I got my timezone arithmetic wrong, so perhaps now I should indeed wait until the Monday build it good.

@iloveeclipse
Copy link
Member

@stephan-herrmann : just merge if you are done, if you wish, now. I can help next day to fix the SDK build.

@stephan-herrmann stephan-herrmann merged commit 0a9af29 into eclipse-jdt:master Apr 15, 2024
9 checks passed
@stephan-herrmann stephan-herrmann deleted the issue1382 branch April 15, 2024 20:34
@stephan-herrmann
Copy link
Contributor Author

@stephan-herrmann : just merge if you are done, if you wish, now. I can help next day to fix the SDK build.

Thanks!

@iloveeclipse
Copy link
Member

robstryker pushed a commit to robstryker/eclipse.jdt.core that referenced this pull request Jul 18, 2024
…ipse-jdt#2341)

fixes eclipse-jdt#1382

Emit a class constant into the constant pool for every class from which
a constant is used.

Touch bundles affected by this change
gayanper pushed a commit to gayanper/eclipse.jdt.core that referenced this pull request Sep 7, 2024
…ipse-jdt#2341)

fixes eclipse-jdt#1382

Emit a class constant into the constant pool for every class from which
a constant is used.

Touch bundles affected by this change
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 this pull request may close these issues.

Generate reference to classes holding constants used in the code
2 participants