[core-libs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control

[David Holmes]

Clarification ...

On 17/05/2018 6:37 PM, David Holmes wrote:
> Hi Remi,
> 
> On 17/05/2018 6:16 PM, Remi Forax wrote:
>> Hi all,
>>
>> ----- Mail original -----
>>> De: "Alan Bateman" <Alan.Bateman at oracle.com>
>>> À: "David Holmes" <david.holmes at oracle.com>, "core-libs-dev" 
>>> <core-libs-dev at openjdk.java.net>
>>> Envoyé: Mardi 15 Mai 2018 15:53:44
>>> Objet: Re: [core-libs] RFR (L): 8010319: Implementation of JEP 181: 
>>> Nest-Based Access Control
>>
>>> On 15/05/2018 01:52, David Holmes wrote:
>>>> This review is being spread across four groups: langtools, core-libs,
>>>> hotspot and serviceability. This is the specific review thread for
>>>> core-libs - webrev:
>>>>
>>>> http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v1/
>>
>> [...]
>>
>>> Maybe a question for Kumar but are we planning to pull in any ASM
>>> updates for JDK 11? NestMembers extends Attribute looks okay, I'm less
>>> sure about the change to ClassReader as I don't know if there is
>>> somewhere else in ASM that has the list of attributes to always parse.
>>
>> With my ASM hat,
>> the current master of ASM (the release of ASM 6.2 is scheduled for the 
>> next week-end) already supports nestmates (and constant dynamic and 
>> preview feature) so i suppose that at some point in the future Kumar 
>> will merge it to the JDK.
> 
> Unfortunately Kumar is no longer with us.

By which I simply mean he is no longer at Oracle.

David
-----

>> We have recently changed the way we implement features in ASM, instead 
>> of having features lingering in different branches, we now integrate 
>> them directly in the master under an experimental flag 
>> (ASM7_EXPERIMENTAL), which means for the JDK that it is no longer 
>> necessary to wait until the release of ASM 7 because it can use the 
>> experimental support of ASM 6.2.
>> (note that experimental doesn't mean full of bugs, or half baked or 
>> anything like this, it means that the feature is not yet integrated in 
>> a released JDK).
>>
>> I've taking a look to the code in this patch, i've two comments,
>> - in Attributes, it seems that the code store the bytecode slice 
>> corresponding to the attribute only to use its length as argument of 
>> the ByteVector which is like an ArrayList of byte, it grows 
>> automatically so the initial capacity is a perf optimization. Perhaps 
>> the byte array is used somewhere else ?
>> - patching the ClassReader.accept is really a quick hack because the 
>> method accept with 3 arguments is not patched so if this method is 
>> called somewhere in the JDK it will behave as it should.
> 
> I'll take a look at this. To be honest I don't even remember who 
> provided those changes ... I thought you had provided feedback at some 
> point in the past :) There's a valhalla-dev email with a link that's no 
> longer valid:
> 
> https://gitlab.ow2.org/asm/asm/tree/NEST_MATES
> 
> Thanks,
> David
> 
>> [...]
>>
>>>
>>> -Alan.
>>
>> Rémi
>>