Bug 350897

Comment 1 Ayushman Jain 2011-07-01 02:35:43 EDT

Srikanth, can you please take a look? Thanks!

Comment 2 Srikanth Sankaran 2011-07-01 03:21:45 EDT

Olivier, let me know if you need some support from the
compiler proper.

Comment 3 Markus Keller 2011-07-06 09:43:50 EDT

Created attachment 199184 [details]
API Stub and clients

Olivier asked me: Please describe exactly what you expect from this new API if possible with examples.

It's really the same as bug 174436 for the ClassInstanceCreation case. To better understand the problem myself and to be sure that we really need this API, it was the easiest for me to just start the implementation and actually use the API.

The attached patch contains the JDT UI implementation and the requested API. The missing piece is DefaultBindingResolver#isResolvedTypeInferredFromExpectedType(ClassInstanceCreation).


Example:

package diamond;
public class X2<T> {
	T field1;
	public X2(T param){
		field1 = param;
	}

	public static void main(String[] args) {
		X2<Object> a = new X2<Object>("hello");
		X2.testFunction(a.getField()); //prints 2
		X2<String> b = new X2<>("hello");
		X2.testFunction(b.getField()); // prints 1

		X2<Object> c = new X2<>(null);
		X2.testFunction(c.getField()); // prints 2
		X2<String> d = new X2<>(null);
		X2.testFunction(d.getField()); // prints 1
	}
	public static void testFunction(String param){
		System.out.println(1 + ", String param: " + param);
	}
	public static void testFunction(Object param){
		System.out.println(2);
	}
	public T getField(){
		return field1;
	}
}

The interesting case is variable d:
		X2<String> d = new X2<>(null);

That's the only place in the example, where the new method should return true. At variable b, the type of the CIC has been inferred from the argument type.

At variable c, the assignment context didn't matter, since the argument (null) already resulted in type Object.

Once the JDT Core method is correctly implemented, the "Inline Local Variable" refactoring on a, b, c, and d should not change the output of the program any more, and it should only add an explicit type argument when inlining d.

Comment 4 Srikanth Sankaran 2011-07-07 01:19:28 EDT

Created attachment 199222 [details]
Compiler changes.

This patch contains the compiler changes using which
suitable APIs could be created.

Now org.eclipse.jdt.internal.compiler.ast.AllocationExpression.inferredReturnType
is set if the expected type was used to drive the inference.

Comment 5 Srikanth Sankaran 2011-07-07 01:32:09 EDT

(In reply to comment #3)

> That's the only place in the example, where the new method should return true.
> At variable b, the type of the CIC has been inferred from the argument type.
> 
> At variable c, the assignment context didn't matter, since the argument (null)
> already resulted in type Object.

FYI - This last part of the comment isn't correct. For both c & d, we proceed to 15.12.2.8.

In the analogous example with static factories:

public class X<T> {
	static <U> X<U> getX(String s) {
		return null;
	}
	static <U> X<U> getX(Object o) {
		return null;
	}
    public static void main(String[] args) {
    	X<Object> x1 = getX(null);
    	X<String> x2 = getX(null); 
    }	
}
both the message sends have the inferredReturnType bit set to true.

Comment 6 Srikanth Sankaran 2011-07-07 01:34:19 EDT

(In reply to comment #4)

> Now
> org.eclipse.jdt.internal.compiler.ast.AllocationExpression.inferredReturnType
> is set if the expected type was used to drive the inference.

Ayush, this bit must be cleared unconditionally if we implement any algorithms
that attempt to use <> inference when it was not actually present in the source.

Comment 7 Markus Keller 2011-07-07 10:10:00 EDT

(In reply to comment #5)
> FYI - This last part of the comment isn't correct. For both c & d, we proceed
> to 15.12.2.8.

Fair enough, that's fine for me. Changing the DefaultBindingResolver to use the following solves the problem for me:

	boolean isResolvedTypeInferredFromExpectedType(ClassInstanceCreation classInstanceCreation) {
		Object oldNode = this.newAstToOldAst.get(classInstanceCreation);
		if (oldNode instanceof AllocationExpression) {
			AllocationExpression allocationExpression = (AllocationExpression) oldNode;
			return allocationExpression.inferredReturnType;
		}
		return false;
	}

Comment 8 Olivier Thomann 2011-07-07 10:14:27 EDT

I'll prepare the final patch.

Comment 9 Olivier Thomann 2011-07-07 13:37:30 EDT

Created attachment 199279 [details]
Proposed fix

I'll add regression tests as well based on Markus' example.

Comment 10 Olivier Thomann 2011-07-07 13:47:20 EDT

Created attachment 199281 [details]
Proposed fix + regression test

Same patch with a regression test.

Comment 11 Olivier Thomann 2011-07-07 13:47:39 EDT

Released in BETA_JAVA7 branch.

Comment 12 Ayushman Jain 2011-07-14 02:53:26 EDT

(In reply to comment #11)
> Released in BETA_JAVA7 branch.

Olivier, can you also update the build notes "whats new" with the APIs? Thanks!

Comment 13 Olivier Thomann 2011-07-14 10:51:44 EDT

(In reply to comment #12)
> Olivier, can you also update the build notes "whats new" with the APIs? Thanks!
I will. I will also check all new APIs since 3.7 and make sure they appear in the what's new section.

Comment 14 Raksha Vasisht 2011-07-20 03:58:26 EDT

Verified.