Re: RFR: 8266851: Implement JEP 403: Strongly Encapsulate JDK Internals [v3]

2021-05-18 Thread Mark Reinhold 
On Fri, 14 May 2021 08:54:23 GMT, Alan Bateman  wrote:

>> Mark Reinhold has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Remove now-unnecessary uses and mentions of the --illegal-access option
>
> test/jdk/tools/launcher/modules/illegalaccess/IllegalAccessTest.java line 26:
> 
>> 24: /**
>> 25:  * @test
>> 26:  * @requires vm.compMode != "Xcomp"
> 
> I think `@requires vm.compMode != "Xcomp" was added because the test took too 
> long with -Xcomp. I think it can be dropped now.

Dropped.

> test/jdk/tools/launcher/modules/illegalaccess/IllegalAccessTest.java line 37:
> 
>> 35: 
>> 36: /**
>> 37:  * Basic test of --illegal-access=value to deny or permit access to JDK 
>> internals.
> 
> The comment should probably be updated as the test no longer tests denying or 
> permitting access.

Fixed.

-

PR: https://git.openjdk.java.net/jdk/pull/4015

Re: RFR: 8266851: Implement JEP 403: Strongly Encapsulate JDK Internals [v3]

2021-05-14 Thread Alan Bateman 
On Thu, 13 May 2021 23:07:07 GMT, Mark Reinhold  wrote:

>> Please review this implementation of JEP 403 
>> (https://openjdk.java.net/jeps/403).
>> Alan Bateman is the original author of almost all of it.  Passes tiers 1-3 
>> on 
>> {linux,macos,windows}-x64 and {linux,macos}-aarch64.
>
> Mark Reinhold has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove now-unnecessary uses and mentions of the --illegal-access option

The changes look good.

I searched the test and src trees for other remaining usages "--illegal-access" 
and didn't find any more (except for localized resources).

It's likely that some of the updated tests no longer need "/othervm". It would 
need a close inspection of each test to see if it still needed.

test/jdk/tools/launcher/modules/illegalaccess/IllegalAccessTest.java line 26:

> 24: /**
> 25:  * @test
> 26:  * @requires vm.compMode != "Xcomp"

I think `@requires vm.compMode != "Xcomp" was added because the test took too 
long with -Xcomp. I think it can be dropped now.

test/jdk/tools/launcher/modules/illegalaccess/IllegalAccessTest.java line 37:

> 35: 
> 36: /**
> 37:  * Basic test of --illegal-access=value to deny or permit access to JDK 
> internals.

The comment should probably be updated as the test no longer tests denying or 
permitting access.

-

Marked as reviewed by alanb (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/4015

Re: RFR: 8266851: Implement JEP 403: Strongly Encapsulate JDK Internals [v3]

2021-05-13 Thread Mandy Chung 
On Thu, 13 May 2021 23:07:07 GMT, Mark Reinhold  wrote:

>> Please review this implementation of JEP 403 
>> (https://openjdk.java.net/jeps/403).
>> Alan Bateman is the original author of almost all of it.  Passes tiers 1-3 
>> on 
>> {linux,macos,windows}-x64 and {linux,macos}-aarch64.
>
> Mark Reinhold has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove now-unnecessary uses and mentions of the --illegal-access option

Looks good.

-

Marked as reviewed by mchung (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/4015

Re: RFR: 8266851: Implement JEP 403: Strongly Encapsulate JDK Internals [v3]

2021-05-13 Thread Mark Reinhold 
> Please review this implementation of JEP 403 
> (https://openjdk.java.net/jeps/403).
> Alan Bateman is the original author of almost all of it.  Passes tiers 1-3 on 
> {linux,macos,windows}-x64 and {linux,macos}-aarch64.

Mark Reinhold has updated the pull request incrementally with one additional 
commit since the last revision:

  Remove now-unnecessary uses and mentions of the --illegal-access option

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4015/files
  - new: https://git.openjdk.java.net/jdk/pull/4015/files/719ff4ee..1c14998e

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4015&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4015&range=01-02

  Stats: 26 lines in 10 files changed: 0 ins; 8 del; 18 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4015.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4015/head:pull/4015

PR: https://git.openjdk.java.net/jdk/pull/4015