Bugzilla – Full Text Bug Listing
| Summary: | [1.5][compiler] Name Clash error occurs | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | akujtan | ||||||
| Component: | Core | Assignee: | Srikanth Sankaran <srikanth_sankaran> | ||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | amj87.iitr, daniel_megert, eclipse, frederic_fusier, msa, Olivier_Thomann, satyam.kandula | ||||||
| Version: | 3.6 | Flags: | frederic_fusier:
review+
satyam.kandula: review+ |
||||||
| Target Milestone: | 3.6.1 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows XP | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
Although this compiles successfully with javac 1.6.0_21, I am not sure javac is correct; I think this actually should be an error. The relevant section of the Java Language Spec is 8.4.2:
"It is a compile-time error to declare two methods with override-equivalent signatures (defined below) in a class.
Two methods have the same signature if they have the same name and argument types.
Two method or constructor declarations M and N have the same argument types if all of the following conditions hold:
* They have the same number of formal parameters (possibly zero)
* They have the same number of type parameters (possibly zero)
* Let <A1,...,An> be the formal type parameters of M and let <B1,...,Bn> be the formal type parameters of N. After renaming each occurrence of a Bi in N's type to Ai the bounds of corresponding type variables and the argument types of M and N are the same."
The methods both have the same name; they both have one formal parameter; and neither has a type parameter (although the classes that declare the methods do have type parameters, that is not part of the definition). Importantly, the return type is also not part of the definition.
We can simplify your example to make Function have only one type argument, and hard-code its return type to be Boolean. javac will still compile without error. However, if we then change the return type to be boolean, with a lower-case 'b', javac reports a name clash.
So javac is behaving differently depending on the return type alone; and that is a violation of the JLS, as I read it. I suspect that Galileo and javac are both in error, and Helios is correct.
Disclaimer: I am not a language lawyer.
(In reply to comment #1) > The methods both have the same name; they both have one formal parameter; and > neither has a type parameter (although the classes that declare the methods do > have type parameters, that is not part of the definition). Importantly, the > return type is also not part of the definition. When you say "The methods both have ... " are you referring to Concrete's apply methods ? They are not clashing here since their signature is altogether different. I do think the code is valid and eclipse is behavior is not. This bug is very likely a duplicate of bug# 321548 rather than bug #317719 where javac was at fault. I will confirm after more analysis. (In reply to comment #2) > When you say "The methods both have ... " are you referring to Concrete's > apply methods ? They are not clashing here since their signature is > altogether different. Yes, those are the methods I mean. But I am not sure I agree with your assertion. First, we need to ignore the return type. Nothing in the JLS says that the return type is part of the signature. So, replace the example with one where the return types are the same. I think if you do that in bug 321548, then javac will also report a name clash. So that says that javac is (incorrectly, I think) paying attention to return type. That said, it does make sense that the compiler _should_ be able to tell these apart, given the substitutions of actual types for type variables in the class definition. But I don't see where in the language spec that takes effect. I am not by any means sophisticated when it comes to the JLS and generics, so maybe I am just misunderstanding. But I do think we should base our investigations on an example that uses the same return type, because at least that part of the spec seems pretty clear. (In reply to comment #3) > (In reply to comment #2) > > When you say "The methods both have ... " are you referring to Concrete's > > apply methods ? They are not clashing here since their signature is > > altogether different. > > Yes, those are the methods I mean. But I am not sure I agree with your > assertion. These methods are not clashing at all as their signatures are blatantly different (even ignoring the return types as you rightly point out below we should). No substitution is needed to establish this fact as there are no type variables in the picture in the signature of these two methods. They use concrete types in their signatures. > First, we need to ignore the return type. Nothing in the JLS says that the > return type is part of the signature. So, replace the example with one where > the return types are the same. Yes. > I think if you do that in bug 321548, then javac will also report a name clash. > So that says that javac is (incorrectly, I think) paying attention to return > type. Yes, javac would report an error if you change the int to void consistently in the example in bug# 321548, AND that would be the right behavior. In order for virtual method dispatch to work correctly, the compiler sometimes has to generate an internal bridge method. For example given: public interface Interface1<T> { public void hello(T greeting); } public interface Interface2<T> { public int hello(T greeting); } public class ErasureTest implements Interface1<String>, Interface2<Double> { public void hello(String greeting) { } public int hello(Double greeting) { return 0; } } ErasureTest's method hello(String greeting) overrides Interface1's void hello(T greeting); So if you have a call that looks like: Interface1<String> i = new ErasureTest(); i.hello("Blah"); However this will not happen unless the compiler generates a suitable bridge method. At the JVM level method dispatch very much involves return type ("method descriptor" which is used to symbolically reference methods to be invoked is a combination of name, parameter count and types and return type all erased) So the above call will be translated to 11: invokeinterface #5, 2 // InterfaceMethod Interface1.hell o:(Ljava/lang/Object;)V The method descriptor used in invocation here is hello:(Ljava/lang/Object;)V So this byte code instruction says invoke the interface method named hello which takes an java.lang.Object as argument and returns void. This call cannot be bound to ErasureTest's method void hello(String greeting) whose descriptor is hello:(Ljava/lang/String;)V So in order for polymorphism to work expected in the presence of generics, the compiler will silently a synthesize bridge method in ErasureTest of the form public void hello(Object o) { // syntnetic method hello(String o); // invoke Erasure's test void hello(String) method } Now returning to your example where the return types are forced to be the same (say void), the compiler is in an unhappy of situation of having to synthesize two bridge methods with the same descriptor which should bridge the calls to two different destination - an impossible task and hence a valid complain about name clash. Bottom line is in a class, no two methods cannot have same descriptor: if either because the user attempted to supply two such methods (may be in the same class, or by inheritance + supply a new method) or because the situation got entered into by the compiler's need to generate a bridge method collided with what the programmer has provided, a name clash error will result. JLS3 discusses this in 15.12.4.5 "Create Frame, Synchronize, Transfer Control" in the discussion box. (search for "bridge", in my pdf copy it is on page 478) Also worth noting is that the text of the compiler's complaint does not say anything about these two methods. (Multiple markers at this line - Name clash: The method hello(T) of type Interface1<T> has the same erasure as hello(T) of type Interface2<T> but does not override it - Name clash: The method hello(T) of type Interface2<T> has the same erasure as hello(T) of type Interface1<T> but does not override it ) In summary, after preliminary analysis, it does point to an eclipse issue. I'll confirm after detailed analysis. HTH. (In reply to comment #4) I pressed the commit button too fast, some clarifications are in this update: > So if you have a call that looks like: > > Interface1<String> i = new ErasureTest(); > i.hello("Blah"); I was going to say, this call is supposed to be dispatched to ErasureTest's hello(String) and that > However this will not happen unless the compiler generates a > suitable bridge method. [...] > So in order for polymorphism to work expected in the presence > of generics, the compiler will silently a synthesize bridge > method in ErasureTest of the form > > public void hello(Object o) { // syntnetic method > hello(String o); // invoke Erasure's test void hello(String) method > } The method should look like: public void hello(Object o) { // syntnetic method hello((String) o); // invoke Erasure's test void hello(String) method } Targetting 3.6.1 since this is a regression with respect to 3.5. *** Bug 321548 has been marked as a duplicate of this bug. *** (In reply to comment #6) > Targetting 3.6.1 since this is a regression with > respect to 3.5. Thanks for targetting 3.6.1. I have a codebase that won't build in Helios because of this bug. Created attachment 176204 [details]
Patch under consideration
All tests pass. Frederic, Satyam, please review, TIA. Satyam, please review for 3.6.1. TIA. This problem has been dormant for sometime and came to the fore due to the change done in bug# 289247 To be clear, this is not a regression caused by the fix for bug# 289247. Basically, while verifying that a type satisfies the contract it is obligated to satisfy given its supertypes, we should not be looking for name clashes between methods from supertypes that have been overridden in the current type (and so are hidden in the current class and so cannot contribute to a clash). See also https://bugs.eclipse.org/bugs/show_bug.cgi?id=293615 for a very similar, but not identical problem. Comment on attachment 176204 [details]
Patch under consideration
Grrrr. This patch breaks some cases and will have
to be reworked.
Created attachment 176228 [details] Revised patch The older patch attached to comment# 9 passes all the tests. However while experimenting somewhat with the bits I discovered some issues: (1) The original fix for bug# 83162 is not proper. It did make the problem go away as a side effect of the purported fix, but it was not really addressing the root cause of the problem reported in that bug which is that it is impossible for one bridge method to bridge to two targets. (2) There was also no regression test for the bug 83162 that directly captured the scenario in bug 83162 comment 0 There were several related variants, but none that mapped directly to the reported sample. (This has now been added as a part of the current fix) - Part of the current fix involved massaging some of the same code involved in (1) above, that removed the indirect fix to bug 83162 that the code referred to in (1) was affording. The current patch includes proper fix for bug 83162 also. Essentially, a name clash (methods having the same erasure, without being override-equivalent) can arise because - Two programmer supplied methods in a class clash - A programmer supplied method clashes with an inherited method. - A synthetic bridge method clashes with a programmer supplied method - A synthetic method clashes with another synthetic method. - Two inherited methods clash. You can also have a clash between a synthetic method and an inherited method, but at this time, both javac (5,6,7) and eclipse don't handle this case well: Try running the following program with both javac and eclipse: class Base { boolean equalTo(Object other) {return false;} } interface EqualityComparable<T> { boolean equalTo(T other); } public class SomeClass extends Base implements EqualityComparable<String> { public boolean equalTo(String other) { return true; } public static void main(String args[]) { new SomeClass().equalTo(args); } } I don't think the CCE is justified in either case and I believe both compilers are buggy. Passes all tests, Frederic, please review for 3.6.1 - TIA. Satyam, please review, TIA. Ayush, please review, TIA. Patch looks good to me Patch looks good for me. Released in HEAD for 3.7 M2
3.6 maintenance stream for 3.6.1
(In reply to comment #14) [...] > You can also have a clash between a synthetic method and an inherited > method, but at this time, both javac (5,6,7) and eclipse don't handle > this case well: > > Try running the following program with both javac and eclipse: [...] > I don't think the CCE is justified in either case and I believe > both compilers are buggy. Raised bug 322740 for follow up. Verified for 3.6.1 using build M20100825-0800. Verified for 3.7M2 using I20100914-0100 |
Build Identifier: 20100617-1415 When I switched from Galileo to Helios this code started throwing the error, "Name clash: The method apply(F) of type Function<F,T> has the same erasure as apply(T) of type Predicate<T> but does not override it" Whereas it would compile fine in previous versions. Code: class Bar {} class Foo {} interface Function<F, T> { T apply(F f); } interface Predicate<T> { boolean apply(T t); } public class Concrete implements Predicate<Foo>, Function<Bar, Boolean> { @Override public Boolean apply(Bar two) { return null; } @Override public boolean apply(Foo foo) { return false; } } Reproducible: Always Steps to Reproduce: Try to compile the code.