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

[Karen Kinnear]

Harold,

This looks much cleaner!

thanks,
Karen

> On Jan 12, 2017, at 2:18 PM, harold seigel <harold.seigel at oracle.com> wrote:
> 
> 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
>>> 
>> 
>