Re: invoke-special patch

[Per Bothner <per at bothner dot com>]

Chris Dean wrote:
> Attached is a patch to implement invoke-special.  This procedure is very
> similar to invoke and invoke-static but is used to invoke a method in
> certain special ways.  One interesting use is to invoke a method in your
> super-class like the Java language super keyword.

It's a good patch (very few people have submitted Kawa patches for 
anything close to this difficulty) - but it could be better.  Here are 
some questions and suggestions:

It needs ChangeLog entries.

kawa.texi:  Instead of "This procedure is only available at compile-time."
perhaps the "The compiler must be able to inline this procedure (because
you cannot force a specific method to be called using reflection).
Therefore the @var{class} and @var{name} must resolve at compile-time
to a specific method."

Also "used to invoke a method in certain ways" is a bit lame as a
description.  How about: "invokes the specified method, ignoring any
methods in subclasses that might overide it"

The new PrimProcedure method might be better as a static method?

Any reason why ClassMethods.removeMethods is public?
If it is public, it should have a documentation comment.
Or just make it private.  It might be better called
removeRedundantMethods.  Or just leave it the way it was ...

For invoke-special, removeMethods should not be needed.
Instead, just call ClassType.getMethods with searchSupers==0.
A possible down-side:  This will not find methods that are
inherited (and not overidden), so code may break (at compile-time,
not run-time) if a method is moved to a superclass.  On the
other hand, maybe that's ok.

In general style changes (such a new removeMethods method,
and changes to PrimProcedure.compileArgs) should be separate
from real changes, so I can evaluate them separately.
Especially if they're non-obvious.

In Invoke.inline, I'd inline:
+    int margsLength = calcMargsLength(nargs);
+    int argsStartIndex = calcArgsStartIndex();
+    int objIndex = calcObjIndex();
and do:
     int margsLength, argsStartIndex, objIndex;
     if (kind == 'V')
       {
         // (invoke-virtual this name args ...)
         margsLength = nargs - 1;
         argsStartIndex = 2;
         objIndex = 0;
       }
     else ...
This is more efficient, and I think easier to understand.

The change in Inline.inline to supress the warning if the type
is Object is reasonable, but it has a downside that people
won't add the type declarations that allow inlining and
catch errors.  This should be a separate discussion - perhaps
it should be controlled by some future "optimize" switch.
Until then, a possible solution may be to supress the warning
in immediate mode:  If ((InlineCalls) walker).getCompilation().immediate.

I don't understand the need for ClassType.isValid().

I need to update the copyright dates.
--
	--Per Bothner
per@bothner.com   http://per.bothner.com/