8388261: [lworld] javac creates early_larval_frame and ACC_STRICT_INIT in record non-preview class files#2643
8388261: [lworld] javac creates early_larval_frame and ACC_STRICT_INIT in record non-preview class files#2643liach wants to merge 6 commits into
Conversation
|
👋 Welcome back liach! A progress list of the required criteria for merging this PR into |
|
@liach This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 1 new commit pushed to the
Please see this link for an up-to-date comparison between the source branch of this pull request and the ➡️ To integrate this PR with the above commit message to the |
Webrevs
|
|
/contributor add @vicente-romero-oracle |
|
@liach |
| boolean isRecordField = isInstanceField && (sym.owner.flags_field & RECORD) != 0; | ||
| if (allowValueClasses && (isInstanceFieldOfValueClass || isRecordField)) { | ||
| implicit |= FINAL | STRICT; | ||
| preview.markUsesPreview(pos); // STRICT_INIT is a preview VM feature |
There was a problem hiding this comment.
Ok -- this also means that if a record has no component, no pollution occurs, which I suppose it's ok.
|
Note: this is blocked by #2645 and requires openjdk/jdk#31916 to be merged into lworld. |
|
@liach - Your testing is showing 8 failures of:
Is this because of what you mention about TestEnableNativeAccessJarManifest.java above? |
|
@dcubed-ojdk Yes. It's already fixed in mainline, we can wait for a later merge before integrating this. |
…/lw-record-strict
|
@liach - I'm cherry-picking this fix from main-line:
to clear the runway for this fix. |
|
Since this fix is also blocked by this Valhalla fix:
I presume you will be integrating JDK-8388343 and this fix together after getting approvals. |
|
I don't think I need JDK-8388343 any more - I am fixing the BadCanonicalCtrTest (Maurizio says this will be broken when value objects become permanent feature anyways) in this patch instead. |
mcimadamore
left a comment
There was a problem hiding this comment.
Looks good. Please make sure we get a clean test run
|
Ran tier 1-6 on Oracle's CI, no related failure. Thanks for all the reviews for this last-minute spec issue! /integrate |
|
Going to push as commit 53b4a9e.
Your commit was automatically rebased without conflicts. |
We should mark record classes as using preview VM features if they use strictly-initialized fields or early larval frames.
Maurizio considered this to be P2 given the spec violations, especially with the spurious early larval frames in the class files.
Progress
Issue
Reviewers
Contributors
<vromero@openjdk.org>Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/valhalla.git pull/2643/head:pull/2643$ git checkout pull/2643Update a local copy of the PR:
$ git checkout pull/2643$ git pull https://git.openjdk.org/valhalla.git pull/2643/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 2643View PR using the GUI difftool:
$ git pr show -t 2643Using diff file
Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/2643.diff
Using Webrev
Link to Webrev Comment