bugKawa - Bugs: bug #39048, Bad method call resolution?

 
 

bug #39048: Bad method call resolution?

Submitter:  Matthieu Vachon <maoueh>
Submitted:  Thu 23 May 2013 03:48:52 AM UTC
   
 
Category:  None Severity:  3 - Normal
Item Group:  None Status:  Fixed
Privacy:  Public Assigned to:  bothner
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Sun 09 Jun 2013 12:10:33 AM UTC, comment #3: 

I checked in fix (1) "There is an exact match for image-descriptor, which should take precedence over getImageDescriptor."

I also checked in a testcase based on your test, but tweaked so I don't need to create a helper class (or any new files at all).  See testsuite/use-slots.scm.  See also bug #39209 about why a separate file (or at least separate evaluation) for the base class is needed.

Per Bothner <bothner>
Group administrator
Fri 07 Jun 2013 02:33:33 PM UTC, comment #2: 

Thanks for the link to the documentation, I guess I missed it when I first search for it. Didn't know this feature since we do not use it. But we use the other form which consist of using dashes to separate words of a method call.

First, about point 2), I totally agree with you that this case is
a bit special. We use it in our code base (i.e. a field is a
procedure) but we don't have cases where the field collapse
with a method but I guess it could happen. For example,
we could have a field `validator` and one of the method in the class could be named `getValidator`.

This is how I see a possible solution to the problem. We have some sort of ambiguity and must choose the closest possible match. I think a ranking mechanism (maybe with other criteria) could solve almost every case. For other cases, users should be responsible to avoid collision.

From my point of view, if we have more than one possible method that could be called, we should first look at the arity of the method. If arities are different, pick the one with the closest arity (or even issue a warning if arity does not match any).

If there is still more than one method, I see two possibilities.

 1. We use name to discriminate. A kind of distance between the two names could be used, and we pick the one with the closest match. So `image-descriptor`, since it exactly match outer-scope function, would call this one. On the other side, using `get-image-descriptor` would call class method.

 2. We use the types to discriminate. But it could be more difficult to choose the right method in this case. Not sure it would be that good or even possible due to dynamic nature of language.

I have a preference for only using 1. But we could combine the two possibilities and order of checks could be 1 then 2 or 2 then 1. But I have a preference for order 1 then 2, which should be more accurate.

Just my two cent on the issue. Whatever way you decide to take, I will follow it. In fact, I already workaround the problem by creating a proxy method outside of class scope that is calling the right procedure. The proxy just have a name that do no collide with `getImageDescriptor`.

Regards,
Matt

Matthieu Vachon <maoueh>
Mon 03 Jun 2013 05:59:26 PM UTC, comment #1: 

Well, in general a method getImageDescriptor corresponds to a "property" with the name image-descriptor.  See:
http://www.gnu.org/software/kawa/Field-operations.html
Thus if you write image-descriptor within the scope of a class that has a method getImageDescriptor, then that is viewed as accessing the property image-descriptor.

Now, there are two reasons one could view Kawa as doing the wrong thing for your program:
(1) There is an exact match for image-descriptor, which should take precedence over getImageDescriptor.
(2) The image-descriptor is in function-call position, so we should not accept getImageDescriptor for image-descriptor.

Argument (2) is awkward, because we might have a function-valued field - though that would be rare, and perhaps should require an exact match.

It looks like either or both fixes would be relatively straight-forward (the relevant code starts line 664 in Translator.java).  However, I have to ponder a bit (and experiment a bit) to determine the best fix.

Per Bothner <bothner>
Group administrator
Thu 23 May 2013 03:48:52 AM UTC, original submission:  

Assume the following interface:


package org.kawa.test.getterclash;

public interface Simple {
    public String getImageDescriptor();
}


And the following Kawa source file:


(module-name <org.kawa.test.getterclash>)
(module-compile-options main: #t)

(define (image-descriptor argument)
  (format "Ok with argument: ~a~%" argument))

(define (create-simple)
  (object (<org.kawa.test.getterclash.Simple>)
    ((getImageDescriptor)
     (image-descriptor "Should call define above, not getImageDescriptor"))))


(define (main)
  (let ((simple :: <org.kawa.test.getterclash.Simple> (create-simple)))
    (invoke simple 'getImageDescriptor)))

(main)


The problem here is that Kawa will call `getImageDescriptor` when seeing `(image-descriptor "Should call define above, not getImageDescriptor")` inside object method implementation. That will then cause a StackOverFlow at runtime since method is calling itself. This can also be seen by the warning issued mentioning `(image-descriptor argument)` is not used.

I check the documentation about that but did not see any places saying that define without prefix `get-` will resolve to method `getXxxxYyyy`.

If it's the case, I don't think this should be the behavior. Call should be more explicit and only `get-image-descriptor` should have worked in my opinion.

At least, even if we keep current mapping, since the called procedure as the not same arity, it should not call class getter.

I attached my test case in a zip archive.

Regards,
Matt

Matthieu Vachon <maoueh>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #28143:  getter-clash.zip added by maoueh (1KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by bothner (Posted a comment)
  • -email is unavailable- added by maoueh (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2013-06-09 bothner StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2013-06-03 bothner StatusNone In Progress
        Assigned toNone bothner
    2013-05-23 maoueh Attached File- Added getter-clash.zip, #28143

    Back to the top

    Powered by Savane 3.13-4b48.
    Corresponding source code