Bug 192774 - Annotation AST nodes should have unique IAnnotationBindings
Summary: Annotation AST nodes should have unique IAnnotationBindings
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.3.1   Edit
Assignee: Olivier Thomann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 191082
  Show dependency tree
 
Reported: 2007-06-14 19:39 EDT by Walter Harley CLA
Modified: 2008-09-16 09:46 EDT (History)
3 users (show)

See Also:

Attachments
test case (2.50 KB, patch)
2007-06-14 19:40 EDT, Walter Harley CLA 		no flags Details | Diff
Proposed fix + regression test (9.66 KB, patch)
2007-07-09 08:40 EDT, Olivier Thomann CLA 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Walter Harley CLA 2007-06-14 19:39:31 EDT 
The attached test method passes in 3.2.2 but fails in 3.3.  The reason is that in LookupEnvironment we now cache and reuse annotation bindings if the type and values are the same.  The problem with this is that org.eclipse.jdt.core.dom.CompilationUnit.findDeclaringNode() does a reverse lookup of an ASTNode from an IBinding, based on the internal compiler binding contained in the IBinding; so if two ASTNodes share the same internal compiler binding, the wrong ASTNode is returned.  This is causing APT to report incorrect source positions; see bug 191082.
Comment 1 Walter Harley CLA 2007-06-14 19:40:13 EDT 
Created attachment 71383 [details]
test case

Attached test case patch
Comment 2 Olivier Thomann CLA 2007-06-15 11:14:24 EDT 
I think the mistake was to share the compiler annotation bindings.
In fact we cannot do that because of the reverse mapping that we need to be able to do between DOM binding -> declaring node.
If I revert this, I'll need to fix the apt mirror implementation for annotation since the equals is based on the fact that annotation bindings are identical.
I am working on it.
Both changes need to be done at the same time.
Comment 3 Olivier Thomann CLA 2007-07-05 09:46:45 EDT 
Released for 3.3.1.
Regression tests added in org.eclipse.jdt.apt.tests.annotations.mirrortest.MirrorDeclarationTestAnnotationProcessor#testClassDeclaration
org.eclipse.jdt.core.tests.dom.ASTConverter15Test#test0276
Comment 4 Olivier Thomann CLA 2007-07-05 09:54:48 EDT 
Released for 3.4M1.
Same regression tests as for 3.3.1.
Comment 5 Olivier Thomann CLA 2007-07-09 08:40:50 EDT 
Created attachment 73309 [details]
Proposed fix + regression test

Philippe,
+1 for 3.3.1?
Comment 6 Olivier Thomann CLA 2007-07-09 08:42:36 EDT 
Philippe,

Since the fix is in the compiler, I request the +1 from you. I forgot to ask for it before I released it.
Could you please also add your +1 for bug 191082 since this is exactly the same problem?
Comment 7 Frederic Fusier CLA 2007-08-07 06:20:12 EDT 
Code verified for 3.4M1 using build I20070806-1800.
Comment 8 Eric Jodet CLA 2007-09-03 09:44:31 EDT 
Code verified for 3.3.1 using build M20070831-2000