Re: RFR: 8307818: Convert Indify tool to Classfile API [v4]

2024-05-20 Thread Chen Liang
On Mon, 20 May 2024 15:34:28 GMT, Oussama Louati  wrote:

>> An indify tool in j.l.i tests (also in vmTestBase) convert some source-code 
>> private static methods with MT_ MH_, and INDY_ prefixes into MethodHandle, 
>> MethodType, and CallSite constants.
>> It currently uses ad-hoc code to process class files and intends to migrate 
>> to ASM; but since we have the Classfile API, we can migrate to Classfile API 
>> instead.
>
> Oussama Louati has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update test/jdk/java/lang/invoke/indify/Indify.java
>   
>   Co-authored-by: Glavo 

test/jdk/java/lang/invoke/indify/Indify.java line 1362:

> 1360: List bsmArgs = new ArrayList<>();
> 1361: for (LoadableConstantEntry lce : 
> classModel.constantPool().bootstrapMethodEntry(i).arguments()){
> 1362: bsmArgs.add(lce);

Suggestion:

List bsmArgs = new 
ArrayList<>(classModel.constantPool().bootstrapMethodEntry(i).arguments());

Also remove the closing brace in next line.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1606944693


Re: RFR: 8307818: Convert Indify tool to Classfile API [v4]

2024-05-20 Thread Chen Liang
On Mon, 20 May 2024 15:34:28 GMT, Oussama Louati  wrote:

>> An indify tool in j.l.i tests (also in vmTestBase) convert some source-code 
>> private static methods with MT_ MH_, and INDY_ prefixes into MethodHandle, 
>> MethodType, and CallSite constants.
>> It currently uses ad-hoc code to process class files and intends to migrate 
>> to ASM; but since we have the Classfile API, we can migrate to Classfile API 
>> instead.
>
> Oussama Louati has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update test/jdk/java/lang/invoke/indify/Indify.java
>   
>   Co-authored-by: Glavo 

test/jdk/java/lang/invoke/indify/Indify.java line 201:

> 199: } else if (ex != err) {
> 200: err.addSuppressed(ex);
> 201: }

Suggestion:

err.addSuppressed(ex);
}

test/jdk/java/lang/invoke/indify/Indify.java line 445:

> 443: final char[] poolMarks;
> 444: final Map Constants = new HashMap<>();
> 445: final Map IndySignatures = new HashMap<>();

Please use lowerCamelCase field names.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1606938989
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1606941104


Re: RFR: 8307818: Convert Indify tool to Classfile API [v4]

2024-05-20 Thread Oussama Louati
> An indify tool in j.l.i tests (also in vmTestBase) convert some source-code 
> private static methods with MT_ MH_, and INDY_ prefixes into MethodHandle, 
> MethodType, and CallSite constants.
> It currently uses ad-hoc code to process class files and intends to migrate 
> to ASM; but since we have the Classfile API, we can migrate to Classfile API 
> instead.

Oussama Louati has updated the pull request incrementally with one additional 
commit since the last revision:

  Update test/jdk/java/lang/invoke/indify/Indify.java
  
  Co-authored-by: Glavo 

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18841/files
  - new: https://git.openjdk.org/jdk/pull/18841/files/040e925b..992c9773

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

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

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