Re: Patch: Remove exception catching from normal path in Arrays.equals.

[Andrew Haley <aph at redhat dot com>]

Mohan Embar writes:
 > 
 > While we're on this subject, there are myriad places (natClass.cc,
 > natRuntime.cc, natArray.cc, natConstructor.cc, natField.cc, natMethod.cc,
 > natResourceBundle.cc) where gnu.gcj.StackTrace is being used
 > like this (example from natArray.cc):
 > 
 > try
 > {
 >   for (int i = 1; !caller; i++)
 >     {
 >       caller = t->classAt (i);
 >     }
 >   caller_loader = caller->getClassLoaderInternal();
 > }
 > catch (::java::lang::ArrayIndexOutOfBoundsException *e)
 > {
 > }
 
Note that the catch clause will not be invoked unless there is a bug in gcj.

 > Unless I'm not understanding something correctly, it seems to me that this
 > variant is much easier to read:
 > 
 > for (int i = 1; !caller && i < t->length (); ++i)
 >   {
 >     caller = t->classAt (i);
 >   }
 > if (caller)
 >   caller_loader = caller->getClassLoaderInternal ();

This complicates the code for no good reason.

 > And this one would be even better:
 > 
 > Class* caller = t->tryTofindFirstNonNullClassStartingAtIndex (1);
 > if (caller)
 >   caller_loader = caller->getClassLoaderInternal ();
 > 
 > ...where tryTofindFirstNonNullClassStartingAtIndex, a new method of
 > StackTrace, is replaced by a better name (I'm bad at names).

:-)

This isn't a totally bad idea.  But findCallerClass might be a better name.

 > I've pondered submitting a patch for this, but haven't, because
 > I don't think that the code in question suffers from bad performance.
 > The patch would be mostly aesthetic.
 > 
 > Should I submit a patch anyway? Or is this not worth it?

I don't see that it's easier to read.

This area of libgcj is going to get some fairly serious attention
quite soon because there are problems with access control of these
methods.

Andrew.