RFR: 8152404: Stabilize PackageEntry::package_exports_do

[David Holmes]

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
>