RFR(L): 8197405: Improve messages of AbstractMethodErrors and IncompatibleClassChangeErrors.

[David Holmes]

Hi Goetz,

I couldn't work through all of this but it touches on some areas I'm 
working on with JEP 181 (Nestmates).

I think this is well intentioned but I think there are far more complex 
possibilities that make your improved messages inaccurate by trying to 
be too specific.

Also at least for ICCE there may be existing tests that check the ICCE 
error message and which will now need updating.

More below ...

On 10/02/2018 12:01 AM, Lindenmaier, Goetz wrote:
> Hi,
> 
> I changed the test to use jasm.
> http://cr.openjdk.java.net/~goetz/wr18/8197405-ameExMsg/webrev.02/

src/hotspot/share/interpreter/linkResolver.cpp

1774 void LinkResolver::throw_abstract_method_error(methodHandle& 
method, Klass *abstract_klass,

The Klass* pointer is the resolved method class - yes? The name 
"abstract_klass" is misleading as it need not be abstract. Relatedly 
your error messages may not be accurate. Suppose you have:

class A {
   public void m() {}

   void test(A a) {
     a.m();
   }
}

abstract class B extends A {
   public abstract void m();
}


If you pass test() a receiver that is a subclass of B and which doesn't 
define an implementation of m, then IIUC the resolved method class is A, 
which is not abstract - but your error message would say "abstract class A".

I think if you want to report whether a class is abstract then you need 
to actually query is_abstract(). Or simply report things more directly:

"Receiver class R does not define or inherit an implementation of the 
resolved method X.y"

It may also be useful to report the access modifier of the resolved 
method (and the selected method that was found to be abstract) as that 
can also affect selection.

1780   // If method is null, use void signature.

How can method be null? Surely it is either the selected method (which 
is abstract) or it's the resolved method (for which no selected method 
could be found).

1785   if(recv_klass) {

Style: if (recv_klass != NULL) {

But how can the receiver class be null here ?


Cheers,
David
-----

> Also, I fixed a bug and disabled the test on aarch/arm.
> 
> I started to port the simple stuff to aarch/arm.
> I'm happy to include a further patch for these, but if I
> have time I will also try to implement the rest, can't test
> it though.
> 
> Best regards,
>    Goetz.
> 
>> -----Original Message-----
>> From: David Holmes [mailto:david.holmes at oracle.com]
>> Sent: Donnerstag, 8. Februar 2018 23:54
>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-runtime-
>> dev at openjdk.java.net
>> Subject: Re: RFR(L): 8197405: Improve messages of AbstractMethodErrors
>> and IncompatibleClassChangeErrors.
>>
>> Hi Goetz,
>>
>> Binary class files can not be checked-in to the repo. You'll need to use
>> jasm or jcod files that then get compiled as part of the test.
>>
>> Thanks,
>> David
>>
>> On 9/02/2018 12:41 AM, Lindenmaier, Goetz wrote:
>>> Hi,
>>>
>>> This change improves the messages of AbstractMethodErrors and
>> IncompatibleClassChangeErrors.
>>> Please review.
>>> http://cr.openjdk.java.net/~goetz/wr18/8197405-ameExMsg/webrev.01/
>>>
>>> It adds tests that check some of the improved messages.
>>> The tests check the following improvements:
>>>
>>> AbstractMethodErrors:
>>>
>>> test_ameInt, test case 1:
>>> before: no message / null
>>> after:  Missing implementation of interface method
>> MyAbstractInt.anAbstractMethod()Ljava/lang/String;
>>>
>>> test_ameInt, test case 2:
>>> before: no message / null
>>> after:  Class ImplementsSomeFunctionsInt does not implement the
>> requested method
>>>           aFunctionOfMyInterface()Ljava/lang/String; inherited from interface
>> MyInterfaceInt1
>>>
>>> test_ame, interpreted:
>>> before: no message / null
>>> after:  Missing implementation of interface method
>> MyInterface.aFunctionOfMyInterface()V
>>>
>>> test_ame, compiled:
>>> before: MyAbstract.aFunctionOfMyInterface()V
>>> after:  Class ImplementsSomeFunctions does not implement the requested
>> method
>>>           aFunctionOfMyInterface()V inherited from abstract class MyAbstract
>>>
>>> IncompatibleClassChangeErrors:
>>>
>>> test_icc_compiled_itable_stub
>>> before: vtable stub
>>> after:  Class ICC_B does not implement the requested interface ICC_iB
>>>
>>> Best regards,
>>>     Goetz.
>>>