Comment 1 Stephan Herrmann 2013-07-31 17:45:03 EDT

Looking at the AST after parsing reveals a bit of what's going on here:

import java.lang.annotation.*;
@Target(ElementType.TYPE_USE) @interface NonNull {
  class value {
    value() {
      super();
    }
  }
  public class X extends @NonNull() Object {
    public static @NonNull() int i = 0;
    public X() {
      super();
    }
    <clinit>() {
    }
  }
}

This happens partly due to the syntax error at int[].class, but it's strange why we 
end up with duplicate @NonNull()?

Actually, that directly leads to the root cause: the AST is not well-formed,
two type references (the type's superclass and the field's type) share the same
instance of NormalAnnotation!!

From here it's easy to see that the recipient of the annotation is a field
and is not a field at the same time, depending on how you look at the AST.

My guess: during error recovery someone is not properly consuming the type
annotation from its stack.

I vaguely remember we already had a similar situation.

Comment 2 Stephan Herrmann 2013-07-31 18:16:57 EDT

Created attachment 233998 [details]
sketch of a possible fix

No, not the parser's stack is corrupt, but the reference to the annotation
is replicated via
- RecoveredType#pendingAnnotations
- SingleTypeReference#annotations

At the point we regularly consume the annotation into the superclass reference
we forgot that we previously stashed it into the RecoveredType.

Resetting that stash at the real consumption resolves the immediate problem,
but raises these questions:
- should this fix apply to other currentElements besides RecoveredType?
- do more locations in the parser exist that need a similar fix?

Looking for Recovered*#pendingAnnotations gives 5 matches.

Just within Parser I see 8 locations decrementing #typeAnnotationPtr, which all
are candidates for creating illegal sharing.

Comment 3 Stephan Herrmann 2013-08-04 06:35:48 EDT

This looks similar:

Given a CU with syntax errors and this method:

public Object[] @NonNull [] foo() {
  return null;
}

The MarkerAnnotation "@NonNull" will end up both 
- as a declaration annotation in MethodDeclaration#annotations
- as a type annotation of the method's return type 
  (ArrayTypeReference#annotationsOnDimensions)

In my workspace I added inside ASTNode#resolveAnnotations()
(FYI, these lines are needed for bug 392384):

	final Binding annotationRecipient = annotation.recipient;
	if (annotationRecipient != null && recipient != null) {
		switch (recipient.kind()) {
// ...
// this section is new:
		case Binding.TYPE_USE :
			TypeUseBinding typeUse = (TypeUseBinding) recipient;
			typeUse.tagBits = ((TypeUseBinding) annotationRecipient).tagBits;
//

With the above input I get CCE because the receiver of a TYPE_USE annotation is not
a TypeUseBinding but a MethodBinding.

This illegal sharing of AST nodes only happens during syntax recovery.

Comment 4 Jay Arthanareeswaran 2013-08-07 00:47:10 EDT

Manoj, would you like to take this up?

Comment 5 Stephan Herrmann 2013-08-31 08:02:31 EDT

(In reply to Stephan Herrmann from comment #3)
> // ...
> // this section is new:
> 		case Binding.TYPE_USE :
> 			TypeUseBinding typeUse = (TypeUseBinding) recipient;
> 			typeUse.tagBits = ((TypeUseBinding) annotationRecipient).tagBits;
> //

This addition was released via commit 427a01e331e57852efc472cca3a8360bd121fda9
but reverted in commit 8a1621e802c664e59aba36b8a87f59ae57902e37.

This means that the mentioned strategy for triggering CCE no longer works,
but I still believe that syntax recovery creates malformed structures.

Comment 6 Stephan Herrmann 2013-09-20 20:12:48 EDT

*** Bug 417662 has been marked as a duplicate of this bug. ***

Comment 7 Srikanth Sankaran 2013-10-09 05:19:30 EDT

I released a temporary fix here: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?h=BETA_JAVA8&id=70293ebfba1aa3d3dce9544e23e757d77487db91

Will leave this open for continued analysis and eventual
proper resolution.

Comment 8 Stephan Herrmann 2013-10-09 08:44:00 EDT

BTW, did you see my sketch in comment 2?
I'm afraid malformed AST will hit us in more situations than just this CCE...

Comment 9 Srikanth Sankaran 2013-10-09 11:55:25 EDT

(In reply to Stephan Herrmann from comment #8)
> BTW, did you see my sketch in comment 2?
> I'm afraid malformed AST will hit us in more situations than just this CCE...

Agreed, that is why this is still left open. ATM, I don't have the time for
due diligence on this task - hence the first aid.

Comment 10 Srikanth Sankaran 2013-11-25 12:21:16 EST

Cumulative fix for bug 414038 and bug 420320 delivered via: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?h=BETA_JAVA8&id=1821b3dbff03a19f0410029f03844aad56801cc1.

Fix was to simply make sure type annotations are mixed up with the
recovery machinery.

If type annotation recovery is necessary, it will have to dealt with
as a separate problem. I don't think it is as important as SE5 annotations
discovery.

Comment 11 Srikanth Sankaran 2013-11-25 12:22:55 EST

(In reply to Srikanth Sankaran from comment #10)

> Fix was to simply make sure type annotations are mixed up with the
                                               are NOT mixed up with the

Comment 12 Stephan Herrmann 2013-11-28 12:06:04 EST

(In reply to Srikanth Sankaran from comment #10)
> If type annotation recovery is necessary, it will have to dealt with
> as a separate problem. [...]

I've filed bug 422784 with a reference to the tentative fix in comment 2.