[ping] RE: RFR(M): 8199852: Print more information about class loaders in LinkageErrors.

[Lindenmaier, Goetz]

Hi Lois, David, George or others,

I would like to finish this one up, could I please get some
(hopefully final) reviews?

Thanks,
  Goetz.

> -----Original Message-----
> From: Lindenmaier, Goetz
> Sent: Mittwoch, 18. April 2018 09:55
> To: 'David Holmes' <david.holmes at oracle.com>; 'hotspot-runtime-
> dev at openjdk.java.net' <hotspot-runtime-dev at openjdk.java.net>
> Subject: RE: RFR(M): 8199852: Print more information about class loaders in
> LinkageErrors.
> 
> Hi,
> 
> I rebased the webrev to the jdk repo:
> http://cr.openjdk.java.net/~goetz/wr18/8199852-exMsg_Linkage/03-jdk/
> Could I please get the final go?
> 
> Thanks and best regards,
>   Goetz.
> 
> 
> > -----Original Message-----
> > From: Lindenmaier, Goetz
> > Sent: Freitag, 13. April 2018 10:10
> > To: 'David Holmes' <david.holmes at oracle.com>; hotspot-runtime-
> > dev at openjdk.java.net
> > Subject: RE: RFR(M): 8199852: Print more information about class loaders in
> > LinkageErrors.
> >
> > Hi David,
> >
> > I fixed what you mentioned here. I also found an older mail
> > of yours with some comments I implemented now. I copied
> > that mail's content here to have only one mail ... find references
> > to some incremental webrevs in my replies below.
> >
> > Further I moved the test classes of test differentLE/ into a package.
> >
> > Comprehensive new webrev:
> > http://cr.openjdk.java.net/~goetz/wr18/8199852-exMsg_Linkage/03
> >
> >
> > > -----Original Message-----
> > > From: David Holmes <david.holmes at oracle.com>
> > > Sent: Thursday, April 12, 2018 8:50 AM
> > > To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-
> runtime-
> > > dev at openjdk.java.net
> > ...
> > > This looks reasonable to me. Two minor comments:
> > > - updated tests need updated copyright notice
> > > - I think you can avoid duplicating test1() to test2() by passing in the
> > >    loader name and the expected message as parameters
> > Fixed both. Incremental webrev:
> > http://cr.openjdk.java.net/~goetz/wr18/8199852-exMsg_Linkage/03-
> > incremental2/
> >
> >
> > > -----Original Message-----
> > > From: David Holmes <david.holmes at oracle.com>
> > > Sent: Sunday, March 25, 2018 11:49 PM
> > > To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-
> runtime-
> > > dev at openjdk.java.net
> > ...
> > > > Maybe I should suppress printing verbose information about these
> > > > well known loaders.  (There is is_builtin_class_loader_data() to
> > > > identify such.)
> > >
> > > Yes it might be good to simplify things for the predefined loaders.
> > > Though I'd use:
> > >    SystemDictionary::is_platform_class_loader
> > >    SystemDictionary::is_system_class_loader
> > > for that check.
> > I implemented omitting the parent for these loaders.
> >
> > > Maybe it makes sense to have a set pattern for this descriptive text,
> > > and use "unnamed" if the loader has no name:
> > >
> > > loader <name>, instanceof <class>, child of loader <name> ...
> > I implemented printing "<unnamed>" for empty names.  I would like
> > To keep the format with brackets, because the enclosing text might use
> > commas, too, and then the sentence structure might get confusing.
> >
> > See this incremental webrev for these two changes:
> > http://cr.openjdk.java.net/~goetz/wr18/8199852-exMsg_Linkage/03-
> > incremental1a/
> >
> > > Add this bug to the @bug in the test(s).
> > Done.
> >
> > > > I'm not clear where you distinguish type and class.
> > > Using "class" is inaccurate as it can be a class, interface or array type.
> > >
> > > In the above the entity is a "type" as we are referring to a <type-name,
> > > classloader> pair. But you can also just read it as "class or interface
> > > or array type"
> > So why not print what it really is? I had added external_kind() for this in
> > My previous exception message change.
> > Unfortunately only in SystemDictionary::check_constraints() this is easily
> > possible, and it improves the message I think.
> >
> > Incremental webrev for these two changes:
> > http://cr.openjdk.java.net/~goetz/wr18/8199852-exMsg_Linkage/03-
> > incremental1b/
> >
> > I also implemented a test for the call to external_kind(), but I missed to put
> > that into
> > the incremental webrev.
> >
> > Best regards,
> >   Goetz.