Bug 418347 - [1.8][compiler] Type annotations from SE7 locations not handled properly during code generation.
Summary: [1.8][compiler] Type annotations from SE7 locations not handled properly duri...
Status: RESOLVED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 4.4   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: BETA J8   Edit
Assignee: Srikanth Sankaran CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 409246 (view as bug list)
Depends on:
Blocks: 287648
  Show dependency tree
 
Reported: 2013-09-30 10:43 EDT by Srikanth Sankaran CLA
Modified: 2013-10-02 06:33 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Srikanth Sankaran CLA 2013-09-30 10:43:55 EDT
BETA_JAVA8:

// ---
class Outer<K>  {
	class Inner<P> {
	}
	public @T(1) Outer<@T(2) String>.@T(3) Inner<@T(4) Integer> @T(5) [] omi @T(6) [];
}
@java.lang.annotation.Target (java.lang.annotation.ElementType.TYPE_USE)
@interface T {
	int value();
}
// --

Given the above, ECJ generates only 5 annotations. There should be 6. The one
with the type path: FIELD, location=[ARRAY, ARRAY, INNER_TYPE] is absconding.

AST and bindings look correct. 

While debugging, you may notice that the extended dimensions would appear ahead 
of the base dimension. This is the right behavior. See bug 418096.
Comment 1 Srikanth Sankaran CLA 2013-09-30 10:46:55 EDT
Andy, please take a look, TIA.

In STB#resolveTypesFor(FieldBuilding) if you break at the call to
Annotation.isTypeUseCompatible and look at field.type you can see that
the binding reflects all the annotations, which should mean AST has all
the annotations too.
Comment 2 Srikanth Sankaran CLA 2013-10-01 07:00:09 EDT
It looks like all 6 annotations make it to class file, but javap shows only
5. I'll investigate whether this a bug in javac or in the way we emit. TypeAnnotationWalker does not like the annotations either.
Comment 3 Srikanth Sankaran CLA 2013-10-01 07:58:14 EDT
*** Bug 409246 has been marked as a duplicate of this bug. ***
Comment 4 Srikanth Sankaran CLA 2013-10-01 12:07:42 EDT
Found a bunch of issues while working on this: 

(1) 
// -- for 
import java.lang.annotation.*;
import static java.lang.annotation.ElementType.*;

@Target({TYPE_USE}) @interface P { }
@Target({TYPE_USE}) @interface O { }
@Target({TYPE_USE}) @interface I { }

public abstract class X<T> {
	class Y<Q> {
	}
	void foo(@P Y<P> p) {}
}

We don't generate the INNER path entry.

(2) In

// --
import java.lang.annotation.*;
import static java.lang.annotation.ElementType.*;

@Target({TYPE_USE}) @interface P { }
@Target({TYPE_USE}) @interface O { }
@Target({TYPE_USE}) @interface I { }

public abstract class X {
	class Y {
		class Z {}
	}
	void foo(@P X.@O Y.@I Z[] p) {}
}

We attribute both @O and @I to Z.

I'll include fixes for these. I also took this opportunity to cleanup
several issues in the relevant portions of code.
Comment 5 Srikanth Sankaran CLA 2013-10-01 12:44:09 EDT
(3) We generate bad attributes for

// --
public abstract class X {
	java.util.List [][] l = new java.util.ArrayList @pkg.NonNull [0] @pkg.NonNull[];     
}
Comment 6 Srikanth Sankaran CLA 2013-10-02 06:33:18 EDT
Fixed via http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?h=BETA_JAVA8&id=e1bb62a6f97249ff77a2f0164d289109644f1d09

Tests added here: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?h=BETA_JAVA8&id=f0f1d42036089cdc450ad38e200db5fad2a1d087

I refactored the class LocationCollector significantly to promote reuse,
take advantage of inheritance etc.

Looking at the classes involved in code generation of type annotations, there
is much more opportunities for clean up and restructuring. I think the 
classes AnnotationContext, AnnotationCollector and LocationCollector should
all be folded into one: AnnotationContext. In particular, making one AST traversal
to discover annotations and another to compute their path entries is wasteful.
This is a project for another day through: https://bugs.eclipse.org/bugs/show_bug.cgi?id=418490


This fixes bug 404719 too.