Bug 102136 - [compiler] Error message on ambiguous constructor only cites one of the conflicting methods
Summary: [compiler] Error message on ambiguous constructor only cites one of the confl...
Status: NEW
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.1   Edit
Hardware: PC Linux
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: JDT-Core-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-06-29 07:04 EDT by Maxime Daniel CLA
Modified: 2015-11-23 14:48 EST (History)
1 user (show)

See Also:


Attachments
draft patch for more complete strategy (9.55 KB, patch)
2010-03-13 12:42 EST, Stephan Herrmann CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Maxime Daniel CLA 2005-06-29 07:04:41 EDT
Eclipse 3.1

On the following source:
public class X {
	X(Integer o) {
	}
	X(String s) {
	}
	static X x = new X(null); // (a)
}

The compiler reports an error on line (a) which reads:
The constructor X(Integer) is ambiguous

whereas javac's diagnostic cites the two conflicting constructors.
Comment 1 Olivier Thomann CLA 2006-10-06 15:50:40 EDT
This is more about improving error message.
We might want to report more than one method in the error message.
Comment 2 Missing name Mising name CLA 2007-04-15 17:23:22 EDT
This applies to static and instance method calls as well as constructors.  It would be clearer to refer to "X(null)" or "X(?)" to indicate which parameter makes the call ambiguous (in the case of the constructor/method having multiple arguments), or list the possible matches as Olivier suggests.
Comment 3 Stephan Herrmann CLA 2010-03-13 12:06:27 EST
I just came about this very problem finding the actual error message
not very helpful: X(Integer) is *not* ambiguous, only the ctor *call* is.

My first idea was (like comment 2) to report 
  "The constructor X(null) is ambiguous"
This would be really easy to implement, except that we might lose some
information if ambiguity is caused by erasing, in which case method
parameters are more interesting than call arguments.

The experiment I made only involved method
   Scope.mostSpecificMethodBinding(..)
Simply change
  return new ProblemMethodBinding(visible[0], visible[0].selector, visible[0].parameters, ProblemReasons.Ambiguous);
to
  return new ProblemMethodBinding(visible[0], visible[0].selector, argumentTypes, ProblemReasons.Ambiguous);

That would be the poor-man's solution :)
To review the impact of this change you might want to run
AmbiguousMethodTest where compiler output of 20 tests changes with the 
above modification. At a quick glance these changes looked OK to me.

Reporting two (of possibly more) conflicting methods would
certainly be beneficiary (and doesn't seem too difficult either).

Olivier, what would you suggest?
Comment 4 Stephan Herrmann CLA 2010-03-13 12:42:20 EST
Created attachment 161963 [details]
draft patch for more complete strategy

I quickly drafted a patch that implements the more explicit version
mentioning two conflicting method matches.

I added the test from this bug which now prints:
  Ambiguous constructor call for the type X, both <init>(Integer) and <init>(String) match

I should mention one location where a ProblemMethodBinding with 
ProblemReason.Ambiguous is created, which I didn't change, because I didn't
immediately grasp the logic. It's in Scope.findMethod(..).

For that reason I conservatively added new ProblemIDs rather than
changing the output for existing ones (which would of course be
better to shield content assist from the change).

With this patch in place AmbiguousMethodTest produces 24 failures 
pointing you to the output changes.
Comment 5 Stephan Herrmann CLA 2010-03-13 12:44:54 EST
(In reply to comment #4)
>   Ambiguous constructor call for the type X, both <init>(Integer) and
> <init>(String) match

Ups, I should have replaced the selector ("<init">) with the declaringClass'
readableName ("X"), sorry.