Bugzilla – Full Text Bug Listing
| Summary: | [1.7][compiler] Type inference for generic allocation can be avoided for invalid constructor | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Ayushman Jain <amj87.iitr> | ||||||||||
| Component: | Core | Assignee: | Ayushman Jain <amj87.iitr> | ||||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||||
| Severity: | normal | ||||||||||||
| Priority: | P3 | CC: | deepakazad, srikanth_sankaran | ||||||||||
| Version: | 3.7 | ||||||||||||
| Target Milestone: | 3.7.1 | ||||||||||||
| Hardware: | All | ||||||||||||
| OS: | All | ||||||||||||
| Whiteboard: | |||||||||||||
| Attachments: |
|
||||||||||||
Created attachment 195479 [details]
experimental patch
(In reply to comment #1) > Created attachment 195479 [details] [diff] > experimental patch This is not quite right. To get the correct constructor we first need the inferred type. The other approach can be that inside org.eclipse.jdt.internal.compiler.lookup.Scope.getStaticFactory(ReferenceBinding, TypeBinding[], InvocationSite), we skip the constructors which have different number of arguments than the number on the invocation site. This is not really mentioned in the spec though. I don't see any use of contriving factory methods corresponding to constructors with different number of arguments. They will never map onto the constructor that has actually been called. Srikanth, do you see any reason why we generate synthetic factories for ctors with number of args <= args with which ctor has been invoked? (In reply to comment #2) > (In reply to comment #1) > > Created attachment 195479 [details] [details] [diff] > > experimental patch > > This is not quite right. To get the correct constructor we first need the > inferred type. > > The other approach can be that inside > org.eclipse.jdt.internal.compiler.lookup.Scope.getStaticFactory(ReferenceBinding, > TypeBinding[], InvocationSite), we skip the constructors which have different > number of arguments than the number on the invocation site. Yes, we can, there are no positives to synthesizing factory methods where arg count would disqualify a constructor. We need to accommodate varargs properly. Scope.java lines 525-530 has the relevant code. We should also clean up things so that when inference fails we set resolved type to null and return null immediately. This will prevent the flurry of additional errors that we see today - cf javac. Created attachment 195691 [details]
proposed fix
This patch makes sure no static factory method is created corresponding to a constructor with no. of arguments different from that used in the allocation expression. Varargs have also been appropriately handled. Also, if no matching constructor is found, we return null instead of constructing the problem binding.
Srikanth, can you please see if the behaviour with this patch is ok? I'll add a test and release it.
One aesthetic point: Do you think the user will be better assisted with an "invalid constructor" error than a "cannot infer elided type" error? Or should the latter error be elaborated to say that the inference failed due to invalid constructor? With above patch, we match javac's diagnosis though.
Created attachment 195699 [details]
Another patch
Same patch with some obvious comments deleted and the error handling
approach unified for all manner of errors not just argument count
mismatch.
This patch is untested, in particular will require remastering of some
tests as we will not report a flurry of secondary errors now.
I think it is ok to just report failure in inference as that matches
javac behavior.
Added tests. Released in BETA_JAVA7 Disabled tests org.eclipse.jdt.core.tests.compiler.regression.GenericsRegressionTest_1_7._test0034() to 36 will be enabled after the fix for bug 345968 Created attachment 195879 [details]
released patch
Verified by code inspection. |
BETA_JAVA7 Currently for the following case the type inference mechanism is wastefully performed to infer types for diamond case. But since the constructor itself is invalid, we should short-circuit that. class X<T> { X (T a) { System.out.println("const1"); } X() { System.out.println("const2"); } void foo(T a) { System.out.println(a); } public static void main(String[] args) { X<String> x = new X<>("",""); // See the errors here } }