RFR: 8152404: Stabilize PackageEntry::package_exports_do

[Rachel Protacio]

Thanks for the review, David. I've fixed that comment discrepancy.

Rachel


On 6/3/2016 12:40 AM, David Holmes wrote:
> Hi Rachel,
>
> On 3/06/2016 1:36 AM, Rachel Protacio wrote:
>> Hi, David,
>>
>> New webrev: http://cr.openjdk.java.net/~rprotacio/8152404.01/
>>
>> Replies inline.
>>
>> On 5/25/2016 5:10 PM, David Holmes wrote:
>>> Hi Rachel,
>>>
>>> On 26/05/2016 1:10 AM, Rachel Protacio wrote:
>>>> Hello,
>>>>
>>>> Please review this bug fix for some module export checking code. The
>>>> PackageEntry::package_exports_do() function had assumed calling
>>>> is_qual_exported() would verify that _qualified_exports was not NULL,
>>>> but as that was not the case, I've re-defined the PackageEntry member
>>>> variables and export checking functions.
>>>
>>> In src/share/vm/classfile/packageEntry.hpp the documentation no longer
>>> makes it clear how the three variables are defined for the four
>>> different situation ie for an unnamed export what are the values of
>>> _qualified_exports and _is_exported_unqualified?
>> I've updated the comment at the top of the file (thanks, Lois!). Does
>> this make it more clear?
>
> Yes - thanks.
>
> This comment is no longer accurate though:
>
>  37 //   - a flag indicating if package is exported, either 
> qualifiedly or
>   38 //     unqualifiedly.
>
> as the flag is now only for the unqualified case.
>
>>>
>>> Overall (given I'm not familiar with the different export qualities) I
>>> found it a little unclear how the different states were defined and
>>> what the legal transitions are between them. I would have expected to
>>> see more assertions regarding those three state variables, and checks
>>> for when they get set or cleared. In one place you actually removed
>>> such an assertion.
>> Good point. I had removed it because I removed the _is_exported
>> variable, but I've now put assertions into the .hpp state functions.
>
> Looks good.
>
> No further comments from me.
>
> Thanks,
> David
> -----
>
>> Thanks,
>> Rachel
>>>
>>> Thanks,
>>> David
>>>
>>>> Passes JPRT, jck (vm, lang, and api/java_lang), and RBT hotspot_all 
>>>> and
>>>> non-colo tests.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8152404
>>>> Open webrev: http://cr.openjdk.java.net/~rprotacio/8152404.00/
>>>>
>>>> Thanks,
>>>> Rachel
>>