Re: [cp-patches] Patch: RFC: removing accessor methods

[Mark Wielaard]

Hi,

On Mon, 2004-09-20 at 01:37, Tom Tromey wrote:
> This jumbo patch fixes almost all the instances of private member
> access requiring trampoline methods that were found by gcjx.

Very nice. This makes the code a little faster and smaller.
At least when compiling to bytecode. I guess when compiling to native or
using some other representation for java source code you won't have this
problem in the first place.

> In addition to this I propose we adopt this as part of our coding
> style.  I can prepare a patch against the documentation for this.

Yes that would be a good idea.
We should add a warning log to developer.classpath.org as soon as it is
stable and we have gcjx compiled on it (a preview is already available
at classpath-dev.wildebeest.org, but not much content and Jim Pick and I
are still analyzing the stability of the setup. be gentle)

> Even after this patch there is one remaining instance of this problem:
> 
> /home/tromey/gnu/classpath/classpath//java/net/URLClassLoader.java:817:11: 
> warning: method `java.security.SecureClassLoader.defineClass 
> (java.lang.String, byte[], int, int, java.security.CodeSource)' requires 
> forwarding call
> /home/tromey/gnu/classpath/classpath//java/security/SecureClassLoader.java:80:36:
>  warning: method is defined here
> 
> Fixing this would require adding an explicit helper method to
> URLClassLoader -- but in this case we might as well let the compiler
> do it for us.  (This particular warning comes from referencing a
> protected member that is inherited by the outer class; so it is not
> quite the same as referencing a private member.)

Interesting, this is precisely one of the PrivilegedActions that Bryce
likes to complain about. And in this case he would (again) be right. We
construct a PrivilegedAction object on the fly each time. This could be
made into a static package local DefineClassAction class object that is
initialized once and then always reused. Then we would need the explicit
package local helper method to use it.

> Any comments?  Concerns?  Objections?

This should really have comments for why field, methods or constructors
are package local. At least the lone new no-argument constructors
deserve a comment. If you manage to get this into classpath and libgcj
at the same time that would be great.

Also we would like to have a warning when such a package local method
isn't used anymore so we can make it private again after refactoring.
But I guess that needs whole-package-analysis which is harder.

Thanks,

Mark

signature.asc
Description: This is a digitally signed message part