RFR(S) 8172288: Fix error message when trying to define an existing package to a module

[harold seigel]

Hi,

Please review this updated webrev: 
http://cr.openjdk.java.net/~hseigel/bug_8172288.hs.3/webrev/index.html

It contains the three changes listed below.  Only file modules.cpp has 
changed since the last webrev.

This fix was tested with the JTreg hotspot, java/io, java/lang, 
java/util and other JTReg tests, with the JCK lang and VM tests, and 
with RBT tiers2 - tier5 tests on Linux X64.

Thanks, Harold


On 1/12/2017 10:16 AM, harold seigel wrote:
> Thanks everyone for all the reviews.
>
> I'll post a new webrev with the following changes, and testing 
> details, once the re-testing completes.
>
> 1. Rename throw_dup_pkg_IAE() to throw_dup_pkg_exception()
>
> 2. Remove the package_name argument from throw_dup_pkg_exception() 
> because the package name can be extracted from the existing_pkg argument.
>
> 3. Remove dupl_pk_index and use existing_pkg instead.
>
> Harold
>
>
> On 1/12/2017 9:30 AM, George Triantafillou wrote:
>> Hi Karen,
>>
>> On 1/12/2017 9:26 AM, Karen Kinnear wrote:
>>> Harold,
>>>
>>> Looks good - with the same minor comment George caught, perhaps 
>>> throw_dup_pkg_exception?
>>>
>>> George - there are two tests attached.
>> Thanks, I saw the additions to 
>> test/runtime/modules/JVMDefineModule.java, but I wanted to know which 
>> hotspot tests were run with Harold's change.
>>
>> -George
>>>
>>> thanks,
>>> Karen
>>>
>>>> On Jan 12, 2017, at 9:18 AM, George Triantafillou 
>>>> <george.triantafillou at oracle.com> wrote:
>>>>
>>>> Hi Harold,
>>>>
>>>> src/share/vm/classfile/modules.cpp - lines 266, 449, and 806:
>>>>
>>>> Change "throw_dup_pkg_IAE" to "throw_dup_pkg_ISE".
>>>>
>>>> How was the change tested?  Thanks.
>>>>
>>>> -George
>>>>
>>>> On 1/12/2017 8:50 AM, harold seigel wrote:
>>>>> Hi,
>>>>>
>>>>> Please review this updated webrev for this bug.  The updated 
>>>>> webrev includes a small JDK change and hotspot changes to throw 
>>>>> the correct exceptions.
>>>>>
>>>>> New JDK webrev: 
>>>>> http://cr.openjdk.java.net/~hseigel/bug_8172288.jdk.2/webrev/index.html 
>>>>>
>>>>>
>>>>> New Hotspot webrev: 
>>>>> http://cr.openjdk.java.net/~hseigel/bug_8172288.hs.2/
>>>>>
>>>>> Note also that the bug has been re-titled to: Fix Jigsaw related 
>>>>> module/package error messages and throw correct exceptions
>>>>>
>>>>> Thanks, Harold
>>>>>
>>>>> On 1/10/2017 10:18 AM, harold seigel wrote:
>>>>>> Yes.  I'll modify the bug to include that.
>>>>>>
>>>>>> What exception should the JVM throw for this?
>>>>>>
>>>>>> Thanks, Harold
>>>>>>
>>>>>>
>>>>>> On 1/10/2017 10:12 AM, Alan Bateman wrote:
>>>>>>> On 10/01/2017 15:02, harold seigel wrote:
>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Please review this small fix for JDK-8172288. Here's a sample 
>>>>>>>> of the new message:
>>>>>>>>
>>>>>>>> Package mypackage6 for module dupl.pkg.module is already in 
>>>>>>>> another module, Module_A, defined to the class loader
>>>>>>>>
>>>>>>>> Adding the class loader name to the message will be done as 
>>>>>>>> part of JDK-8169559 
>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8169559>.
>>>>>>>>
>>>>>>>> Open Webrev: 
>>>>>>>> http://cr.openjdk.java.net/~hseigel/bug_8172288/webrev/index.html
>>>>>>> IllegalArgumentException isn't a good exception for these cases, 
>>>>>>> is now the time to proposing this too?
>>>>>>>
>>>>>>> -Alan
>>
>