Summary: | Eclipse erroneously reports method "is ambiguous for type" | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] JDT | Reporter: | Tony Weddle <tony> | ||||||
Component: | Core | Assignee: | Srikanth Sankaran <srikanth_sankaran> | ||||||
Status: | VERIFIED FIXED | QA Contact: | |||||||
Severity: | normal | ||||||||
Priority: | P3 | CC: | jarthana, kent_johnson, Olivier_Thomann | ||||||
Version: | 3.6 | ||||||||
Target Milestone: | 3.6 M4 | ||||||||
Hardware: | PC | ||||||||
OS: | Windows XP | ||||||||
Whiteboard: | |||||||||
Attachments: |
|
Description
Tony Weddle
2009-10-26 22:27:07 EDT
Srikanth, please investigate while Kent can still guide you through the code. Thanks. Javac & eclipse behavior match upto 3.5M7 and started diverging in 3.5RC1. After preliminary investigation, this looks like is due to bad fix for bug# 275471. From bug# 275471 comment#3 > We no longer will force the hierarchy of every parameter type to be resolved > for an exact method match to proceed in 1.5. > Instead we will check to see if the parameter type's hierarchy has been > connected, if not we will look for all possible method matches. This changed behavior results in org.eclipse.jdt.internal.compiler.lookup.Scope.findExactMethod(ReferenceBinding, char[], TypeBinding[], InvocationSite) skipping over the exact match. Srikanth, We need to skip the exact match in this case, but there is another problematic test when we try to decide if the two methods are 'equal' because of substitution. (In reply to comment #3) > Srikanth, > We need to skip the exact match in this case, but there is another problematic > test when we try to decide if the two methods are 'equal' because of > substitution. Did you mean "we need to" or that "we will have to live with skipping the exact match" ? I mean 'we need to'. We have to look for inherited methods that could match, so the exact match cannot be used in this case. Once we skip the exact match, we then have to find the best match from the 2 possible matches. Created attachment 150825 [details]
Potential patch
Kent, please take a look. This fixes the problem without
breaking anything else. Let us discuss if something is
amiss tomorrow morning your time - Thanks.
1. Would normally add this test to AmbiguousMethodTest instead of GenericTypeTest 2. The change looks good, but you may want to rework it to be : if (candidate.hasSubstitutedParameters()) { for (int j = i + 1; j < visiblesCount; j++) { MethodBinding otherCandidate = candidates[j]; if (otherCandidate.hasSubstitutedParameters()) { if (otherCandidate == candidate || (candidate.declaringClass == otherCandidate.declaringClass && ...)) return new ProblemMethodBinding(candidates[i], ...); } } } (In reply to comment #7) > 1. Would normally add this test to AmbiguousMethodTest instead of > GenericTypeTest > 2. The change looks good, but you may want to rework it to be : > if (candidate.hasSubstitutedParameters()) { > for (int j = i + 1; j < visiblesCount; j++) { > MethodBinding otherCandidate = candidates[j]; > if (otherCandidate.hasSubstitutedParameters()) { > if (otherCandidate == candidate > || (candidate.declaringClass == otherCandidate.declaringClass && ...)) > return new ProblemMethodBinding(candidates[i], ...); > } > } > } Agree on both counts, I'll take it from there, Thanks! (In reply to comment #2) > Javac & eclipse behavior match upto 3.5M7 and started diverging > in 3.5RC1. > After preliminary investigation, this looks like is due to bad fix > for bug# 275471. For the record, this defect was latent and pre-existent and started manifesting after the checkin for the fix of bug# 275471, but was not caused by it. Olivier, do you want this for 3.5.2 ? (a regression relative to 3.4.2 or to 3.5 M7) (In reply to comment #9) > Olivier, do you want this for 3.5.2 ? (a regression relative to 3.4.2 > or to 3.5 M7) Yes depending on the risk's assessment. Created attachment 151069 [details]
Revised patch
Revised patch incorporting review comments
Released in HEAD for 3.6M4 Released. Verified for 3.6M4 using build I20091207-1800 |