RFR 8204620: ModuleEntry::is_non_jdk_module() determination for what is a jdk module is incorrect

[Lois Foltan]

Looks good!
Lois

On 6/13/2018 3:25 PM, Harold David Seigel wrote:

> Hi Lois, Mandy,
>
> Thanks for the reviews.
>
> Please review this updated webrev:
>
>     http://cr.openjdk.java.net/~hseigel/bug_8204620.2/webrev/index.html
>
> It has the changes you suggested plus a few other minor ones.
>
> Thanks! Harold
>
>
> On 6/13/2018 11:09 AM, Lois Foltan wrote:
>> On 6/13/2018 8:44 AM, Harold David Seigel wrote:
>>
>>> Hi,
>>>
>>> Please review this fix for how the JVM determines when to print a 
>>> module's version.  The fix implements the refinement described in 
>>> the bug's comment on 12-Jun-2018.
>>>
>>> This method is currently only called from ClassCastExceptions but 
>>> will be used for other messages as part of JDK-8166633 
>>> <https://bugs.openjdk.java.net/browse/JDK-8166633>.
>>>
>>> Open Webrev: 
>>> http://javaweb.us.oracle.com/~hseigel/webrev/bug_8204620/webrev/index.html
>>>
>>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8204620
>>>
>>> This fix was tested with Mach5 tiers 1 and 2 tests and builds on 
>>> Linux-X64, Windows, Solaris Sparc, and Mac OS X, with tiers 3-5 
>>> tests on Linux-x64, and with JCK-11 Lang and VM tests.
>>>
>>> Thanks, Harold
>>>
>> Looks good, a couple of comments:
>>
>> - module.[c/h]pp - maybe consider the method name "show_version" 
>> instead of "show_module_version".  Chances are this will be invoked 
>> by a ModuleEntry* variable whose name most likely will already have 
>> the word "module" in it.
>> - module.cpp - line #81, since the version field within a ModuleEntry 
>> is a Symbol*, can you use fast_compare instead of strcmp?
>> - test - needs copyright updated.
>>
>> Thanks for making this change, it will help with the upcoming error 
>> message work!
>> Lois
>