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

[harold seigel]

Thanks Lois!

Harold


On 1/12/2017 2:21 PM, Lois Foltan wrote:
> Looks good, thank you for making those changes.
> Lois
>
> On 1/12/2017 2:18 PM, harold seigel 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
>>>>
>>>
>>
>