RFR(M) 8208999 Some use of Klass* should be replaced by InstanceKlass*

[David Holmes]

Hi Ioi,

I took a quick look at this but it is not a review. This seems to 
highlight further problems in the arrangement of types and methods to 
me. For example in klass.cpp we have:

   bool Klass::verify_itable_index(int i) {
     assert(is_instance_klass(), "");
!   int method_count = 
klassItable::method_count_for_interface(InstanceKlass::cast(this));
     assert(i >= 0 && i < method_count, "index out of bounds");
     return true;
   }

but if "this" must be an instanceKlass then verify_itable_index should 
not be a method in klass but a method in instanceKlass.

Cheers,
David

On 7/08/2018 7:26 AM, Ioi Lam wrote:
> Hi, please review this clean up
> 
> https://bugs.openjdk.java.net/browse/JDK-8208999
> http://cr.openjdk.java.net/~iklam/jdk12/8208999_klass_should_be_instanceklass.v01/ 
> 
> 
> I changed the following APIs so that we can avoid many unnecessary type 
> casts
> from Klass* -> InstanceKlass*.
> 
>      Klass::java_super()                    -> now returns InstanceKlass*
>      InstanceKlass::local_interfaces()      -> Array<InstanceKlass*>*
>      InstanceKlass::transitive_interfaces() -> Array<InstanceKlass*>*
> 
> I also modified APIs in SystemSictionary and SystemSictionary to return 
> InstanceKlass*
> where possible.
> 
> Most changes are mundane, but here are some notable changes:
> 
> 1. To save space, Klass::_secondary_supers (an Array<Klass*>*) may be 
> pointing
>     to the same array as InstanceKlass::_transitive_interfaces (an
>     Array<InstanceKlass*>*).
> 
>     To make this happen, I added Array::as_const_array_of().
>     See InstanceKlass::compute_secondary_supers() and array.h.
> 
>     I've added enough 'const' and asserts to catch mis-uses of this type
>     of casting. If someone has a better suggestion, please let me know.
> 
> 2. I changed Klass::_secondary_supers to 'const Array<Klass*>*' because
>     of #1.
> 
>     There are many pointers returned by Klass/InstanceKlass that should be
>     'const', as most callers should read them only without modifying, 
> but that's
>     outside the scope of this RFE.
> 
> 3. SystemDictionary::resolve_super_or_fail() currently accepts the format
>     of "Ljava/lang/Object;" for the "super_name" parameter. I am keeping 
> the
>     same behavior in my patch. However, I wonder if this actually correct
>     per JVMLS. I filed JDK-8209030.
> 
>     See the call to resolve_instance_class_or_null_helper from 
> resolve_super_or_fail.
> 
> 
> Testing: hs tiers 1/2/3/4
> 
> Thanks
> - Ioi