Breaking ties in method dispatch; and, better error messages

[Chas Emerick]

I hit a bug in 1.3.0-alpha6 as well as in -master-SNAPSHOT related to selecting methods when overloads create the potential for ties.  For background, the concrete situation is related to this class in rummage that has a number of overloads for different primitive numeric types, e.g.:

https://github.com/cemerick/rummage/blob/master/src/main/java/cemerick/rummage/DataUtils.java#L207

Calling encodeRealNumberRange with properly coerced arguments (in my case, I needed the (long, int, long) overload) resulted in the dreaded "More than one matching method found" error message.  Odd, since I'm matching the signature exactly.

This is caused by a compiler bug; filed as CLJ-789, with a patch that includes a minimal testcase (more easily viewable in my corresponding github branch).  Seems straightforward enough; I had to tweak the build slightly to add test Java source compilation, but I presume that wouldn't be controversial.

***

Somewhat more interesting is that, in the process of understanding what was going on in the above problem, I implemented some better error reporting for the Compiler.getMatchingParams method.  The result has been pushed here; the impact is that instead of:

More than one matching method found: foo

the same error now reports as:

More than one matching method found: foo(long[], int, int) and foo(long[], long, int) for arguments of type (long[], long, long)

 

I assume that's desirable; if I get a yea from a core team member, I'll open a ticket and attach a patch.

Cheers,

- Chas

[Stuart Halloway]

Somewhat more interesting is that, in the process of understanding what was going on in the above problem, I implemented some better error reporting for the Compiler.getMatchingParams method.  The result has been pushed here; the impact is that instead of:

More than one matching method found: foo

the same error now reports as:

More than one matching method found: foo(long[], int, int) and foo(long[], long, int) for arguments of type (long[], long, long)

Yes please!

Stu

[Chas Emerick]

On its way, then.

Q: Should I extend this to reflection warnings as well?  e.g. instead of:

Reflection warning, %s:%d - call to %s can't be resolved

would this be preferred:

Reflection warning, %s:%d - call to %s can't be resolved with arguments of type (foo, bar, baz)

- Chas

[Stuart Halloway]

> On its way, then.
>
> Q: Should I extend this to reflection warnings as well? e.g. instead of:
>
> Reflection warning, %s:%d - call to %s can't be resolved
>
> would this be preferred:
>
> Reflection warning, %s:%d - call to %s can't be resolved with arguments of type (foo, bar, baz)

That sounds good too.

Stu

[Chas Emerick]

Done.  Issue with patch, the latter browsable here.

- Chas

[ataggart]

For my weekly dead-horse flogging, the reorg-reflector patch (i.e., no
functional behaviour changes) on the CLJ-445 ticket includes this
messaging improvement, as well as cleaning up the interactions between
Compiler and Reflector, and removing repeated code. If this was split
out into a separate ticket would it get in any sooner?

Aside: the prim-conversion patch on CLJ-445 fixes the problem in
CLJ-789.

http://dev.clojure.org/jira/browse/CLJ-445

[Chas Emerick]

I was vaguely aware of CLJ-445, but did not recognize its full scope. :-( Alex, we seem to get ourselves tangled up too often! :-)

I'm not really in a position to fully evaluate Alex's patches, but their objective is good, and the basic structure of things appears to be in line with what I was considering proposing before punting and making the minimal tweaks that I did. Unfortunately, I see it's been pushed to the backlog, and not on Release.Next.

If that can change, and CLJ-445 can get through, then by all means, prefer it. If not, then either my patches or some minimal subset of Alex's (I _really_ don't care) _must_ get into 1.3.0 before that window is closed — otherwise, it is broken in certain circumstances, as I documented up-thread.

- Chas

> --
> You received this message because you are subscribed to the Google Groups "Clojure Dev" group.
> To post to this group, send email to cloju...@googlegroups.com.
> To unsubscribe from this group, send email to clojure-dev...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/clojure-dev?hl=en.
>

[Christopher Redinger]

Yes, the deal with 445 being pushed to Backlog was that it was just
too big to bring in to Release.next as is. And really, it was the
changes to Compiler & Reflector that I think pushed it over the edge.

Alex - I believe you were working to pull out smaller patches from 445
anyway - would you mind pulling this out and see if we can get it in?

We are hoping to get an alpha-7 out soon. No promises, but maybe this
weekend. It would be nice if we could get this in as well.

[ataggart]

The reorg-reflector patch file contains 19 step-by-step commits for
easier comprehension. One could conceivably stop applying them at any
point. While that file contains messaging and structural changes, the
behaviour is unchanged, thus hopefully could make it in without too
much trouble.

The prim-conversion patch file continues from the base established by
reorg-reflector. It contains 3 commits, the final one being the
rather massive change to the method resolution code which can't really
be broken up without the individual commits being rendered
nonfunctional. That said, if the preceding commits are applied and
disregarded, it should be rather straightforward to understand, and I
can help out as needed.

> >> For more options, visit this group athttp://groups.google.com/group/clojure-dev?hl=en.

[ataggart]

Created a separate issue for the Reflector/Compiler reorganization:

http://dev.clojure.org/jira/browse/CLJ-792