Re: OVERRIDE Multiple methods with the same name and argument types

["Daniel Bonniot" <bonniot at cma dot ensmp dot fr>]

> > gnu.bytecode.ClassType currently allows to have several methods with the
> > same name and argument types, even when using the
> > addMethod(String name, int flags, Type[] arg_types, Type return_type)
> > that pretends to check if the method already exists.
>
> addMethod(String, int, Type[], Type) should probably be deprecated.
> The name is misleading, since it doesn't always add a new method.  The
> functionality has been largely subsumed by getDeclaredMethod, which
> are also more convenient.
>
I agree with deprecating it, and modifing the callers to combine
getDeclaredMethod and addMethod. That's what I do at the end of this message
for PrimProcedure.

> > The problem is that addMethod creates a new one if the old has different
> > return type and/or access flags. I consider this is a bug, since the
> > classfile generated will be invalid.
>
> Well, gnu.bytecode is not meant to prevent you from generating
> invalid or meaningless classfiles.  It is meant to be a somewhat
> low-level toolkit.  That doesn't mean we can't add some error-checking
> when it is simple to catch errors!
>

I think most errors leading to invalid classfiles should be detected. I
mean, there can't be any use in generating invalid classfiles ! But you're
right, it should do simple error checking first. My proposal is to add this
getDeclaredMethod that checks the return type, but you don't have to use it.
And this is a simple check !

> > I encountered this problem using new PrimProcedure(ClassType classtype,
> > Type[] argTypes). It led to several constructors in the same class with
just
> > differing flags (when a public constructor already existed, it created a
new
> > one).
>
> Can you go into more detail?  I.e. who calls PrimProcedure, and why
> are we getting a new constructor if one already exists?
>

Remember I have the point of view of a compiler writer, that uses
gnu.bytecode/gnu.expr as a code generator.
So I call PrimProcedure when I want to generate code for a "new" expression
(call to the constructor).

I get a new constructor because:
* PrimProcedure adds the method (in my case "<init>") with hard wired 0 (or
Access.STATIC for static methods) access flags.
* ClassType.addMethod(String name, int flags, Type[] arg_types, Type
return_type)
   adds a new method if the old one with same name and arg types has
different return type or flag.
   So the check is there, but it decides to add a new method, which means
the class generated will be invalid.

So even if I correctly added a -- say PUBLIC -- constructor to a ClassType
ct, and then try to compile a
    new CallExp (new QuoteExp (new PrimProcedure (ct, ...)), ...)
This adds a second constructor to ct, just with a different access flag.

> > I propose:
> > * Add a getDeclaredMethod(String name, Type[] arg_types, Type
return_type)
> > to ClassType. This is a convenient method that calls
> > getDeclaredMethod(String name, Type[] arg_types) and then if a method
was
> > found, it verifies that its return type is the same, otherwise throws an
> > Error.
>
> I'm not sure I like that.  I'm not sure it belongs in gnu.bytecode.
> For example, if there is an error because a Scheme programmer
> mis-wrote primitive-virtual-method (for example), we want a
> standard compiler error message, including source line number.
> Throwing an exception should be avoided.  We should throw Error
> to indicate internal errors in Kawa, not for other compile-time errors.
>
Yeah. The error should be detected earlier, in a language specific fashion.
That doesn't prevent us from adding a check in gnu.bytecode, which could
help debugging the high-level compiler (Kawa, or mine, or ...).
I agree throw new Error is not that good, there should be a hierarchy of
exceptions root at something like BytecodeException. Then it could be
catched by the high level compiler, and reported as an internal error. But
this is a lot of work...
Are there people using gnu.expr to compile some high level language around ?
What do they think of those issues ?

> > * Modify PrimProcedure, so that it first checks if the method exists (in
> > that case verify the static flag is correct), and if not creates a new
one.
>
> Again, I think it's the wrong place to signal the error - you should
> try to catch it earlier.
>
There are two issues:
- error reporting. I agree the error should be detected before. I think it
would be fine to check also here, because it would greatly help using
gnu.expr in various compilers. But that's your choice, this is not
eseential, since I can do without it.
- creation of new methods (like in the "<init>" example above). Something
has to be done in PrimProcedure, since it does the wrong thing at the
moment. This is linked to the fact that is calls the addMethod that should
be deprecated. Just changing this would solve the problem. For instance it
could be:

  public PrimProcedure(int op_code, ClassType classtype, String name,
         Type retType, Type[] argTypes)
  {
    this.op_code = op_code;
    if (op_code == 185) // invokeinterface
      classtype.access_flags |= Access.INTERFACE;

    method = classtype.getDeclaredMethod(name, argTypes);

    if (method == null)
      method = classtype.addMethod(name,
       op_code == 184 ? Access.STATIC : 0,
       argTypes, retType);
    else
      method.setStaticFlag(op_code == 184);

    this.retType = retType;
    this.argTypes= argTypes;
  }

That is, it does not throw errors, but it does not create a method if one
matching already exists.
No change to ClassType is required for this version (except the deprecation
which would be nice).

> Have you looked at whether the classes in gnu.kawa.reflect will
> work for you?

They seem to be linked to the Scheme world, aren't they ?

Daniel