RFR(M): 8165550: Add class loader names to ClassCastException message

Rachel Protacio rachel.protacio at oracle.com
Mon Nov 14 21:00:07 UTC 2016


New webrev: http://cr.openjdk.java.net/~rprotacio/8165550.01/

Corrected according to David's review. Comments below.


On 11/10/2016 6:24 PM, David Holmes wrote:
> Okay on with the review:
>
> src/share/vm/classfile/moduleEntry.cpp
>
> +     const char* loc = "";
> +     loc = location()->as_C_string();
>
> can just be:
>
> +     const char* loc = location()->as_C_string();
>
Fixed.
> ---
>
> src/share/vm/runtime/sharedRuntime.cpp
>
> 1941 const char* class_loader_and_module_name(Klass* klass, Klass* 
> base_klass) {
>
> I'm unclear in what way "base_klass" is a "base" class ?? And why, if 
> it is not an instance class must be it in java.base ??
I really meant "bottom" class, following the logic used to parse it out. 
I've moved that internal to the function now. So if the klass is an 
instance klass or instance klass array, we look for further information. 
Otherwise (if it's a primitive array), it's from java.base.
>
> Nit: I don't think these need to be #defines nor have such verbose names:
>
> 1936 #define CLASS_LOADER_NAME_DELIMITER "/"
> 1937 #define CLASS_LOADER_NAME_DELIMITER_LENGTH 
> strlen(CLASS_LOADER_NAME_DELIMITER)
>
> why not just
>
> const char* delim = "\";
> int delim_len = strlen(delim);
>
> Plus you are inconsistent with the handling of the "/" delimiter 
> versus the "@" delimiter.
Fixed.
>
> ---
>
>  test/runtime/modules/CCE_module_msg.java
>
> Nit: in the catch block from line #95 can you store getMessage() into 
> a local so it isn't called 5 times. Thanks. :)
Fixed too. Thanks!
Rachel
>
> Thanks,
> David
>
>> Rachel
>>
>> On 11/10/2016 5:02 PM, David Holmes wrote:
>>> Hi Rachel,
>>>
>>> Just wondering, in the most complex case, exactly what will this new
>>> message look like? We seem to be including module names as well as
>>> classloader names, and of course class names.
>>>
>>> Arguably we should only add the additional info when they differ
>>> between the types ie if both in same classloader and same module then
>>> only report class name; if same loader but different module report
>>> module:class; if different loaders then report everything
>>> loader:module:class. Also not sure module version is needed at all in
>>> this message.
>>>
>>> Thanks,
>>> David
>>>
>>> On 11/11/2016 7:38 AM, Rachel Protacio wrote:
>>>> Hi,
>>>>
>>>> Please review this Jigsaw enhancement which adds the new format of 
>>>> class
>>>> loader names to VM ClassCastException messages. Includes an
>>>> updated/expanded jtreg test.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8165550
>>>> Open webrev: http://cr.openjdk.java.net/~rprotacio/8165550.00/
>>>>
>>>> Passes JPRT and jck vm and lang tests.
>>>>
>>>> Thanks!
>>>> Rachel
>>



More information about the hotspot-runtime-dev mailing list