RE: A few small fixes

[Jeroen Frijters]

David Holmes wrote:
> Jeroen Frijters wrote:
> > David Holmes wrote:
> > > Why is it incorrect to call loadClass(name, true)?
> >
> > Because it isn't public. The fact that it happens to be accessible
> > because it is protected and Class happens to be in the same
> > package as
> > ClassLoader doesn't mean it is right to call it. Plus, the fact that
> > loadClass(name) is public and non-final means that subclasses can
> > override it, so we need to call that to be compatible.
> 
> I find that logic rather strange. It could be that the main intent was
> to allow loadClass(String, boolean) to be package accessible and that
> it was made "protected" because it might be useful for subclasses to
> override.
> 
> In short there is nothing wrong with classes within the same package
> using package accessible methods. ;-)

The question I asked myself was: Would loadClass(name, resolve) be
package accessible if Java still had the "private protected" access
modifier from the pre-1.0 days? I think it wouldn't.

> > This is *not* in internal implementation detail. I've
> > written a custom
> > class loader and ran it under the Sun VM and observed the
> > Classloader.loadClass(name) being called from Class.forName().
> 
> ?? ;-)  Did you also observe ClassLoader.resolve() being called?

No, since ClassLoader.resolveClass() is final, I couldn't override it.

> Given that loadClass(name) is specified as equivalent to
> loadClass(name,false) I see these as functionally equivalent:
> 
>    Class clazz = cl.loadClass(name);
>    cl.resolveClass(clazz);
> 
> vs.
> 
>    cl.loadClass(name, true);
> 
> You can prefer the former if you like, but the latter is certainly not
> incorrect. Mind you, the former requires that you invoke the
> non-public resolveClass() method - so by your earlier reasoning that
> would be incorrect. In which case I'm not sure how you will
> resolve/link the class as required by Class.forName. ;-)

There is no public equivalent of resolveClass, so it wouldn't be
incorrect to call it from Class.forName().

> > This isn't some random thing I dreamed up. I actually ran into this
> > problem while trying to get JBoss to run on IKVM. JBoss has a broken
> > custom class loader that overrides loadClass(name).
> 
> Ah I see. I had no idea where this approach came from, but if JBoss is
> broken report it as a bug. The semantics of loadClass(name) are
> well-defined. Pity they didn't make it final.

The fact that ClassLoader.loadClass(name) isn't final is an unfortunate
design flaw, but since it cannot be changed and there is code out there
that overrides it, I suggest we take the easy solution of calling it
(and explicitly calling revolveClass, like your did above).

> > I assume you mean using JNI to call ClassLoader.loadClass(name)
> > non-virtually. As can be deduced from the above, Sun's
> > doesn't do this, so neither should we.
> 
> No I meant that both Class and ClassLoader could defer to a VM
> specific mechanism. But thinking further it's probably not an
> unreasonable assumption that forName actually invokes method(s) on the
> classloader that is passed to it. It's a pity that this isn't clearly
> specified though.

There is so much unspecified that it isn't funny, but this is all the
more reason to try to emulate the Sun implementation (where it is
externally visible), instead of just implementing what the documentation
says.

Regards,
Jeroen