Re: RFR: 8330458: Add missing @since tag to ClassFile.JAVA_23_VERSION

[Adam Sotona]

On Wed, 17 Apr 2024 20:46:26 GMT, Joe Darcy  wrote:

> 8330458: Add missing @since tag to ClassFile.JAVA_23_VERSION

Marked as reviewed by asotona (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/18828#pullrequestreview-2007907265

Integrated: 8330467: NoClassDefFoundError when lambda is in a hidden class

[Adam Sotona]

On Wed, 17 Apr 2024 08:46:59 GMT, Adam Sotona  wrote:

> Current implementation of `LambdaMetafactory` does not allow to use lambdas 
> in hidden classes. Invocation throws `NoClassDefFoundError` instead.
> 
> This patch includes lambda implementation in a hidden class under the special 
> handling of `useImplMethodHandle`.
> The patch also fixes `j/l/i/defineHiddenClass/BasicTest::testLambda` to 
> correctly cover this test case.
> 
> Please review.
> 
> Thanks,
> Adam

This pull request has now been integrated.

Changeset: 706b421c
Author:Adam Sotona 
URL:   
https://git.openjdk.org/jdk/commit/706b421ccaff2129512ee3ea15ef1d6c264cbe01
Stats: 58 lines in 3 files changed: 53 ins; 0 del; 5 mod

8330467: NoClassDefFoundError when lambda is in a hidden class

Reviewed-by: psandoz, mchung

-

PR: https://git.openjdk.org/jdk/pull/18810

Re: RFR: 8330467: NoClassDefFoundError when lambda is in a hidden class [v4]

[Adam Sotona]

> Current implementation of `LambdaMetafactory` does not allow to use lambdas 
> in hidden classes. Invocation throws `NoClassDefFoundError` instead.
> 
> This patch includes lambda implementation in a hidden class under the special 
> handling of `useImplMethodHandle`.
> The patch also fixes `j/l/i/defineHiddenClass/BasicTest::testLambda` to 
> correctly cover this test case.
> 
> Please review.
> 
> Thanks,
> Adam

Adam Sotona has updated the pull request incrementally with one additional 
commit since the last revision:

  Update 
src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java
  
  Co-authored-by: Mandy Chung 

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18810/files
  - new: https://git.openjdk.org/jdk/pull/18810/files/e8eb1752..091ee2af

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18810=03
 - incr: https://webrevs.openjdk.org/?repo=jdk=18810=02-03

  Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod
  Patch: https://git.openjdk.org/jdk/pull/18810.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18810/head:pull/18810

PR: https://git.openjdk.org/jdk/pull/18810

Re: RFR: 8328481: Implement Module Imports [v8]

[Alan Bateman]

On Wed, 17 Apr 2024 19:33:09 GMT, Jan Lahoda  wrote:

>> This is an implementation of JEP JDK-8315129: Module Import Declarations 
>> (Preview). Please see the JEP for details:
>> https://bugs.openjdk.org/browse/JDK-8315129
>> 
>> It is mostly straightforward - the module imports are parsed, and then 
>> expanded to import-on-demand in `TypeEnter`.
>> There is a few notable aspects, however:
>> - the AST node for import (`JCImport`) is holding the imported element as a 
>> field access, because so far, the imported element always had to have a '.' 
>> (even for import-on-demand). But for module imports, it is permissible to 
>> import from a module whose name does not have a dot (`import module m;`). 
>> The use of field access for ordinary import seems very useful, so I 
>> preferred to keep that, and created a new internal-only AST node for module 
>> imports. There is still only one public API AST node/interface, so this is 
>> purely an implementation choice.
>> - JShell now supports module imports as well; and the default, implicit, 
>> script is changed to use it to import all of `java.base` if preview is 
>> enabled. It is expected that the default would be changed if/when the module 
>> imports feature is finalized.
>
> Jan Lahoda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fixing ListModuleDeps test.

src/java.base/share/classes/jdk/internal/javac/PreviewFeature.java line 84:

> 82: @JEP(number=461, title="Stream Gatherers", status="Preview")
> 83: STREAM_GATHERERS,
> 84: @JEP(number=0, title="Module Imports", status="Preview")

I see this has been assigned JEP 476 so I assume you can set the number for the 
first integration.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18614#discussion_r1570018046

RFR: 8330542: Add two sample configuration files

[Joe Wang]

Add two sample configuration files:

  jaxp-strict.properties: used to set strict configuration, stricter than 
jaxp.properties in previous versions such as JDK 22

  jaxp-compat.properties: used to regain compatibility from any more restricted 
configuration than previous versions such as JDK 22

-

Commit messages:
 - fix whitespace
 - 8330542: Add two sample configuration files

Changes: https://git.openjdk.org/jdk/pull/18831/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk=18831=00
  Issue: https://bugs.openjdk.org/browse/JDK-8330542
  Stats: 229 lines in 2 files changed: 229 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/18831.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18831/head:pull/18831

PR: https://git.openjdk.org/jdk/pull/18831

Re: RFR: 8330458: Add missing @since tag to ClassFile.JAVA_23_VERSION

[Iris Clark]

On Wed, 17 Apr 2024 20:46:26 GMT, Joe Darcy  wrote:

> 8330458: Add missing @since tag to ClassFile.JAVA_23_VERSION

Marked as reviewed by iris (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/18828#pullrequestreview-2007547483

Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v6]

[Claes Redestad]

On Wed, 17 Apr 2024 15:21:32 GMT, Adam Sotona  wrote:

>> java.base java.lang.invoke package heavily uses ASM to generate lambdas and 
>> method handles.
>> 
>> This patch converts ASM calls to Classfile API.
>> 
>> This PR is continuation of https://github.com/openjdk/jdk/pull/12945
>> 
>> Any comments and suggestions are welcome.
>> 
>> Please review.
>> 
>> Thank you,
>> Adam
>
> Adam Sotona has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains 12 additional 
> commits since the last revision:
> 
>  - applied suggested changes
>  - Merge branch 'master' into JDK-8294960-invoke
>  - Reversion of ClassBuilder changes
>  - Merge branch 'master' into JDK-8294960-invoke
>  - Apply suggestions from code review
>
>Co-authored-by: liach <7806504+li...@users.noreply.github.com>
>  - Apply suggestions from code review
>
>Co-authored-by: liach <7806504+li...@users.noreply.github.com>
>  - added missing comment
>  - fixed InnerClassLambdaMetafactory for hidden caller classes
>  - performance improvements
>  - consolidation of descriptors handling
>  - ... and 2 more: https://git.openjdk.org/jdk/compare/9359450b...304054b9

There is a slew of internal benchmarks testing lambda initialization. Some 
focused, many indirect. I've given a few pointers offline.

-

PR Comment: https://git.openjdk.org/jdk/pull/17108#issuecomment-2062633171

Re: RFR: 8330458: Add missing @since tag to ClassFile.JAVA_23_VERSION

[Jonathan Gibbons]

On Wed, 17 Apr 2024 20:46:26 GMT, Joe Darcy  wrote:

> 8330458: Add missing @since tag to ClassFile.JAVA_23_VERSION

Marked as reviewed by jjg (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/18828#pullrequestreview-2007300284

Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v6]

[Sergey Kuksenko]

On Wed, 17 Apr 2024 15:21:32 GMT, Adam Sotona  wrote:

>> java.base java.lang.invoke package heavily uses ASM to generate lambdas and 
>> method handles.
>> 
>> This patch converts ASM calls to Classfile API.
>> 
>> This PR is continuation of https://github.com/openjdk/jdk/pull/12945
>> 
>> Any comments and suggestions are welcome.
>> 
>> Please review.
>> 
>> Thank you,
>> Adam
>
> Adam Sotona has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains 12 additional 
> commits since the last revision:
> 
>  - applied suggested changes
>  - Merge branch 'master' into JDK-8294960-invoke
>  - Reversion of ClassBuilder changes
>  - Merge branch 'master' into JDK-8294960-invoke
>  - Apply suggestions from code review
>
>Co-authored-by: liach <7806504+li...@users.noreply.github.com>
>  - Apply suggestions from code review
>
>Co-authored-by: liach <7806504+li...@users.noreply.github.com>
>  - added missing comment
>  - fixed InnerClassLambdaMetafactory for hidden caller classes
>  - performance improvements
>  - consolidation of descriptors handling
>  - ... and 2 more: https://git.openjdk.org/jdk/compare/983bd2da...304054b9

Sorry, but I am afraid we don't have benchmarks targeting lambdas startup.


From: Mandy Chung ***@***.***>
Sent: Wednesday, April 17, 2024 9:32 AM
To: openjdk/jdk
Cc: Sergey Kuksenko; Mention
Subject: [External] : Re: [openjdk/jdk] 8294960: Convert 
java.base/java.lang.invoke package to use the Classfile API to generate lambdas 
and method handles (PR #17108)

@cl4es
 and 
@kuksenko
 can you advice what performance benchmarks to run to approve this change?

—
Reply to this email directly, view it on 
GitHub,
 or 
unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>

-

PR Comment: https://git.openjdk.org/jdk/pull/17108#issuecomment-2062338565

RFR: 8330458: Add missing @since tag to ClassFile.JAVA_23_VERSION

[Joe Darcy]

8330458: Add missing @since tag to ClassFile.JAVA_23_VERSION

-

Commit messages:
 - JDK-8330458: Add missing @since tag to ClassFile.JAVA_23_VERSION

Changes: https://git.openjdk.org/jdk/pull/18828/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk=18828=00
  Issue: https://bugs.openjdk.org/browse/JDK-8330458
  Stats: 5 lines in 1 file changed: 3 ins; 0 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/18828.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18828/head:pull/18828

PR: https://git.openjdk.org/jdk/pull/18828

Re: RFR: 8327964: Simplify BigInteger.implMultiplyToLen intrinsic [v5]

[Dean Long]

On Wed, 17 Apr 2024 12:35:54 GMT, Yudi Zheng  wrote:

>> Moving array construction within BigInteger.implMultiplyToLen intrinsic 
>> candidate to its caller simplifies the intrinsic implementation in JIT 
>> compiler.
>
> Yudi Zheng has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   address comment.

I think you'll want to ask port maintainers for aarch64/arm/ppc/riscv/s390 to 
review and test those changes.
There may be some opportunities for minor improvements, but those could be done 
later.  For example, we are computing `zlen` for the squareToLen stub even 
though the value is unused.  And both x86 and aarch64 seem to have unneeded 
save/restore code, even though I think all these registers are killed when 
called by a C2 runtime stub.

-

PR Comment: https://git.openjdk.org/jdk/pull/18226#issuecomment-2062138149

Re: RFR: 8327964: Simplify BigInteger.implMultiplyToLen intrinsic [v5]

[Dean Long]

On Wed, 17 Apr 2024 12:35:54 GMT, Yudi Zheng  wrote:

>> Moving array construction within BigInteger.implMultiplyToLen intrinsic 
>> candidate to its caller simplifies the intrinsic implementation in JIT 
>> compiler.
>
> Yudi Zheng has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   address comment.

src/hotspot/cpu/x86/macroAssembler_x86.cpp line 6662:

> 6660:   push(tmp5);
> 6661: 
> 6662:   push(xlen);

There may be an opportunity here (separate RFE?) to get rid of the save/restore 
for these.  I don't think it's necessary if this is called as part of a C2 stub.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18226#discussion_r1569452818

Re: RFR: 8329222: java.text.NumberFormat (and subclasses) spec updates [v4]

[Naoto Sato]

On Wed, 17 Apr 2024 17:16:55 GMT, Justin Lu  wrote:

>> Please review this PR which is a large spec reformatting for 
>> _java.text.NumberFormat_ and related subclasses, specifically DecimalFormat 
>> and CompactNumberFormat.
>> 
>> The motivation for this change was the difficulty of readability for these 
>> classes. This PR consists of changes such as better separating the text into 
>> sections with headers, advising to skip the pattern related sections if not 
>> needed, and general wording improvements.
>> 
>> To help with reviewing I have attached a 
>> [specdiff](https://cr.openjdk.org/~jlu/JDK-8329222/java.base/java/text/package-summary.html)
>>  as well as the built docs: 
>> [nFmt](https://cr.openjdk.org/~jlu/api/java.base/java/text/NumberFormat.html),
>>  
>> [dFmt](https://cr.openjdk.org/~jlu/api/java.base/java/text/DecimalFormat.html),
>>  and 
>> [cnFmt](https://cr.openjdk.org/~jlu/api/java.base/java/text/CompactNumberFormat.html);
>> 
>> I will update the CSR once the wording is finalized.
>
> Justin Lu has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Make apparent the distinction of cnFmt and dFmt for factory methods.
>   Loosen wording from cast to convert.

LGTM

-

Marked as reviewed by naoto (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18731#pullrequestreview-2007009092

Re: RFR: 8330467: NoClassDefFoundError when lambda is in a hidden class [v3]

[Mandy Chung]

On Wed, 17 Apr 2024 16:08:31 GMT, Adam Sotona  wrote:

>> Current implementation of `LambdaMetafactory` does not allow to use lambdas 
>> in hidden classes. Invocation throws `NoClassDefFoundError` instead.
>> 
>> This patch includes lambda implementation in a hidden class under the 
>> special handling of `useImplMethodHandle`.
>> The patch also fixes `j/l/i/defineHiddenClass/BasicTest::testLambda` to 
>> correctly cover this test case.
>> 
>> Please review.
>> 
>> Thanks,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - typo in comment fix
>  - applied suggested changes

Looks fine to me.   A hidden class cannot be resolved by name.   I tweaked the 
comment to make it clear.

src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java 
line 182:

> 180: // live implementation method handle to the proxy class to invoke
> 181: // directly. (javac prefers to avoid this situation by generating
> 182: // bridges in the target class)

Suggestion:

// lambda class has no access to the resolved method, or does
// 'invokestatic' on a hidden class which cannot be resolved by name.
// Instead, we need to pass the live implementation method handle to
// the proxy class to invoke directly. (javac prefers to avoid this
// situation by generating bridges in the target class)

-

Marked as reviewed by mchung (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18810#pullrequestreview-2007006920
PR Review Comment: https://git.openjdk.org/jdk/pull/18810#discussion_r1569431448

Re: RFR: 8327964: Simplify BigInteger.implMultiplyToLen intrinsic [v5]

[Dean Long]

On Wed, 17 Apr 2024 19:45:02 GMT, Dean Long  wrote:

>> Yudi Zheng has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   address comment.
>
> src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp line 4702:
> 
>> 4700: const Register tmp5  = r15;
>> 4701: const Register tmp6  = r16;
>> 4702: const Register tmp7  = r17;
> 
> No need for r17 or sorting tmps.  Make tmp0 r3, or r6, r7, etc.

Also, I don't see why the code below saves and restores r4/r5.  Maybe 
@theRealAph knows?  Aren't all these registers killed across a runtime call?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18226#discussion_r1569427241

Re: RFR: 8327964: Simplify BigInteger.implMultiplyToLen intrinsic [v5]

[Dean Long]

On Wed, 17 Apr 2024 12:35:54 GMT, Yudi Zheng  wrote:

>> Moving array construction within BigInteger.implMultiplyToLen intrinsic 
>> candidate to its caller simplifies the intrinsic implementation in JIT 
>> compiler.
>
> Yudi Zheng has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   address comment.

src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp line 4693:

> 4691: const Register xlen  = r1;
> 4692: const Register z = r2;
> 4693: const Register zlen  = r3;

LibraryCallKit::inline_squareToLen() is still computing zlen and passing it as 
the 4th arg, even though the value is unused.

src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp line 4702:

> 4700: const Register tmp5  = r15;
> 4701: const Register tmp6  = r16;
> 4702: const Register tmp7  = r17;

No need for r17 or sorting tmps.  Make tmp0 r3, or r6, r7, etc.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18226#discussion_r1569419199
PR Review Comment: https://git.openjdk.org/jdk/pull/18226#discussion_r1569420732

Re: RFR: 8327964: Simplify BigInteger.implMultiplyToLen intrinsic [v5]

[Dean Long]

On Wed, 17 Apr 2024 12:35:54 GMT, Yudi Zheng  wrote:

>> Moving array construction within BigInteger.implMultiplyToLen intrinsic 
>> candidate to its caller simplifies the intrinsic implementation in JIT 
>> compiler.
>
> Yudi Zheng has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   address comment.

src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp line 4670:

> 4668: const Register tmp5  = r15;
> 4669: const Register tmp6  = r16;
> 4670: const Register tmp7  = r17;

Why not minimize changes and continue to use r5 for tmp0?  I see no need for 
r17 or to reassign all the other tmp registers.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18226#discussion_r1569401544

Re: RFR: 8328481: Implement Module Imports [v8]

[Jan Lahoda]

> This is an implementation of JEP JDK-8315129: Module Import Declarations 
> (Preview). Please see the JEP for details:
> https://bugs.openjdk.org/browse/JDK-8315129
> 
> It is mostly straightforward - the module imports are parsed, and then 
> expanded to import-on-demand in `TypeEnter`.
> There is a few notable aspects, however:
> - the AST node for import (`JCImport`) is holding the imported element as a 
> field access, because so far, the imported element always had to have a '.' 
> (even for import-on-demand). But for module imports, it is permissible to 
> import from a module whose name does not have a dot (`import module m;`). The 
> use of field access for ordinary import seems very useful, so I preferred to 
> keep that, and created a new internal-only AST node for module imports. There 
> is still only one public API AST node/interface, so this is purely an 
> implementation choice.
> - JShell now supports module imports as well; and the default, implicit, 
> script is changed to use it to import all of `java.base` if preview is 
> enabled. It is expected that the default would be changed if/when the module 
> imports feature is finalized.

Jan Lahoda has updated the pull request incrementally with one additional 
commit since the last revision:

  Fixing ListModuleDeps test.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18614/files
  - new: https://git.openjdk.org/jdk/pull/18614/files/27f9cfb5..7fa0ad51

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18614=07
 - incr: https://webrevs.openjdk.org/?repo=jdk=18614=06-07

  Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/18614.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18614/head:pull/18614

PR: https://git.openjdk.org/jdk/pull/18614

Re: RFR: 8330467: NoClassDefFoundError when lambda is in a hidden class [v3]

[Paul Sandoz]

On Wed, 17 Apr 2024 16:08:31 GMT, Adam Sotona  wrote:

>> Current implementation of `LambdaMetafactory` does not allow to use lambdas 
>> in hidden classes. Invocation throws `NoClassDefFoundError` instead.
>> 
>> This patch includes lambda implementation in a hidden class under the 
>> special handling of `useImplMethodHandle`.
>> The patch also fixes `j/l/i/defineHiddenClass/BasicTest::testLambda` to 
>> correctly cover this test case.
>> 
>> Please review.
>> 
>> Thanks,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - typo in comment fix
>  - applied suggested changes

Looks good to me. I conservatively bumped the review count to 2.

-

Marked as reviewed by psandoz (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18810#pullrequestreview-2006903761

Re: RFR: 8329222: java.text.NumberFormat (and subclasses) spec updates [v4]

[Justin Lu]

> Please review this PR which is a large spec reformatting for 
> _java.text.NumberFormat_ and related subclasses, specifically DecimalFormat 
> and CompactNumberFormat.
> 
> The motivation for this change was the difficulty of readability for these 
> classes. This PR consists of changes such as better separating the text into 
> sections with headers, advising to skip the pattern related sections if not 
> needed, and general wording improvements.
> 
> To help with reviewing I have attached a 
> [specdiff](https://cr.openjdk.org/~jlu/JDK-8329222/java.base/java/text/package-summary.html)
>  as well as the built docs: 
> [nFmt](https://cr.openjdk.org/~jlu/api/java.base/java/text/NumberFormat.html),
>  
> [dFmt](https://cr.openjdk.org/~jlu/api/java.base/java/text/DecimalFormat.html),
>  and 
> [cnFmt](https://cr.openjdk.org/~jlu/api/java.base/java/text/CompactNumberFormat.html);
> 
> I will update the CSR once the wording is finalized.

Justin Lu has updated the pull request incrementally with one additional commit 
since the last revision:

  Make apparent the distinction of cnFmt and dFmt for factory methods.
  Loosen wording from cast to convert.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18731/files
  - new: https://git.openjdk.org/jdk/pull/18731/files/95626d94..33b02f00

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18731=03
 - incr: https://webrevs.openjdk.org/?repo=jdk=18731=02-03

  Stats: 10 lines in 3 files changed: 0 ins; 1 del; 9 mod
  Patch: https://git.openjdk.org/jdk/pull/18731.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18731/head:pull/18731

PR: https://git.openjdk.org/jdk/pull/18731

Re: RFR: 8314480: Memory ordering spec updates in java.lang.ref [v22]

[Brent Christian]

On Fri, 5 Apr 2024 23:13:39 GMT, Brent Christian  wrote:

>> Classes in the `java.lang.ref` package would benefit from an update to bring 
>> the spec in line with how the VM already behaves. The changes would focus on 
>> _happens-before_ edges at some key points during reference processing.
>> 
>> A couple key things we want to be able to say are:
>> - `Reference.reachabilityFence(x)` _happens-before_ reference processing 
>> occurs for 'x'.
>> - `Cleaner.register()` _happens-before_ the Cleaner thread runs the 
>> registered cleaning action.
>> 
>> This will bring Cleaner in line (or close) with the memory visibility 
>> guarantees made for finalizers in [JLS 
>> 17.4.5](https://docs.oracle.com/javase/specs/jls/se18/html/jls-17.html#jls-17.4.5):
>> _"There is a happens-before edge from the end of a constructor of an object 
>> to the start of a finalizer (§12.6) for that object."_
>
> Brent Christian has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Another update to reachabilityFence() @apiNote

AFAICT, all review feedback on this change has been addressed. I would love to 
have some reviewers take another look. Thanks!

-

PR Comment: https://git.openjdk.org/jdk/pull/16644#issuecomment-2061795930

Integrated: 8319516: AIX System::loadLibrary needs support to load a shared library from an archive object

[Suchismith Roy]

On Wed, 21 Feb 2024 11:32:41 GMT, Suchismith Roy  wrote:

> Allow support for both .a and .so files in AIX.
> If .so file is not found, allow fallback to .a extension.
> JBS Issue: [JDK-8319516](https://bugs.openjdk.org/browse/JDK-8319516)

This pull request has now been integrated.

Changeset: 4895a15a
Author:Suchismith Roy 
Committer: Martin Doerr 
URL:   
https://git.openjdk.org/jdk/commit/4895a15a779fab70290b4df3b464d79a14937e28
Stats: 149 lines in 2 files changed: 149 ins; 0 del; 0 mod

8319516: AIX System::loadLibrary needs support to load a shared library from an 
archive object

Reviewed-by: mdoerr, mchung

-

PR: https://git.openjdk.org/jdk/pull/17945

Re: RFR: 8319516: AIX System::loadLibrary needs support to load a shared library from an archive object [v29]

[Martin Doerr]

On Wed, 17 Apr 2024 11:20:24 GMT, Suchismith Roy  wrote:

>> Allow support for both .a and .so files in AIX.
>> If .so file is not found, allow fallback to .a extension.
>> JBS Issue: [JDK-8319516](https://bugs.openjdk.org/browse/JDK-8319516)
>
> Suchismith Roy has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update LoadAIXLibraryFromArchiveObject.java
>   
>   Nits,coding style and variable name

Thanks! Let's ship it!

-

PR Comment: https://git.openjdk.org/jdk/pull/17945#issuecomment-2061781048

Re: RFR: 8330182: Start of release updates for JDK 24 [v2]

[Iris Clark]

On Wed, 17 Apr 2024 05:43:12 GMT, Joe Darcy  wrote:

>> Get JDK 24 underway.
>
> Joe Darcy has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Correct release date as observed in review feedback.
>  - Improve javadoc of class file update.

Marked as reviewed by iris (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/18787#pullrequestreview-2006604933

Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v6]

[Mandy Chung]

On Wed, 17 Apr 2024 15:21:32 GMT, Adam Sotona  wrote:

>> java.base java.lang.invoke package heavily uses ASM to generate lambdas and 
>> method handles.
>> 
>> This patch converts ASM calls to Classfile API.
>> 
>> This PR is continuation of https://github.com/openjdk/jdk/pull/12945
>> 
>> Any comments and suggestions are welcome.
>> 
>> Please review.
>> 
>> Thank you,
>> Adam
>
> Adam Sotona has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains 12 additional 
> commits since the last revision:
> 
>  - applied suggested changes
>  - Merge branch 'master' into JDK-8294960-invoke
>  - Reversion of ClassBuilder changes
>  - Merge branch 'master' into JDK-8294960-invoke
>  - Apply suggestions from code review
>
>Co-authored-by: liach <7806504+li...@users.noreply.github.com>
>  - Apply suggestions from code review
>
>Co-authored-by: liach <7806504+li...@users.noreply.github.com>
>  - added missing comment
>  - fixed InnerClassLambdaMetafactory for hidden caller classes
>  - performance improvements
>  - consolidation of descriptors handling
>  - ... and 2 more: https://git.openjdk.org/jdk/compare/6af94676...304054b9

@cl4es and @kuksenko can you advice what performance benchmarks to run to 
approve this change?

-

PR Comment: https://git.openjdk.org/jdk/pull/17108#issuecomment-2061720876

Re: RFR: 8319516: AIX System::loadLibrary needs support to load a shared library from an archive object [v29]

[Mandy Chung]

On Wed, 17 Apr 2024 11:20:24 GMT, Suchismith Roy  wrote:

>> Allow support for both .a and .so files in AIX.
>> If .so file is not found, allow fallback to .a extension.
>> JBS Issue: [JDK-8319516](https://bugs.openjdk.org/browse/JDK-8319516)
>
> Suchismith Roy has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update LoadAIXLibraryFromArchiveObject.java
>   
>   Nits,coding style and variable name

Marked as reviewed by mchung (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/17945#pullrequestreview-2006514775

Re: RFR: 8330467: NoClassDefFoundError when lambda is in a hidden class [v2]

[Adam Sotona]

On Wed, 17 Apr 2024 15:39:11 GMT, Paul Sandoz  wrote:

>> Adam Sotona has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   applied suggested changes
>
> test/jdk/java/lang/invoke/defineHiddenClass/src/Lambda.java line 28:
> 
>> 26: public class Lambda implements HiddenTest {
>> 27:  public void test() {
>> 28:  Function f = o -> o.toString();
> 
> Recommend you retain the existing method reference e.g.:
> 
>  Function fmref = Object::toString;
>  Function flexp = o -> o.toString();
>  String s = fmref.apply(this) + flexp.apply(this);
>  ...
> 
> Or split them out into two tests (easier to debug if case fails).

I've split the tests, thanks!

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18810#discussion_r1569099751

Re: RFR: 8330467: NoClassDefFoundError when lambda is in a hidden class [v3]

[Adam Sotona]

> Current implementation of `LambdaMetafactory` does not allow to use lambdas 
> in hidden classes. Invocation throws `NoClassDefFoundError` instead.
> 
> This patch includes lambda implementation in a hidden class under the special 
> handling of `useImplMethodHandle`.
> The patch also fixes `j/l/i/defineHiddenClass/BasicTest::testLambda` to 
> correctly cover this test case.
> 
> Please review.
> 
> Thanks,
> Adam

Adam Sotona has updated the pull request incrementally with two additional 
commits since the last revision:

 - typo in comment fix
 - applied suggested changes

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18810/files
  - new: https://git.openjdk.org/jdk/pull/18810/files/c637ab90..e8eb1752

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18810=02
 - incr: https://webrevs.openjdk.org/?repo=jdk=18810=01-02

  Stats: 18 lines in 3 files changed: 15 ins; 0 del; 3 mod
  Patch: https://git.openjdk.org/jdk/pull/18810.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18810/head:pull/18810

PR: https://git.openjdk.org/jdk/pull/18810

Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v6]

[Adam Sotona]

On Wed, 17 Apr 2024 15:21:32 GMT, Adam Sotona  wrote:

>> java.base java.lang.invoke package heavily uses ASM to generate lambdas and 
>> method handles.
>> 
>> This patch converts ASM calls to Classfile API.
>> 
>> This PR is continuation of https://github.com/openjdk/jdk/pull/12945
>> 
>> Any comments and suggestions are welcome.
>> 
>> Please review.
>> 
>> Thank you,
>> Adam
>
> Adam Sotona has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains 12 additional 
> commits since the last revision:
> 
>  - applied suggested changes
>  - Merge branch 'master' into JDK-8294960-invoke
>  - Reversion of ClassBuilder changes
>  - Merge branch 'master' into JDK-8294960-invoke
>  - Apply suggestions from code review
>
>Co-authored-by: liach <7806504+li...@users.noreply.github.com>
>  - Apply suggestions from code review
>
>Co-authored-by: liach <7806504+li...@users.noreply.github.com>
>  - added missing comment
>  - fixed InnerClassLambdaMetafactory for hidden caller classes
>  - performance improvements
>  - consolidation of descriptors handling
>  - ... and 2 more: https://git.openjdk.org/jdk/compare/ee3b164d...304054b9

Thank you for the review.

> How is the performance result compared to using ASM?

I've actually triggered a large set of benchmarks to see the impact. Any hint 
which benchmarks to focus on would be welcome.

Thank you.

-

PR Comment: https://git.openjdk.org/jdk/pull/17108#issuecomment-2061616868

Re: RFR: 8330467: NoClassDefFoundError when lambda is in a hidden class [v2]

[Paul Sandoz]

On Wed, 17 Apr 2024 12:51:55 GMT, Adam Sotona  wrote:

>> Current implementation of `LambdaMetafactory` does not allow to use lambdas 
>> in hidden classes. Invocation throws `NoClassDefFoundError` instead.
>> 
>> This patch includes lambda implementation in a hidden class under the 
>> special handling of `useImplMethodHandle`.
>> The patch also fixes `j/l/i/defineHiddenClass/BasicTest::testLambda` to 
>> correctly cover this test case.
>> 
>> Please review.
>> 
>> Thanks,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   applied suggested changes

test/jdk/java/lang/invoke/defineHiddenClass/src/Lambda.java line 28:

> 26: public class Lambda implements HiddenTest {
> 27:  public void test() {
> 28:  Function f = o -> o.toString();

Recommend you retain the existing method reference e.g.:

 Function fmref = Object::toString;
 Function flexp = o -> o.toString();
 String s = fmref.apply(this) + flexp.apply(this);
 ...

Or split them out into two tests (easier to debug if case fails).

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18810#discussion_r1569060714

Re: RFR: 8328481: Implement Module Imports [v7]

[Jan Lahoda]

> This is an implementation of JEP JDK-8315129: Module Import Declarations 
> (Preview). Please see the JEP for details:
> https://bugs.openjdk.org/browse/JDK-8315129
> 
> It is mostly straightforward - the module imports are parsed, and then 
> expanded to import-on-demand in `TypeEnter`.
> There is a few notable aspects, however:
> - the AST node for import (`JCImport`) is holding the imported element as a 
> field access, because so far, the imported element always had to have a '.' 
> (even for import-on-demand). But for module imports, it is permissible to 
> import from a module whose name does not have a dot (`import module m;`). The 
> use of field access for ordinary import seems very useful, so I preferred to 
> keep that, and created a new internal-only AST node for module imports. There 
> is still only one public API AST node/interface, so this is purely an 
> implementation choice.
> - JShell now supports module imports as well; and the default, implicit, 
> script is changed to use it to import all of `java.base` if preview is 
> enabled. It is expected that the default would be changed if/when the module 
> imports feature is finalized.

Jan Lahoda has updated the pull request incrementally with one additional 
commit since the last revision:

  Cleanup.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18614/files
  - new: https://git.openjdk.org/jdk/pull/18614/files/c75b8b7a..27f9cfb5

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18614=06
 - incr: https://webrevs.openjdk.org/?repo=jdk=18614=05-06

  Stats: 8 lines in 1 file changed: 0 ins; 8 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/18614.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18614/head:pull/18614

PR: https://git.openjdk.org/jdk/pull/18614

Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v2]

[Matthias Baesken]

> We have already good JLI tracing capabilities. But GetApplicationHome and 
> GetApplicationHomeFromDll lack some tracing and should be enhanced.

Matthias Baesken has updated the pull request incrementally with one additional 
commit since the last revision:

  adjust trace messages

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18699/files
  - new: https://git.openjdk.org/jdk/pull/18699/files/128300a0..344d1b89

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18699=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=18699=00-01

  Stats: 10 lines in 3 files changed: 0 ins; 4 del; 6 mod
  Patch: https://git.openjdk.org/jdk/pull/18699.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18699/head:pull/18699

PR: https://git.openjdk.org/jdk/pull/18699

Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing

[Matthias Baesken]

On Tue, 9 Apr 2024 15:28:08 GMT, Matthias Baesken  wrote:

> We have already good JLI tracing capabilities. But GetApplicationHome and 
> GetApplicationHomeFromDll lack some tracing and should be enhanced.

I adjusted the trace messages a bit to make the coding more consistent to the 
existing Jli trace code,
I removed the print of the function name because this is usually avoided in the 
existing cases.
I removed the  'did not succeed' Jli trace messages in GetJREPath because in 
case of success we would getJli trace  output  ('JRE path is ')  so 
missing success output means it failed.

-

PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2061561008

Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v6]

[Adam Sotona]

> java.base java.lang.invoke package heavily uses ASM to generate lambdas and 
> method handles.
> 
> This patch converts ASM calls to Classfile API.
> 
> This PR is continuation of https://github.com/openjdk/jdk/pull/12945
> 
> Any comments and suggestions are welcome.
> 
> Please review.
> 
> Thank you,
> Adam

Adam Sotona has updated the pull request with a new target base due to a merge 
or a rebase. The incremental webrev excludes the unrelated changes brought in 
by the merge/rebase. The pull request contains 12 additional commits since the 
last revision:

 - applied suggested changes
 - Merge branch 'master' into JDK-8294960-invoke
 - Reversion of ClassBuilder changes
 - Merge branch 'master' into JDK-8294960-invoke
 - Apply suggestions from code review
   
   Co-authored-by: liach <7806504+li...@users.noreply.github.com>
 - Apply suggestions from code review
   
   Co-authored-by: liach <7806504+li...@users.noreply.github.com>
 - added missing comment
 - fixed InnerClassLambdaMetafactory for hidden caller classes
 - performance improvements
 - consolidation of descriptors handling
 - ... and 2 more: https://git.openjdk.org/jdk/compare/5270f351...304054b9

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/17108/files
  - new: https://git.openjdk.org/jdk/pull/17108/files/500bb8f0..304054b9

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=17108=05
 - incr: https://webrevs.openjdk.org/?repo=jdk=17108=04-05

  Stats: 34095 lines in 899 files changed: 18206 ins; 9917 del; 5972 mod
  Patch: https://git.openjdk.org/jdk/pull/17108.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17108/head:pull/17108

PR: https://git.openjdk.org/jdk/pull/17108

Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v5]

[Adam Sotona]

On Tue, 16 Apr 2024 22:49:28 GMT, Mandy Chung  wrote:

>> Adam Sotona has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains 10 additional 
>> commits since the last revision:
>> 
>>  - Reversion of ClassBuilder changes
>>  - Merge branch 'master' into JDK-8294960-invoke
>>  - Apply suggestions from code review
>>
>>Co-authored-by: liach <7806504+li...@users.noreply.github.com>
>>  - Apply suggestions from code review
>>
>>Co-authored-by: liach <7806504+li...@users.noreply.github.com>
>>  - added missing comment
>>  - fixed InnerClassLambdaMetafactory for hidden caller classes
>>  - performance improvements
>>  - consolidation of descriptors handling
>>  - InvokerBytecodeGenerator::emit... improvements
>>  - 8294960: Convert java.base/java.lang.invoke package to use the Classfile 
>> API to generate lambdas and method handlers
>
> src/java.base/share/classes/java/lang/invoke/ClassSpecializer.java line 32:
> 
>> 30: import java.lang.classfile.attribute.SourceFileAttribute;
>> 31: import java.lang.constant.ClassDesc;
>> 32: import static java.lang.constant.ConstantDescs.*;
> 
> Nit: this static imports can be moved to line 50 grouped with other static 
> imports.

Fixed, thank you.

> src/java.base/share/classes/java/lang/invoke/ClassSpecializer.java line 53:
> 
>> 51: import static java.lang.invoke.MethodHandleStatics.*;
>> 52: import static java.lang.invoke.MethodHandles.Lookup.IMPL_LOOKUP;
>> 53: import static java.lang.classfile.ClassFile.*;
> 
> Nit: nice to sort these in alphabetical order - this before line 50.

Fixed, thank you.

> src/java.base/share/classes/java/lang/invoke/ClassSpecializer.java line 606:
> 
>> 604: TRANSFORM_MODS = List.of(tms.toArray(new Integer[0]));
>> 605: }
>> 606: private static final MethodTypeDesc MTD_transformHelper = 
>> MethodTypeDesc.of(CD_MethodHandle, CD_int);
> 
> Suggestion:
> 
> private static final MethodTypeDesc MTD_TRANFORM_HELPER = 
> MethodTypeDesc.of(CD_MethodHandle, CD_int);

Fixed, thank you.

> src/java.base/share/classes/java/lang/invoke/GenerateJLIClassesHelper.java 
> line 72:
> 
>> 70: static final String INVOKERS_HOLDER_CLASS_NAME = 
>> INVOKERS_HOLDER.replace('/', '.');
>> 71: static final String BMH_SPECIES_PREFIX = 
>> "java.lang.invoke.BoundMethodHandle$Species_";
>> 72: private static final ClassFile CC = ClassFile.of();
> 
> `ClassFile.of()` returns the ClassFile with default context which is a 
> singleton object.  Is this static variable needed?

Right, it is no more necessary.

> src/java.base/share/classes/java/lang/invoke/GenerateJLIClassesHelper.java 
> line 526:
> 
>> 524: clb.withFlags(ACC_PRIVATE | ACC_FINAL | ACC_SUPER)
>> 525:
>> .withSuperclass(InvokerBytecodeGenerator.INVOKER_SUPER_DESC)
>> 526:
>> .with(SourceFileAttribute.of(clb.constantPool().utf8Entry(className.substring(className.lastIndexOf('/')
>>  + 1;
> 
> Is it because of performance reason to use 
> `SourceFileAttribute.of(Utf8Entry)` rather than 
> `SourceFileAttribute.of(String)`?   If so, a comment to explain would help.

It is redundant relic. Removed, thank you.

> src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java 
> line 36:
> 
>> 34: import java.lang.classfile.ClassBuilder;
>> 35: import java.lang.classfile.ClassFile;
>> 36: import static java.lang.classfile.ClassFile.*;
> 
> Nit: group static imports in line 52 following this file convention.

Fixed, thank you.

> src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java 
> line 41:
> 
>> 39: import java.lang.constant.ClassDesc;
>> 40: import java.lang.constant.ConstantDesc;
>> 41: import static java.lang.constant.ConstantDescs.*;
> 
> Nit: group static imports following this file convention

Fixed, thank you.

> src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java 
> line 57:
> 
>> 55: import java.lang.classfile.constantpool.FieldRefEntry;
>> 56: import java.lang.classfile.constantpool.InterfaceMethodRefEntry;
>> 57: import java.lang.classfile.constantpool.MethodRefEntry;
> 
> Nit: group these imports with the above import java.lang.classfile...

Fixed, thank you.

> src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 28:
> 
>> 26: package java.lang.invoke;
>> 27: 
>> 28: import static java.lang.classfile.ClassFile.*;
> 
> Nit: move this static import to line 64 following this file convention.

Fixed, thank you.

> src/java.base/share/classes/java/lang/invoke/TypeConvertingMethodAdapter.java 
> line 75:
> 
>> 73: UNBOX_DOUBLE  = unbox(CD_Number, 
>> "doubleValue", CD_double);
>> 74: 
>> 75: static private TypeKind primitiveTypeKindFromClass(Class type) {
> 
> Suggestion:
> 
> private static TypeKind 

Re: RFR: 8328481: Implement Module Imports [v6]

[Jan Lahoda]

> This is an implementation of JEP JDK-8315129: Module Import Declarations 
> (Preview). Please see the JEP for details:
> https://bugs.openjdk.org/browse/JDK-8315129
> 
> It is mostly straightforward - the module imports are parsed, and then 
> expanded to import-on-demand in `TypeEnter`.
> There is a few notable aspects, however:
> - the AST node for import (`JCImport`) is holding the imported element as a 
> field access, because so far, the imported element always had to have a '.' 
> (even for import-on-demand). But for module imports, it is permissible to 
> import from a module whose name does not have a dot (`import module m;`). The 
> use of field access for ordinary import seems very useful, so I preferred to 
> keep that, and created a new internal-only AST node for module imports. There 
> is still only one public API AST node/interface, so this is purely an 
> implementation choice.
> - JShell now supports module imports as well; and the default, implicit, 
> script is changed to use it to import all of `java.base` if preview is 
> enabled. It is expected that the default would be changed if/when the module 
> imports feature is finalized.

Jan Lahoda has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now contains 20 commits:

 - Merge branch 'master' into module-imports
 - Including current module name as suggested.
 - Adding more tests for ambiguities.
 - Merge branch 'master' into module-imports
 - Merge branch 'master' into module-imports
 - Merge branch 'master' into module-imports
 - Fixing test.
 - Fixing disambiguation of module imports.
 - Fixing file name computation.
 - Merge branch 'master' into module-imports
 - ... and 10 more: https://git.openjdk.org/jdk/compare/03e84178...c75b8b7a

-

Changes: https://git.openjdk.org/jdk/pull/18614/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk=18614=05
  Stats: 1198 lines in 29 files changed: 1130 ins; 9 del; 59 mod
  Patch: https://git.openjdk.org/jdk/pull/18614.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18614/head:pull/18614

PR: https://git.openjdk.org/jdk/pull/18614

Vector (and integer) API: unsigned min/max

[David Lloyd]

I've been trying the the incubating Vector API and one thing I've missed on
integer-typed vectors is having unsigned min/max operations. There is a
signed min/max operation, and unsigned comparison, but no unsigned min/max.

I guess this is for symmetry with `Math`, which only has signed
`min`/`max`. However, I would point out that while it's not very hard to
implement one's own unsigned min/max for integer types using
`compareUnsigned`, it is a bit harder to do so with vectors. The best I've
come up with is to take one of two approaches:

1. Zero-extend the vector to the next-larger size, perform the min/max, and
reduce it back down again, or
2. Add .MIN_VALUE, min/max with a value or vector also offset by
.MIN_VALUE, and then subtract the offset again

I guess a third approach could be to do a comparison using unsigned
compares, and then use the resultant vector mask to select items from the
original two vectors, but I didn't bother to work out this solution given I
already had the other two options.

Would it be feasible to add unsigned min/max operations for vectors? It
looks like at least AArch64 has support for this as a single instruction,
so it doesn't seem too outlandish.

And as a separate (but related) question, what about
`Math.minUnsigned`/`Math.maxUnsigned` of `int` and `long` for symmetry?

-- 
- DML • he/him

Integrated: 8329948: Remove string template feature

[Maurizio Cimadamore]

On Tue, 9 Apr 2024 11:34:45 GMT, Maurizio Cimadamore  
wrote:

> This PR removes support for the string template feature from the Java 
> compiler and the Java SE API, as discussed here:
> 
> https://mail.openjdk.org/pipermail/amber-spec-experts/2024-April/004106.html

This pull request has now been integrated.

Changeset: 03e84178
Author:Maurizio Cimadamore 
URL:   
https://git.openjdk.org/jdk/commit/03e84178ebfd2ca48b89d65d8f3c291e0c622fb5
Stats: 7400 lines in 66 files changed: 122 ins; 7217 del; 61 mod

8329948: Remove string template feature

Reviewed-by: jlahoda

-

PR: https://git.openjdk.org/jdk/pull/18688

Re: RFR: 8329581: Java launcher no longer prints a stack trace

[wolfseifert]

On Mon, 15 Apr 2024 18:25:02 GMT, Sonia Zaldana Calles  
wrote:

> Hi folks, 
> 
> This PR aims to fix 
> [JDK-8329581](https://bugs.openjdk.org/browse/JDK-8329581). 
> 
> I think the regression got introduced in 
> [JDK-8315458](https://bugs.openjdk.org/browse/JDK-8315458). 
> 
> In the issue linked above, 
> [LauncherHelper#getMainType](https://github.com/openjdk/jdk/pull/16461/files#diff-108a3a3e3c2d108c8c7f19ea498f641413b7c9239ecd2975a6c27d904c2ba226)
>  got removed to simplify launcher code.
> 
> Previously, we used ```getMainType``` to do the appropriate main method 
> invocation in ```JavaMain```. However, we currently attempt to do all types 
> of main method invocations at the same time 
> [here](https://github.com/openjdk/jdk/blob/master/src/java.base/share/native/libjli/java.c#L623).
>  
> 
> Note how all of these invocations clear the exception reported with 
> [CHECK_EXCEPTION_FAIL](https://github.com/openjdk/jdk/blob/140f56718bbbfc31bb0c39255c68568fad285a1f/src/java.base/share/native/libjli/java.c#L390).
>  
> 
> Therefore, if a legitimate exception comes up during one of these 
> invocations, it does not get reported. 
> 
> I propose reintroducing ```LauncherHelper#getMainType``` but I'm looking 
> forward to your suggestions. 
> 
> Cheers, 
> Sonia

I tested now pull/18753 and pull/18786: both solve both issues JDK-8329420 and 
JDK-8329581

-

PR Comment: https://git.openjdk.org/jdk/pull/18786#issuecomment-2061302794

Re: RFR: 8329420: Java 22 (and 23) launcher calls default constructor although main() is static [v4]

[wolfseifert]

On Wed, 17 Apr 2024 09:20:24 GMT, Jan Lahoda  wrote:

>> Consider code like:
>> 
>> public class MainClass {
>> public MainClass() {
>> System.out.println("Constructor called!");
>> }
>> public static void main() {
>> System.out.println("main called!");
>> }
>> }
>> 
>> and compile and run it, with preview enabled, like:
>> 
>> $ javac /tmp/MainClass.java 
>> $ java --enable-preview -classpath /tmp MainClass
>> Constructor called!
>> main called!
>> 
>> 
>> That is wrong, as the `main` method is static, and there is no need to 
>> create a new instance of the class.
>> 
>> The reason is that as launcher attempts to invoke the main method, it goes 
>> in the following order: 1) static-main-with-args; 2) 
>> instance-main-with-args; 3) static-main-without-args; 4) 
>> instance-main-without-args. But, for the instance variants, the code first 
>> creates a new instance of the given class, and only then attempts to lookup 
>> the `main` method, and will pass to the next option when the `main` method 
>> lookup fails. So, when invoking static-main-without-args, the current class 
>> instance may be created for instance-main-with-args, which will then fail 
>> due to the missing `main(String[])` method.
>> 
>> The proposed solution to this problem is to simply first do a lookup for the 
>> `main` method (skipping to the next variant when the given main method does 
>> not exist, without instantiating the current class).
>> 
>> There is also a relatively closely related problem: what happens when the 
>> constructor throws an exception?
>> 
>> public class MainClass {
>> public MainClass() {
>> if (true) throw new RuntimeException();
>> }
>> public void main() {
>> System.out.println("main called!");
>> }
>> }
>> 
>> 
>> when compiled an run, this produces no output whatsoever:
>> 
>> $ javac /tmp/MainClass.java 
>> $ java --enable-preview -classpath /tmp MainClass
>> $
>> 
>> 
>> This is because any exceptions thrown from the constructor are effectively 
>> ignored, and the launcher will continue with the next variant. This seems 
>> wrong - the exception should be printed for the user, like:
>> 
>> $ java --enable-preview -classpath /tmp/ MainClass 
>> Exception in thread "main" java.lang.RuntimeException
>> at MainClass.(MainClass.java:3)
>> 
>> 
>> This patch proposes to do that by not consuming the exceptions thrown from 
>> the constructor, and stop the propagation to the next variant.
>
> Jan Lahoda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fixing typo (should not continue after an exception from a constructor).

I tested now pull/18753 and pull/18786: both solve both issues JDK-8329420 and 
JDK-8329581

-

PR Comment: https://git.openjdk.org/jdk/pull/18753#issuecomment-2061301645

Re: RFR: 8327964: Simplify BigInteger.implMultiplyToLen intrinsic [v3]

[Damon Fenacci]

On Tue, 26 Mar 2024 15:59:33 GMT, Damon Fenacci  wrote:

>> Yudi Zheng has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   address comment.
>
> `multiply_to_len` seems to be used by `generate_squareToLen` as well for 
> aarch64 and riscv but `zlen` is still passed in a register.
> 
> https://github.com/openjdk/jdk/blob/870a6127cf54264c691f7322d775b202705c3bfa/src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp#L4710
> https://github.com/openjdk/jdk/blob/870a6127cf54264c691f7322d775b202705c3bfa/src/hotspot/cpu/riscv/stubGenerator_riscv.cpp#L2881
> 
> I think it might work anyway but it might be better to adapt them if only for 
> completeness.

> @dafedafe @dean-long please take a look and let me know if there are further 
> issues, thanks!

Thanks @mur47x111! I noticed that you found even a few more `zlen` usages  

Did you test the change against all affected platforms?

-

PR Comment: https://git.openjdk.org/jdk/pull/18226#issuecomment-2061301421

Re: RFR: 8319516: AIX System::loadLibrary needs support to load a shared library from an archive object [v25]

[Suchismith Roy]

On Mon, 15 Apr 2024 13:58:38 GMT, Jaikiran Pai  wrote:

>> Suchismith Roy has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Files copy
>
> I'll let Mandy and others familiar with this area and AIX do the formal 
> review of this PR. My involvement here was merely general inputs.

Thank you @jaikiran @mlchung @TheRealMDoerr  Got to learn about new ways to 
test and learnt about the classloader functionalities better.

-

PR Comment: https://git.openjdk.org/jdk/pull/17945#issuecomment-2061282909

Re: RFR: 8316493: Remove the caching fields in AbstractMap [v11]

[Claes Redestad]

On Fri, 10 Nov 2023 08:17:22 GMT, Per Minborg  wrote:

>> This PR outlines a solution for making immutable maps `@ValueBased` by 
>> removing cacheing of certain values in `AbstractMap`.
>> 
>> By removing these caching fields in `AbstractMap`, we can make the immutable 
>> maps `@ValueBased` and at the same time, performance is likely improved 
>> because the JVM is probably able to optimize away object creation anyway via 
>> escape analysis. Also, all maps will occupy less space as we get rid of a 
>> number of objects and references stored for each map.
>> 
>> We need to benchmark this solution to better understand its implications.
>
> Per Minborg has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 14 commits:
> 
>  - Merge branch 'master' into vb-map2
>  - Fix formatting
>  - Remove caching in TreeMap
>  - Remove caching from CHM and CSLM
>  - Move back clone to original position
>  - Reintroduce AbstractMap::clone
>  - Add 'fresh' to implSpec
>  - Remove AbstractMap::clone
>  - Merge master
>  - Merge branch 'master' into vb-map2
>  - ... and 4 more: https://git.openjdk.org/jdk/compare/9cce9fe0...b1bfcd17

Marked as reviewed by redestad (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/15614#pullrequestreview-2006046097

Re: RFR: 8316493: Remove the caching fields in AbstractMap [v11]

[Chen Liang]

On Fri, 10 Nov 2023 08:17:22 GMT, Per Minborg  wrote:

>> This PR outlines a solution for making immutable maps `@ValueBased` by 
>> removing cacheing of certain values in `AbstractMap`.
>> 
>> By removing these caching fields in `AbstractMap`, we can make the immutable 
>> maps `@ValueBased` and at the same time, performance is likely improved 
>> because the JVM is probably able to optimize away object creation anyway via 
>> escape analysis. Also, all maps will occupy less space as we get rid of a 
>> number of objects and references stored for each map.
>> 
>> We need to benchmark this solution to better understand its implications.
>
> Per Minborg has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 14 commits:
> 
>  - Merge branch 'master' into vb-map2
>  - Fix formatting
>  - Remove caching in TreeMap
>  - Remove caching from CHM and CSLM
>  - Move back clone to original position
>  - Reintroduce AbstractMap::clone
>  - Add 'fresh' to implSpec
>  - Remove AbstractMap::clone
>  - Merge master
>  - Merge branch 'master' into vb-map2
>  - ... and 4 more: https://git.openjdk.org/jdk/compare/9cce9fe0...b1bfcd17

Marked as reviewed by liach (Author).

-

PR Review: https://git.openjdk.org/jdk/pull/15614#pullrequestreview-2006050183

Re: RFR: 8330467: NoClassDefFoundError when lambda is in a hidden class [v2]

[Chen Liang]

On Wed, 17 Apr 2024 10:58:59 GMT, Jorn Vernee  wrote:

> Do we also need to check if the parameters or return type are hidden classes?

I think this is already failing and will continue to fail. Don't think we need 
special error messages for this case.

-

PR Comment: https://git.openjdk.org/jdk/pull/18810#issuecomment-2061260710

Re: RFR: 8319516: AIX System::loadLibrary needs support to load a shared library from an archive object [v29]

[Martin Doerr]

On Wed, 17 Apr 2024 11:20:24 GMT, Suchismith Roy  wrote:

>> Allow support for both .a and .so files in AIX.
>> If .so file is not found, allow fallback to .a extension.
>> JBS Issue: [JDK-8319516](https://bugs.openjdk.org/browse/JDK-8319516)
>
> Suchismith Roy has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update LoadAIXLibraryFromArchiveObject.java
>   
>   Nits,coding style and variable name

Update looks good.
Thanks everybody for all the discussions!

-

Marked as reviewed by mdoerr (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17945#pullrequestreview-2006028030

Re: RFR: 8324651: Compiler Implementation for Derived Record Creation (Preview) [v10]

[Maurizio Cimadamore]

On Tue, 9 Apr 2024 09:28:23 GMT, Jan Lahoda  wrote:

>> This is a patch for javac, that adds the Derived Record Creation 
>> expressions. The current draft specification for the feature is:
>> https://cr.openjdk.org/~gbierman/jep468/jep468-20240326/specs/derived-record-creation-jls.html
>> 
>> The current CSR is here:
>> https://bugs.openjdk.org/browse/JDK-8328637
>> 
>> The patch is mostly straightforward, with two notable changes:
>>  - there is a new `ElementKind.COMPONENT_LOCAL_VARIABLE`, as the 
>> specification introduces this term, and it seems consistent with 
>> `ElementKind.BINDING_VARIABLE` that was introduced some time ago.
>>  - there are a bit broader changes in `Flow`, to facilitate the introduction 
>> of variables without an explicit declaration for definite assignment and 
>> effectively final computation.
>
> Jan Lahoda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Reflecting review feedback:
>   - reverting unnecessary changes in TransPatterns
>   - moving the patters/withers/Model test to a more appropriate place

Looks good!

-

Marked as reviewed by mcimadamore (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18509#pullrequestreview-2005974556

Re: RFR: 8329581: Java launcher no longer prints a stack trace

[wolfseifert]

On Mon, 15 Apr 2024 18:25:02 GMT, Sonia Zaldana Calles  
wrote:

> Hi folks, 
> 
> This PR aims to fix 
> [JDK-8329581](https://bugs.openjdk.org/browse/JDK-8329581). 
> 
> I think the regression got introduced in 
> [JDK-8315458](https://bugs.openjdk.org/browse/JDK-8315458). 
> 
> In the issue linked above, 
> [LauncherHelper#getMainType](https://github.com/openjdk/jdk/pull/16461/files#diff-108a3a3e3c2d108c8c7f19ea498f641413b7c9239ecd2975a6c27d904c2ba226)
>  got removed to simplify launcher code.
> 
> Previously, we used ```getMainType``` to do the appropriate main method 
> invocation in ```JavaMain```. However, we currently attempt to do all types 
> of main method invocations at the same time 
> [here](https://github.com/openjdk/jdk/blob/master/src/java.base/share/native/libjli/java.c#L623).
>  
> 
> Note how all of these invocations clear the exception reported with 
> [CHECK_EXCEPTION_FAIL](https://github.com/openjdk/jdk/blob/140f56718bbbfc31bb0c39255c68568fad285a1f/src/java.base/share/native/libjli/java.c#L390).
>  
> 
> Therefore, if a legitimate exception comes up during one of these 
> invocations, it does not get reported. 
> 
> I propose reintroducing ```LauncherHelper#getMainType``` but I'm looking 
> forward to your suggestions. 
> 
> Cheers, 
> Sonia

Just tried to avoid unnecessary duplication of work.

-

PR Comment: https://git.openjdk.org/jdk/pull/18786#issuecomment-2061191285

Re: RFR: 8330467: NoClassDefFoundError when lambda is in a hidden class [v2]

[Adam Sotona]

> Current implementation of `LambdaMetafactory` does not allow to use lambdas 
> in hidden classes. Invocation throws `NoClassDefFoundError` instead.
> 
> This patch includes lambda implementation in a hidden class under the special 
> handling of `useImplMethodHandle`.
> The patch also fixes `j/l/i/defineHiddenClass/BasicTest::testLambda` to 
> correctly cover this test case.
> 
> Please review.
> 
> Thanks,
> Adam

Adam Sotona has updated the pull request incrementally with one additional 
commit since the last revision:

  applied suggested changes

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18810/files
  - new: https://git.openjdk.org/jdk/pull/18810/files/4a53389d..c637ab90

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18810=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=18810=00-01

  Stats: 6 lines in 1 file changed: 0 ins; 0 del; 6 mod
  Patch: https://git.openjdk.org/jdk/pull/18810.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18810/head:pull/18810

PR: https://git.openjdk.org/jdk/pull/18810

Re: RFR: 8330467: NoClassDefFoundError when lambda is in a hidden class

[Adam Sotona]

On Wed, 17 Apr 2024 09:14:31 GMT, Chen Liang  wrote:

>> Current implementation of `LambdaMetafactory` does not allow to use lambdas 
>> in hidden classes. Invocation throws `NoClassDefFoundError` instead.
>> 
>> This patch includes lambda implementation in a hidden class under the 
>> special handling of `useImplMethodHandle`.
>> The patch also fixes `j/l/i/defineHiddenClass/BasicTest::testLambda` to 
>> correctly cover this test case.
>> 
>> Please review.
>> 
>> Thanks,
>> Adam
>
> src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java 
> line 186:
> 
>> 184:!VerifyAccess.isSamePackage(targetClass, 
>> implInfo.getDeclaringClass())) ||
>> 185:implKind == H_INVOKESPECIAL ||
>> 186:implInfo.getDeclaringClass().isHidden();
> 
> Might need to check the difference between `implInfo.getDeclaringClass()` and 
> `implClass`

Yes, we should support only invoke static refs for hidden classes and 
`implClass == implInfo.getDeclaringClass()` in that case.
Thanks!

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18810#discussion_r1568782761

Re: RFR: 8327964: Simplify BigInteger.implMultiplyToLen intrinsic [v3]

[Yudi Zheng]

On Tue, 26 Mar 2024 15:59:33 GMT, Damon Fenacci  wrote:

>> Yudi Zheng has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   address comment.
>
> `multiply_to_len` seems to be used by `generate_squareToLen` as well for 
> aarch64 and riscv but `zlen` is still passed in a register.
> 
> https://github.com/openjdk/jdk/blob/870a6127cf54264c691f7322d775b202705c3bfa/src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp#L4710
> https://github.com/openjdk/jdk/blob/870a6127cf54264c691f7322d775b202705c3bfa/src/hotspot/cpu/riscv/stubGenerator_riscv.cpp#L2881
> 
> I think it might work anyway but it might be better to adapt them if only for 
> completeness.

@dafedafe @dean-long please take a look and let me know if there are further 
issues, thanks!

-

PR Comment: https://git.openjdk.org/jdk/pull/18226#issuecomment-2061162283

Re: RFR: 8327964: Simplify BigInteger.implMultiplyToLen intrinsic [v5]

[Yudi Zheng]

> Moving array construction within BigInteger.implMultiplyToLen intrinsic 
> candidate to its caller simplifies the intrinsic implementation in JIT 
> compiler.

Yudi Zheng has updated the pull request incrementally with one additional 
commit since the last revision:

  address comment.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18226/files
  - new: https://git.openjdk.org/jdk/pull/18226/files/c5d521dc..72ba58ce

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18226=04
 - incr: https://webrevs.openjdk.org/?repo=jdk=18226=03-04

  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/18226.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18226/head:pull/18226

PR: https://git.openjdk.org/jdk/pull/18226

Re: RFR: 8329997: Add provisions for checking memory segment alignment constraints [v6]

[Per Minborg]

> This PR proposes to add a new method `MemorySegment::maxByteAlignment` that 
> returns the maximum byte alignment of a segment (both heap and native 
> segments).
> 
> Clients can then use this method to determine if a segment is properly 
> aligned for any given layout (e.g. following a `MemorySegment::reinterpret` 
> operation).

Per Minborg has updated the pull request incrementally with one additional 
commit since the last revision:

  Update docs for maxByteAlignment

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18779/files
  - new: https://git.openjdk.org/jdk/pull/18779/files/e971e7f6..8b944a35

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18779=05
 - incr: https://webrevs.openjdk.org/?repo=jdk=18779=04-05

  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/18779.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18779/head:pull/18779

PR: https://git.openjdk.org/jdk/pull/18779

Re: RFR: 8327964: Simplify BigInteger.implMultiplyToLen intrinsic [v4]

[Yudi Zheng]

> Moving array construction within BigInteger.implMultiplyToLen intrinsic 
> candidate to its caller simplifies the intrinsic implementation in JIT 
> compiler.

Yudi Zheng has updated the pull request incrementally with one additional 
commit since the last revision:

  address comment.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18226/files
  - new: https://git.openjdk.org/jdk/pull/18226/files/870a6127..c5d521dc

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18226=03
 - incr: https://webrevs.openjdk.org/?repo=jdk=18226=02-03

  Stats: 53 lines in 10 files changed: 3 ins; 6 del; 44 mod
  Patch: https://git.openjdk.org/jdk/pull/18226.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18226/head:pull/18226

PR: https://git.openjdk.org/jdk/pull/18226

Re: RFR: 8314592: Add shortcut to SymbolLookup::find [v5]

[Per Minborg]

> While `SymbolLookup` correctly uses an `Optional` return to denote whether a 
> symbol has been found by the lookup or not (which enables composition of 
> symbol lookups), many clients end up just calling `Optional::get`, or 
> `Optional::orElseThrow()` on the result.
> 
> This PR proposes to add a convenience method `SymbolLookup::findOrThrow` that 
> will do a lookup and, if no symbol can be found, throws an 
> `IllegalArgumentException` with a relevant exception message.

Per Minborg has updated the pull request incrementally with one additional 
commit since the last revision:

  Update src/java.base/share/classes/java/lang/foreign/package-info.java
  
  Co-authored-by: Jorn Vernee 

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18474/files
  - new: https://git.openjdk.org/jdk/pull/18474/files/fa86d837..2ebac9fc

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18474=04
 - incr: https://webrevs.openjdk.org/?repo=jdk=18474=03-04

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/18474.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18474/head:pull/18474

PR: https://git.openjdk.org/jdk/pull/18474

Re: RFR: 8319516: AIX System::loadLibrary needs support to load a shared library from an archive object [v29]

[Suchismith Roy]

> Allow support for both .a and .so files in AIX.
> If .so file is not found, allow fallback to .a extension.
> JBS Issue: [JDK-8319516](https://bugs.openjdk.org/browse/JDK-8319516)

Suchismith Roy has updated the pull request incrementally with one additional 
commit since the last revision:

  Update LoadAIXLibraryFromArchiveObject.java
  
  Nits,coding style and variable name

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/17945/files
  - new: https://git.openjdk.org/jdk/pull/17945/files/2b729673..53343781

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=17945=28
 - incr: https://webrevs.openjdk.org/?repo=jdk=17945=27-28

  Stats: 7 lines in 1 file changed: 0 ins; 0 del; 7 mod
  Patch: https://git.openjdk.org/jdk/pull/17945.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17945/head:pull/17945

PR: https://git.openjdk.org/jdk/pull/17945

Re: RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant [v3]

[Jaikiran Pai]

On Wed, 10 Apr 2024 16:17:32 GMT, Jaikiran Pai  wrote:

>> Can I please get a review of this change which proposes to fix the issue 
>> noted in https://bugs.openjdk.org/browse/JDK-8212895?
>> 
>> As noted in that issue, the `ChronoField.INSTANT_SECONDS` currently is 
>> initialized to have a minimum and maximum values of `Long.MIN_VALUE` and 
>> `LONG.MAX_VALUE` respectively. However, `java.time.Instant` only supports 
>> `-31557014167219200L` and `31556889864403199L` as minimum and maximum values 
>> for the epoch second.
>> 
>> The commit in this PR updates the `ChronoField.INSTANT_SECONDS`'s value 
>> range to match the supported min and max values of `Instant` (as suggested 
>> by Stephen in that JBS issue). This commit also introduces a test to verify 
>> this change. This new test method as well as existing tests in tier1, tier2 
>> and tier3 continue to pass with this change.
>
> Jaikiran Pai has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   no need for {@code} in javadoc

Thank you Naoto and Roger for the inputs and the reviews.

-

PR Comment: https://git.openjdk.org/jdk/pull/18674#issuecomment-2060988355

Re: RFR: 8330467: NoClassDefFoundError when lambda is in a hidden class

[Jorn Vernee]

On Wed, 17 Apr 2024 08:46:59 GMT, Adam Sotona  wrote:

> Current implementation of `LambdaMetafactory` does not allow to use lambdas 
> in hidden classes. Invocation throws `NoClassDefFoundError` instead.
> 
> This patch includes lambda implementation in a hidden class under the special 
> handling of `useImplMethodHandle`.
> The patch also fixes `j/l/i/defineHiddenClass/BasicTest::testLambda` to 
> correctly cover this test case.
> 
> Please review.
> 
> Thanks,
> Adam

Do we also need to check if the parameters or return type are hidden classes? I 
suppose if we compile a direct bytecode reference to the target method, all of 
them need to be referenceable from bytecode, not just the holder.

-

PR Review: https://git.openjdk.org/jdk/pull/18810#pullrequestreview-2005733945

Integrated: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant

[Jaikiran Pai]

On Mon, 8 Apr 2024 09:52:39 GMT, Jaikiran Pai  wrote:

> Can I please get a review of this change which proposes to fix the issue 
> noted in https://bugs.openjdk.org/browse/JDK-8212895?
> 
> As noted in that issue, the `ChronoField.INSTANT_SECONDS` currently is 
> initialized to have a minimum and maximum values of `Long.MIN_VALUE` and 
> `LONG.MAX_VALUE` respectively. However, `java.time.Instant` only supports 
> `-31557014167219200L` and `31556889864403199L` as minimum and maximum values 
> for the epoch second.
> 
> The commit in this PR updates the `ChronoField.INSTANT_SECONDS`'s value range 
> to match the supported min and max values of `Instant` (as suggested by 
> Stephen in that JBS issue). This commit also introduces a test to verify this 
> change. This new test method as well as existing tests in tier1, tier2 and 
> tier3 continue to pass with this change.

This pull request has now been integrated.

Changeset: 89129e3f
Author:Jaikiran Pai 
URL:   
https://git.openjdk.org/jdk/commit/89129e3f672d8af9613ad2a72e64322661836c96
Stats: 17 lines in 2 files changed: 14 ins; 0 del; 3 mod

8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant

Reviewed-by: rriggs, naoto

-

PR: https://git.openjdk.org/jdk/pull/18674

Re: RFR: 8319516: AIX System::loadLibrary needs support to load a shared library from an archive object [v27]

[Suchismith Roy]

On Wed, 17 Apr 2024 10:42:46 GMT, Martin Doerr  wrote:

> Please take a look at Mandy's suggestions. I think they make sense.

Done. Sorry i missed it. I got notified that the patch had got approved so i 
went ahead with integrate.

-

PR Comment: https://git.openjdk.org/jdk/pull/17945#issuecomment-2060978059

Re: RFR: 8319516: AIX System::loadLibrary needs support to load a shared library from an archive object [v28]

[Suchismith Roy]

> Allow support for both .a and .so files in AIX.
> If .so file is not found, allow fallback to .a extension.
> JBS Issue: [JDK-8319516](https://bugs.openjdk.org/browse/JDK-8319516)

Suchismith Roy has updated the pull request incrementally with two additional 
commits since the last revision:

 - Update 
test/jdk/java/lang/RuntimeTests/loadLibrary/aix/LoadAIXLibraryFromArchiveObject.java
   
   remove final
   
   Co-authored-by: Mandy Chung 
 - Update 
test/jdk/java/lang/RuntimeTests/loadLibrary/aix/LoadAIXLibraryFromArchiveObject.java
   
   remove final
   
   Co-authored-by: Mandy Chung 

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/17945/files
  - new: https://git.openjdk.org/jdk/pull/17945/files/12270d06..2b729673

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=17945=27
 - incr: https://webrevs.openjdk.org/?repo=jdk=17945=26-27

  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/17945.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17945/head:pull/17945

PR: https://git.openjdk.org/jdk/pull/17945

Re: RFR: 8319516: AIX System::loadLibrary needs support to load a shared library from an archive object [v27]

[Martin Doerr]

On Mon, 15 Apr 2024 16:10:38 GMT, Suchismith Roy  wrote:

>> Allow support for both .a and .so files in AIX.
>> If .so file is not found, allow fallback to .a extension.
>> JBS Issue: [JDK-8319516](https://bugs.openjdk.org/browse/JDK-8319516)
>
> Suchismith Roy has updated the pull request incrementally with three 
> additional commits since the last revision:
> 
>  - new line
>  - spaces fix
>  - spaces fix

Please take a look at Mandy's suggestions. I think they make sense.

-

PR Comment: https://git.openjdk.org/jdk/pull/17945#issuecomment-2060963452

Integrated: 8330272: Wrong javadoc for ValueLayout.JAVA_LONG/JAVA_DOUBLE on x86 32bit

[Per Minborg]

On Tue, 16 Apr 2024 07:09:55 GMT, Per Minborg  wrote:

> This PR proposes to update the javadocs for `ValueLayout.JAVA_LONG` and 
> `ValueLayout.JAVA_DOUBLE` to reflect the changes made in 
> https://bugs.openjdk.org/browse/JDK-8326172  (we forgot to update the docs 
> when that issue was fixed)

This pull request has now been integrated.

Changeset: e4021adb
Author:Per Minborg 
URL:   
https://git.openjdk.org/jdk/commit/e4021adb287381a6c7775234b401429380075e0f
Stats: 5 lines in 1 file changed: 0 ins; 2 del; 3 mod

8330272: Wrong javadoc for ValueLayout.JAVA_LONG/JAVA_DOUBLE on x86 32bit

Reviewed-by: mcimadamore, jvernee

-

PR: https://git.openjdk.org/jdk/pull/18791

Re: RFR: 8330182: Start of release updates for JDK 24 [v2]

[Adam Sotona]

On Wed, 17 Apr 2024 05:43:12 GMT, Joe Darcy  wrote:

>> Get JDK 24 underway.
>
> Joe Darcy has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Correct release date as observed in review feedback.
>  - Improve javadoc of class file update.

ClassFile changes are OK.

-

Marked as reviewed by asotona (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18787#pullrequestreview-2005504192

Re: RFR: 8329420: Java 22 (and 23) launcher calls default constructor although main() is static [v4]

[Jan Lahoda]

> Consider code like:
> 
> public class MainClass {
> public MainClass() {
> System.out.println("Constructor called!");
> }
> public static void main() {
> System.out.println("main called!");
> }
> }
> 
> and compile and run it, with preview enabled, like:
> 
> $ javac /tmp/MainClass.java 
> $ java --enable-preview -classpath /tmp MainClass
> Constructor called!
> main called!
> 
> 
> That is wrong, as the `main` method is static, and there is no need to create 
> a new instance of the class.
> 
> The reason is that as launcher attempts to invoke the main method, it goes in 
> the following order: 1) static-main-with-args; 2) instance-main-with-args; 3) 
> static-main-without-args; 4) instance-main-without-args. But, for the 
> instance variants, the code first creates a new instance of the given class, 
> and only then attempts to lookup the `main` method, and will pass to the next 
> option when the `main` method lookup fails. So, when invoking 
> static-main-without-args, the current class instance may be created for 
> instance-main-with-args, which will then fail due to the missing 
> `main(String[])` method.
> 
> The proposed solution to this problem is to simply first do a lookup for the 
> `main` method (skipping to the next variant when the given main method does 
> not exist, without instantiating the current class).
> 
> There is also a relatively closely related problem: what happens when the 
> constructor throws an exception?
> 
> public class MainClass {
> public MainClass() {
> if (true) throw new RuntimeException();
> }
> public void main() {
> System.out.println("main called!");
> }
> }
> 
> 
> when compiled an run, this produces no output whatsoever:
> 
> $ javac /tmp/MainClass.java 
> $ java --enable-preview -classpath /tmp MainClass
> $
> 
> 
> This is because any exceptions thrown from the constructor are effectively 
> ignored, and the launcher will continue with the next variant. This seems 
> wrong - the exception should be printed for the user, like:
> 
> $ java --enable-preview -classpath /tmp/ MainClass 
> Exception in thread "main" java.lang.RuntimeException
> at MainClass.(MainClass.java:3)
> 
> 
> This patch proposes to do that by not consuming the exceptions thrown from 
> the constructor, and stop the propagation to the next variant.

Jan Lahoda has updated the pull request incrementally with one additional 
commit since the last revision:

  Fixing typo (should not continue after an exception from a constructor).

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18753/files
  - new: https://git.openjdk.org/jdk/pull/18753/files/c007f61e..aadfb380

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18753=03
 - incr: https://webrevs.openjdk.org/?repo=jdk=18753=02-03

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/18753.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18753/head:pull/18753

PR: https://git.openjdk.org/jdk/pull/18753

Re: RFR: 8329420: Java 22 (and 23) launcher calls default constructor although main() is static [v3]

[Jan Lahoda]

On Wed, 17 Apr 2024 08:51:39 GMT, Jaikiran Pai  wrote:

>> Jan Lahoda has updated the pull request incrementally with three additional 
>> commits since the last revision:
>> 
>>  - Reflecting code formatting suggestion.
>>  - First lookup the main method, and only then the constructor.
>>  - Attempting to solve JDK-8329581 by only ignoring j.l.NoSuchMethodError
>
> src/java.base/share/native/libjli/java.c line 477:
> 
>> 475: // and don't continue with the next variant;
>> 476: // leave any exception pending, so that it is visible to the 
>> caller:
>> 477: return 1;
> 
> Hello Jan, the comment says "don't continue", yet this returns `1` which 
> represents "continue". Should this return `0` instead?

Uh, you are right of course. Thanks for noticing! Fixed. (I believe the 
difference here is not currently observable, as this is the last step in the 
cascade, but may become observable if the order changes.)

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18753#discussion_r1568511740

Re: RFR: 8330467: NoClassDefFoundError when lambda is in a hidden class

[Chen Liang]

On Wed, 17 Apr 2024 08:46:59 GMT, Adam Sotona  wrote:

> Current implementation of `LambdaMetafactory` does not allow to use lambdas 
> in hidden classes. Invocation throws `NoClassDefFoundError` instead.
> 
> This patch includes lambda implementation in a hidden class under the special 
> handling of `useImplMethodHandle`.
> The patch also fixes `j/l/i/defineHiddenClass/BasicTest::testLambda` to 
> correctly cover this test case.
> 
> Please review.
> 
> Thanks,
> Adam

I wonder if we should only support invokeStatic refs for hidden classes, as 
instance call lambdas will always require receiver which can't be represented 
outside of hidden classes.

src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java 
line 186:

> 184:!VerifyAccess.isSamePackage(targetClass, 
> implInfo.getDeclaringClass())) ||
> 185:implKind == H_INVOKESPECIAL ||
> 186:implInfo.getDeclaringClass().isHidden();

Might need to check the difference between `implInfo.getDeclaringClass()` and 
`implClass`

-

PR Review: https://git.openjdk.org/jdk/pull/18810#pullrequestreview-2005483864
PR Review Comment: https://git.openjdk.org/jdk/pull/18810#discussion_r1568505421

Re: RFR: 8329420: Java 22 (and 23) launcher calls default constructor although main() is static [v3]

[Jaikiran Pai]

On Wed, 17 Apr 2024 06:34:25 GMT, Jan Lahoda  wrote:

>> Consider code like:
>> 
>> public class MainClass {
>> public MainClass() {
>> System.out.println("Constructor called!");
>> }
>> public static void main() {
>> System.out.println("main called!");
>> }
>> }
>> 
>> and compile and run it, with preview enabled, like:
>> 
>> $ javac /tmp/MainClass.java 
>> $ java --enable-preview -classpath /tmp MainClass
>> Constructor called!
>> main called!
>> 
>> 
>> That is wrong, as the `main` method is static, and there is no need to 
>> create a new instance of the class.
>> 
>> The reason is that as launcher attempts to invoke the main method, it goes 
>> in the following order: 1) static-main-with-args; 2) 
>> instance-main-with-args; 3) static-main-without-args; 4) 
>> instance-main-without-args. But, for the instance variants, the code first 
>> creates a new instance of the given class, and only then attempts to lookup 
>> the `main` method, and will pass to the next option when the `main` method 
>> lookup fails. So, when invoking static-main-without-args, the current class 
>> instance may be created for instance-main-with-args, which will then fail 
>> due to the missing `main(String[])` method.
>> 
>> The proposed solution to this problem is to simply first do a lookup for the 
>> `main` method (skipping to the next variant when the given main method does 
>> not exist, without instantiating the current class).
>> 
>> There is also a relatively closely related problem: what happens when the 
>> constructor throws an exception?
>> 
>> public class MainClass {
>> public MainClass() {
>> if (true) throw new RuntimeException();
>> }
>> public void main() {
>> System.out.println("main called!");
>> }
>> }
>> 
>> 
>> when compiled an run, this produces no output whatsoever:
>> 
>> $ javac /tmp/MainClass.java 
>> $ java --enable-preview -classpath /tmp MainClass
>> $
>> 
>> 
>> This is because any exceptions thrown from the constructor are effectively 
>> ignored, and the launcher will continue with the next variant. This seems 
>> wrong - the exception should be printed for the user, like:
>> 
>> $ java --enable-preview -classpath /tmp/ MainClass 
>> Exception in thread "main" java.lang.RuntimeException
>> at MainClass.(MainClass.java:3)
>> 
>> 
>> This patch proposes to do that by not consuming the exceptions thrown from 
>> the constructor, and stop the propagation to the next variant.
>
> Jan Lahoda has updated the pull request incrementally with three additional 
> commits since the last revision:
> 
>  - Reflecting code formatting suggestion.
>  - First lookup the main method, and only then the constructor.
>  - Attempting to solve JDK-8329581 by only ignoring j.l.NoSuchMethodError

src/java.base/share/native/libjli/java.c line 477:

> 475: // and don't continue with the next variant;
> 476: // leave any exception pending, so that it is visible to the 
> caller:
> 477: return 1;

Hello Jan, the comment says "don't continue", yet this returns `1` which 
represents "continue". Should this return `0` instead?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18753#discussion_r1568470228

RFR: 8330467: NoClassDefFoundError when lambda is in a hidden class

[Adam Sotona]

Current implementation of `LambdaMetafactory` does not allow to use lambdas in 
hidden classes. Invocation throws `NoClassDefFoundError` instead.

This patch includes lambda implementation in a hidden class under the special 
handling of `useImplMethodHandle`.
The patch also fixes `j/l/i/defineHiddenClass/BasicTest::testLambda` to 
correctly cover this test case.

Please review.

Thanks,
Adam

-

Commit messages:
 - 8330467: NoClassDefFoundError when lambda is in a hidden class

Changes: https://git.openjdk.org/jdk/pull/18810/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk=18810=00
  Issue: https://bugs.openjdk.org/browse/JDK-8330467
  Stats: 8 lines in 2 files changed: 2 ins; 0 del; 6 mod
  Patch: https://git.openjdk.org/jdk/pull/18810.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18810/head:pull/18810

PR: https://git.openjdk.org/jdk/pull/18810

Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v23]

[Severin Gehwolf]

On Tue, 16 Apr 2024 13:54:53 GMT, Alan Bateman  wrote:

> I think it continues to build time, it's just using some hidden jlink option. 
> So yes, it similar to a previous iteration except that it doesn't run as a 
> plugin the pipeline and the delta goes to the lib directory.

OK. If a hidden jlink option is good enough, then this works for me. I hope to 
have time to update the patch in the coming days.

-

PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2060691122

Re: RFR: 8329420: Java 22 (and 23) launcher calls default constructor although main() is static [v3]

[David Holmes]

On Wed, 17 Apr 2024 06:34:25 GMT, Jan Lahoda  wrote:

>> Consider code like:
>> 
>> public class MainClass {
>> public MainClass() {
>> System.out.println("Constructor called!");
>> }
>> public static void main() {
>> System.out.println("main called!");
>> }
>> }
>> 
>> and compile and run it, with preview enabled, like:
>> 
>> $ javac /tmp/MainClass.java 
>> $ java --enable-preview -classpath /tmp MainClass
>> Constructor called!
>> main called!
>> 
>> 
>> That is wrong, as the `main` method is static, and there is no need to 
>> create a new instance of the class.
>> 
>> The reason is that as launcher attempts to invoke the main method, it goes 
>> in the following order: 1) static-main-with-args; 2) 
>> instance-main-with-args; 3) static-main-without-args; 4) 
>> instance-main-without-args. But, for the instance variants, the code first 
>> creates a new instance of the given class, and only then attempts to lookup 
>> the `main` method, and will pass to the next option when the `main` method 
>> lookup fails. So, when invoking static-main-without-args, the current class 
>> instance may be created for instance-main-with-args, which will then fail 
>> due to the missing `main(String[])` method.
>> 
>> The proposed solution to this problem is to simply first do a lookup for the 
>> `main` method (skipping to the next variant when the given main method does 
>> not exist, without instantiating the current class).
>> 
>> There is also a relatively closely related problem: what happens when the 
>> constructor throws an exception?
>> 
>> public class MainClass {
>> public MainClass() {
>> if (true) throw new RuntimeException();
>> }
>> public void main() {
>> System.out.println("main called!");
>> }
>> }
>> 
>> 
>> when compiled an run, this produces no output whatsoever:
>> 
>> $ javac /tmp/MainClass.java 
>> $ java --enable-preview -classpath /tmp MainClass
>> $
>> 
>> 
>> This is because any exceptions thrown from the constructor are effectively 
>> ignored, and the launcher will continue with the next variant. This seems 
>> wrong - the exception should be printed for the user, like:
>> 
>> $ java --enable-preview -classpath /tmp/ MainClass 
>> Exception in thread "main" java.lang.RuntimeException
>> at MainClass.(MainClass.java:3)
>> 
>> 
>> This patch proposes to do that by not consuming the exceptions thrown from 
>> the constructor, and stop the propagation to the next variant.
>
> Jan Lahoda has updated the pull request incrementally with three additional 
> commits since the last revision:
> 
>  - Reflecting code formatting suggestion.
>  - First lookup the main method, and only then the constructor.
>  - Attempting to solve JDK-8329581 by only ignoring j.l.NoSuchMethodError

Okay on the update I suggested. Can't comment on the overall exception 
behaviour - not sure what is expected/specified.

-

PR Review: https://git.openjdk.org/jdk/pull/18753#pullrequestreview-2005279615

Re: RFR: 8329420: Java 22 (and 23) launcher calls default constructor although main() is static [v2]

[David Holmes]

On Tue, 16 Apr 2024 13:26:43 GMT, Jan Lahoda  wrote:

>> src/java.base/share/native/libjli/java.c line 419:
>> 
>>> 417: invokeInstanceMainWithArgs(JNIEnv *env, jclass mainClass, jobjectArray 
>>> mainArgs) {
>>> 418: jmethodID constructor = (*env)->GetMethodID(env, mainClass, 
>>> "", "()V");
>>> 419: CHECK_EXCEPTION_FAIL();
>> 
>> Shouldn't this also not be checked until you know you have an instance 
>> method?
>
> You mean to first check whether the method exists, and only then try to 
> lookup the constructor? Not sure if that has observable effects, but I can do 
> that.
> 
> I missed [JDK-8329581](https://bugs.openjdk.org/browse/JDK-8329581) before, 
> my plan is to adjust this "check for exception and clear it" check to only 
> work for `NoSuchMethodError`, and let the exception pass for any other 
> exception.

Yes that is what I meant. If the main method is static then the class need not 
have any such constructor (unless that is part of the spec for this 
functionality?).

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18753#discussion_r1568370439

Re: RFR: 8329420: Java 22 (and 23) launcher calls default constructor although main() is static [v3]

[Jan Lahoda]

On Wed, 17 Apr 2024 06:34:25 GMT, Jan Lahoda  wrote:

>> Consider code like:
>> 
>> public class MainClass {
>> public MainClass() {
>> System.out.println("Constructor called!");
>> }
>> public static void main() {
>> System.out.println("main called!");
>> }
>> }
>> 
>> and compile and run it, with preview enabled, like:
>> 
>> $ javac /tmp/MainClass.java 
>> $ java --enable-preview -classpath /tmp MainClass
>> Constructor called!
>> main called!
>> 
>> 
>> That is wrong, as the `main` method is static, and there is no need to 
>> create a new instance of the class.
>> 
>> The reason is that as launcher attempts to invoke the main method, it goes 
>> in the following order: 1) static-main-with-args; 2) 
>> instance-main-with-args; 3) static-main-without-args; 4) 
>> instance-main-without-args. But, for the instance variants, the code first 
>> creates a new instance of the given class, and only then attempts to lookup 
>> the `main` method, and will pass to the next option when the `main` method 
>> lookup fails. So, when invoking static-main-without-args, the current class 
>> instance may be created for instance-main-with-args, which will then fail 
>> due to the missing `main(String[])` method.
>> 
>> The proposed solution to this problem is to simply first do a lookup for the 
>> `main` method (skipping to the next variant when the given main method does 
>> not exist, without instantiating the current class).
>> 
>> There is also a relatively closely related problem: what happens when the 
>> constructor throws an exception?
>> 
>> public class MainClass {
>> public MainClass() {
>> if (true) throw new RuntimeException();
>> }
>> public void main() {
>> System.out.println("main called!");
>> }
>> }
>> 
>> 
>> when compiled an run, this produces no output whatsoever:
>> 
>> $ javac /tmp/MainClass.java 
>> $ java --enable-preview -classpath /tmp MainClass
>> $
>> 
>> 
>> This is because any exceptions thrown from the constructor are effectively 
>> ignored, and the launcher will continue with the next variant. This seems 
>> wrong - the exception should be printed for the user, like:
>> 
>> $ java --enable-preview -classpath /tmp/ MainClass 
>> Exception in thread "main" java.lang.RuntimeException
>> at MainClass.(MainClass.java:3)
>> 
>> 
>> This patch proposes to do that by not consuming the exceptions thrown from 
>> the constructor, and stop the propagation to the next variant.
>
> Jan Lahoda has updated the pull request incrementally with three additional 
> commits since the last revision:
> 
>  - Reflecting code formatting suggestion.
>  - First lookup the main method, and only then the constructor.
>  - Attempting to solve JDK-8329581 by only ignoring j.l.NoSuchMethodError

So, I've pushed an update that:
- attemps to solve [JDK-8329581](https://bugs.openjdk.org/browse/JDK-8329581) 
by only ignoring `NoSuchMethodError` when looking up methods. Hence any other 
problem will lead to an error to be printed out, as usually.
https://github.com/openjdk/jdk/pull/18753/commits/dd447e898785a3d7e2def47dd1efcca3db41dd46
- changed the code to first lookup the `main` method, and only then 
constructors, which is what I think @dholmes-ora was suggesting.
https://github.com/openjdk/jdk/pull/18753/commits/83fabec6e5e8bf6deb9efce22ef6ace408cf3d26
- fixed the comment formatting, as suggested by @AlanBateman
https://github.com/openjdk/jdk/pull/18753/commits/c007f61ebc0660cc6f93e29e71dc635aa50ba538

Please let me know what you think. Thanks!

-

PR Comment: https://git.openjdk.org/jdk/pull/18753#issuecomment-2060481245

Re: RFR: 8329420: Java 22 (and 23) launcher calls default constructor although main() is static [v3]

[Jan Lahoda]

> Consider code like:
> 
> public class MainClass {
> public MainClass() {
> System.out.println("Constructor called!");
> }
> public static void main() {
> System.out.println("main called!");
> }
> }
> 
> and compile and run it, with preview enabled, like:
> 
> $ javac /tmp/MainClass.java 
> $ java --enable-preview -classpath /tmp MainClass
> Constructor called!
> main called!
> 
> 
> That is wrong, as the `main` method is static, and there is no need to create 
> a new instance of the class.
> 
> The reason is that as launcher attempts to invoke the main method, it goes in 
> the following order: 1) static-main-with-args; 2) instance-main-with-args; 3) 
> static-main-without-args; 4) instance-main-without-args. But, for the 
> instance variants, the code first creates a new instance of the given class, 
> and only then attempts to lookup the `main` method, and will pass to the next 
> option when the `main` method lookup fails. So, when invoking 
> static-main-without-args, the current class instance may be created for 
> instance-main-with-args, which will then fail due to the missing 
> `main(String[])` method.
> 
> The proposed solution to this problem is to simply first do a lookup for the 
> `main` method (skipping to the next variant when the given main method does 
> not exist, without instantiating the current class).
> 
> There is also a relatively closely related problem: what happens when the 
> constructor throws an exception?
> 
> public class MainClass {
> public MainClass() {
> if (true) throw new RuntimeException();
> }
> public void main() {
> System.out.println("main called!");
> }
> }
> 
> 
> when compiled an run, this produces no output whatsoever:
> 
> $ javac /tmp/MainClass.java 
> $ java --enable-preview -classpath /tmp MainClass
> $
> 
> 
> This is because any exceptions thrown from the constructor are effectively 
> ignored, and the launcher will continue with the next variant. This seems 
> wrong - the exception should be printed for the user, like:
> 
> $ java --enable-preview -classpath /tmp/ MainClass 
> Exception in thread "main" java.lang.RuntimeException
> at MainClass.(MainClass.java:3)
> 
> 
> This patch proposes to do that by not consuming the exceptions thrown from 
> the constructor, and stop the propagation to the next variant.

Jan Lahoda has updated the pull request incrementally with three additional 
commits since the last revision:

 - Reflecting code formatting suggestion.
 - First lookup the main method, and only then the constructor.
 - Attempting to solve JDK-8329581 by only ignoring j.l.NoSuchMethodError

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18753/files
  - new: https://git.openjdk.org/jdk/pull/18753/files/2022aa5a..c007f61e

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18753=02
 - incr: https://webrevs.openjdk.org/?repo=jdk=18753=01-02

  Stats: 117 lines in 2 files changed: 73 ins; 5 del; 39 mod
  Patch: https://git.openjdk.org/jdk/pull/18753.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18753/head:pull/18753

PR: https://git.openjdk.org/jdk/pull/18753