Bug 287648 - [1.8][compiler] Add support for JSR 308 - Type Annotations Specification
Summary: [1.8][compiler] Add support for JSR 308 - Type Annotations Specification
Status: RESOLVED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.6   Edit
Hardware: All All
: P3 enhancement with 1 vote (vote)
Target Milestone: BETA J8   Edit
Assignee: Jay Arthanareeswaran CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 292364 380106 383596 383600 383884 383908 383913 383950 384457 385111 385137 385293 385374 388936 390784 390882 390891 391108 391196 391201 391314 391315 391331 391464 391500 391847 392119 394355 394356 395886 402618 403457 403581 409235 413613 415308 415439 415821 415911 418096 418347 419331 419412
Blocks: 380190
  Show dependency tree
 
Reported: 2009-08-26 03:18 EDT by Srikanth Sankaran CLA
Modified: 2013-10-19 00:32 EDT (History)
13 users (show)

See Also:


Attachments
Grammar changes for JSR308 v0.1 (17.35 KB, patch)
2009-08-26 06:08 EDT, Srikanth Sankaran CLA
no flags Details | Diff
Grammar + Parser changes for JSR308 v0.2 (144.48 KB, patch)
2009-08-27 08:03 EDT, Srikanth Sankaran CLA
no flags Details | Diff
Grammar + Parser changes v0.3 (145.13 KB, patch)
2009-08-27 21:44 EDT, Srikanth Sankaran CLA
no flags Details | Diff
Grammar + Parser + Misc changes v0.4 (146.05 KB, patch)
2009-09-01 09:48 EDT, Srikanth Sankaran CLA
no flags Details | Diff
Grammar + parser + misc changes v0.5 (146.57 KB, patch)
2009-09-03 10:24 EDT, Srikanth Sankaran CLA
no flags Details | Diff
Grammar + Parser + Misc changes v0.6 (176.65 KB, patch)
2009-09-07 09:06 EDT, Srikanth Sankaran CLA
no flags Details | Diff
Grammar + parser changes v0.7 (282.90 KB, patch)
2009-09-24 13:43 EDT, Srikanth Sankaran CLA
no flags Details | Diff
Grammar + Parser changes v0.75 (291.61 KB, patch)
2009-09-25 05:17 EDT, Srikanth Sankaran CLA
no flags Details | Diff
Grammar + Parser changes v0.8 (271.12 KB, patch)
2009-10-14 04:57 EDT, Srikanth Sankaran CLA
no flags Details | Diff
Grammar + Parser changes v0.9 (346.85 KB, patch)
2009-10-20 08:53 EDT, Srikanth Sankaran CLA
no flags Details | Diff
A patch with tests for checking new syntax (84.92 KB, patch)
2009-10-26 03:27 EDT, Srikanth Sankaran CLA
no flags Details | Diff
Patch containing original implementation done in 2010 (2011/01/20) (1.01 MB, patch)
2011-01-20 16:57 EST, Olivier Thomann CLA
no flags Details | Diff
Up to date patch (885.08 KB, patch)
2012-06-14 04:15 EDT, Srikanth Sankaran CLA
no flags Details | Diff
Updated patch (876.53 KB, patch)
2012-06-19 22:35 EDT, Jay Arthanareeswaran CLA
no flags Details | Diff
Patch merged with recent changes in BETA_JAVA8 (876.54 KB, patch)
2012-06-21 03:20 EDT, Jay Arthanareeswaran CLA
no flags Details | Diff
Patch updated as of Jun 2012. (878.77 KB, patch)
2012-06-21 08:31 EDT, Srikanth Sankaran CLA
no flags Details | Diff
First batch of files for review & release (413.98 KB, patch)
2012-06-24 09:47 EDT, Srikanth Sankaran CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Srikanth Sankaran CLA 2009-08-26 03:18:34 EDT
This enhancement request is a placeholder to document,
track progress of, discuss, disseminate information
on the task to incorporate support for jsr 308.

The JSR 308 webpage is http://groups.csail.mit.edu/pag/jsr308/

http://types.cs.washington.edu/jsr308/specification/java-annotation-design.pdf
contains the latest specification.

Section 5 in this document provides the "detailed grammar changes" and
under this head lists ~20 products that will need to undergo change.

This grammar delta is with respect to the grammar outlined in chapter 18
of JLS3.

A few key points here:

    - Even with respect to JLS3, this delta is not comprehensive, so we
can't go just by this section. As one example, see that the delta misses
out annotations on types in the throws specification for a method.
(Reported)

    - The JLS3 grammar is substantially different (while being
equivalent) from what is used by eclipse. To meet LALR(1) criteria,
to perhaps aid with error recovery etc, the grammar used by eclipse
has undergone significant transformations.

    - The very first production in section 5, namely, 

      Type:
          [ Annotations] UnannType

      works nicely for the JLS3 grammar to ensure that any use of a Type
      could be annotated, but is misleadingly simple for the eclipse grammer.
      In contexts where use of Type is more permissive than what is legal,
      Eclispe grammar directly uses other nonterminals from the
      Types subgrammar such as ReferenceType, Name, ClassOrInterfaceType
      etc. Some of these non-terminals are used in contexts where it would
      be illegal to annotate them, so we can't simply "wrap" these into
      new productions with optional preceding annotations as the delta
      spec does with Type.
Comment 1 Srikanth Sankaran CLA 2009-08-26 06:08:30 EDT
Created attachment 145655 [details]
Grammar changes for JSR308 v0.1


    - Here is the result of the very first pass of changes to
      java.g. I think this captures all the annotation related
      changes to productions. HOWEVER, I didn't add the
      putcase, readablename, compliance, recovery related macros
      to most of the new productions. So there is more work here.

    - All the shift/reduce and reduce/reduce conflicts are fixed and
      this modified grammar is declared to be LALR(1) by jikespg

    - I had to tweak a couple of productions relating to error recovery
      to resolve the conflicts. This will surely have side effects that
      need to be understood and addressed.

    - There is very little testing that this grammar has seen so far
      and there are many failures (~70%) in the initial 1000 tests
      (after which I aborted the run). A large %age of these failures
      seem to be at the same spot (AIOOB)

    - I am posting this patch just so folks can get a feel for the kind,
      scope and extent of changes that have gone in. Otherwise, this
      grammar is NOT usable yet.

    - Only minimal attempt has been made to "optimize"/"minimize"
      the changes. On many occasions where optional annotations can
      occur with jsr308, I have had to duplicate a production once
      with and once without - to avoid conflicts.
Comment 2 Olivier Thomann CLA 2009-08-26 12:19:54 EDT
AIOOB might come from missing semantic actions for the new rules.
Comment 3 Srikanth Sankaran CLA 2009-08-27 07:55:46 EDT
(In reply to comment #2)
> AIOOB might come from missing semantic actions for the new rules.

Right. Also to stay LALR(1), I had to completely rewrite the rules
for formal parameters by inlining the Types subgrammar. This also
required "bubbling up" the parser callouts so that the consistency
of the various stacks are maintained. 

Comment 4 Srikanth Sankaran CLA 2009-08-27 08:03:02 EDT
Created attachment 145779 [details]
Grammar + Parser changes for JSR308 v0.2

Improved grammar file. Also included is Parser.java changes.
With these changes, majority of the tests pass (only 2175
failures out of 41000+ tests) - A large %age remaining failures
can be traced to two known issues which are being worked on.
Comment 5 Srikanth Sankaran CLA 2009-08-27 08:09:11 EDT
The parser regenerated from the upgraded grammar is able to
parse the program below without any problems. HOWEVER, the
extra optional annotations whereever they occur in the newly
sanctioned places is simply tossed away at this point - i.e the
concerned Type will be seen to be missing the annotations
by the subsequent phases.


------------------------------------8<-----------------------------------
package pack;

import java.io.Serializable;

@interface ReadOnly {}
@interface NonNull {}
@interface Unicode {}
@interface NonEmpty {}
@interface Julian {}
@interface Persistent {}
@interface Binary {}
@interface Even {}
@interface Ascii {}
@interface Prime {}
@interface Composite {}
@interface Fatal {}

public class Clazz extends @Julian java.util.Date 
	implements @Persistent @NonNull Serializable
{

	Clazz () @Persistent throws @Fatal Exception {}
	
	@Even @Composite
	int valueOf () @NonNull [] [] @NonNull [] @NonNull {
		return new @Prime int [0][0][0];
	}
	public static void main(String @NonNull [] args) {
		

	}
	
	public char @Ascii [] foo(Object @NonEmpty [] @ReadOnly @NonNull[] @Even ...objects ) {
		return (char @Ascii []) new @Ascii char [5];
	}

}


Comment 6 Srikanth Sankaran CLA 2009-08-27 21:44:52 EDT
Created attachment 145880 [details]
Grammar + Parser changes v0.3


Improved grammar + parser files. Now there are only 204 failures
when running all jdt-core tests. These failures appear to be mostly
in formatting, error recovery etc.
Comment 7 Srikanth Sankaran CLA 2009-09-01 09:48:52 EDT
Created attachment 146186 [details]
Grammar + Parser + Misc changes v0.4

Improved patch that fixes all the failures
in formatter tests. 179 more test failures
to investigate...
Comment 8 Srikanth Sankaran CLA 2009-09-03 10:24:52 EDT
Created attachment 146401 [details]
Grammar + parser + misc changes v0.5

Improved patch. Still 128 more "failures" left to
investigate. Most of these have to do with syntax
recovery and may not be real failures at all and
instead may simply represent a vaid alternate recovery
suggestion that is triggered by the changed state
machine resulting from the new grammar.

Under investigation.
Comment 9 Srikanth Sankaran CLA 2009-09-07 09:06:54 EDT
Created attachment 146602 [details]
Grammar + Parser + Misc changes v0.6


Improved patch that (a) fixes one issue with syntax error recovery
and (b) remasters the expected syntax error messages in the case
of a few tests where the message has changed, but is either better
than the original or is as good (or as bad) as the earlier message.

37 more failures still need investigation. (A test that fails across
1.3, 1.4, 1.5, 1.6 is counted as 4 failures, so real failures should
be < 10)
Comment 10 Srikanth Sankaran CLA 2009-09-24 13:43:06 EDT
Created attachment 148041 [details]
Grammar + parser changes v0.7


    The current patch includes parse tree construction changes
to process and attach type annotations to suitable nodes.

    You will have to apply the patch to HEAD, take the grammar
file and feed it to the parser generator and then update parser
files (since the current patch does not include the generated
binary files)

    - This is work in progress and has seen only basic
testing of the new features and so in all probabilities
has bugs of different denominations.

    - The focus of the work has been org.eclipse.jdt.internal.compiler.parser.Parser. Its subclasses may
need more work to handle the new type annotations (in particular
CompletionParser)

    - Known problems : Type annotations are not accepted in 

      @NOTHERE Type.class

      Per the grammar changes given in the JSR document type annotations
      are not legal there, but javac seems to accept it.

    - ArrayCreation and ClassInstanceCreation productions - the annotations
      on dimensions may not be processed correctly.

    - Places where extended dimensions are possible (copyDims users)
      annotations on dimensions may not be fully there. Formal parameter
      handling works correctly, similar changes have to be made in other
      places (method header, local variable, field declarations ...)

    - ~150 tests fail in JDT/Core test suite at this point, these
are mostly in syntax recovery and code completion.

    - At this point, type annotations are attached to
      (as per the case)
              
               TypeReference.annotations,
               TypeParameter.annotations,
               AbstractMethodDeclaration.receiverAnnotations
               ArrayTypeReference.annotationOnDimensions
               ArrayQualifiedTypeReference.annotationsOnDimensions

      Need to see if there is a better home for these.

    - Most likely, we may have to add annotations to NameReference too.
    
    - When JSR308 annotations occur in places where Java 5 annotations
      can occur, they are treated as java 5 annotations from a parse
      tree construction stand point right now.

    - I am not happy with the number of methods for which I had to
      introduce a new parameter 'boolean hasTypeAnnotations' so
      the parser action methods don't over consume a (non-existing)
      type annotation. It would have been nice to be able to push
      a zero in the type annotation length stack to say there is
      no annotation available, but this proved to be very hard in
      many many situations.
     
      Introducing a production like
      PushZeroTypeAnnotation ::= $empty
      leads to a shift/reduce conflicts in many many places and these
      new annotations are pervasive since the optional annotations
      can occur in so many different places. So I had to let the
      method know whether they should expect an annotated type or
      not.

      Need to see if there is a better way.
Comment 11 Srikanth Sankaran CLA 2009-09-24 13:46:25 EDT
Here is a silly contrived program that I used to test. Inside the debugger,
break after the parse tree is constructed (including the method bodies)
and then you can traverse the parse tree (in the debugger) to see the
various annotations.

import java.util.HashMap;
import java.util.Map;

public class Clazz <@A M extends @AA String, @AAA N extends @AAAA Comparable> extends @AAAAA Object implements @AAAAAA Comparable <@AAAAAA Object> {

	Clazz() @A {    
	}

	int @A[][]@AA[] f;
	
	public void main(String @A[] @AA ... args) @AAA throws @AAAA Exception {
		
		HashMap<@A String, @AA String> b1;
		
		int b = (@A int) 10;
		
		char @A[]@AA[] ch = (@AAA char @AAAA[]@AAAA[])(@AAAAA char @AAAAAA[]@AAAAAAA[]) null;

		int [] i = new @A int @AA[10];


		Integer w = new X<@A String, @AA Integer>().get(new @AAA Integer(12));
		throw new @A Exception("test");
		boolean c = null instanceof @A String;
	} 
	public <@A X, @AA Y> void foo(X x, Y @AAA... y) {  
		
	}

	void foo(Map<? super @A Object, ? extends @B String> m){}
	public int compareTo(Object arg0) {
		return 0;
	}

}

class X<@A K, @AA T extends @AAA Object & @AAAA Comparable<? super @AAAAAA T>> {

	public Integer get(Integer integer) {
		return null;
	}
}
Comment 12 Srikanth Sankaran CLA 2009-09-25 05:17:45 EDT
Created attachment 148087 [details]
Grammar + Parser changes v0.75


Revised patch with the following changes:

    - ASTNode print facility now prints type annotations. So it is
lot easier to verify the parse tree construction in a debugger. Just
print the parsed unit or one of its types.

    - Fixed handling of annotations on extended dimensions (new array
creation annotations are still dropped)

    - Synchronized with HEAD to eliminate failures introduced by unrelated
work

    I could verify that on the following program, all type annotations are
correctly printed back (except in "int [] i = new @Q int @R[10];"


----------------------------------------8<-------------------------------
import java.util.HashMap;
import java.util.Map;   

public class Clazz <@A M extends @B String, @C N extends @D Comparable> extends @E Object implements @F Comparable <@G Object> {

	Clazz(char [] ...args) @H {    
	}

	int @I[] f @J[], g = null, h[], i@K[];
	int @L[][]@M[] f2; 
	
	Clazz (int @N [] @O... a @P[]) @Q {}
	int @R[]@S[] aa() {}
	
	int @T[]@U[]@V[] a () @W[]@X[]@Y[] @Z { return null; }
	
	public void main(String @A[] @B ... args) @C throws @D Exception {
		
		HashMap<@E String, @F String> b1;
		
		int b = (@G int) 10;
		
		char @H[]@I[] ch = (@K char @L[]@M[])(@N char @O[]@P[]) null;

		int [] i = new @Q int @R[10];


		Integer w = new X<@S String, @T Integer>().get(new @U Integer(12));
		throw new @V Exception("test");
		boolean c = null instanceof @W String;
	} 
	public <@X X, @Y Y> void foo(X x, Y @Z... y) {  
		
	}

	void foo(Map<? super @A Object, ? extends @B String> m){}
	public int compareTo(Object arg0) {
		return 0;
	}

}

class X<@C K, @D T extends @E Object & @F Comparable<? super @G T>> {

	public Integer get(Integer integer) {
		return null;
	}
}
Comment 13 Srikanth Sankaran CLA 2009-10-14 04:57:05 EDT
Created attachment 149501 [details]
Grammar + Parser changes v0.8


- The grammar has been significantly rewritten so that
  the type annotation stacks are managed better. When
  type annotations are absent in a place where they
  are permissible, type annotations length stack will
  now contain 0. Thus we don't have to explitly pass a
  parameter to the parser action routine to let it know
  whether it can expect annotations in the stack or not.
  This also means the consume* methods could simply call
  getTypeReference(int dimension) as they currently do
  instead of the code pattern :

   hasTypeAnnotations ? getTypeReference() : 
             getUnannotatedTypeReference() 

- Most failures in RunAllJDTCoreTests have been analyzed and
  fixed. Only 3 more tests are failing now. These too are
  not likely to be real issues. The error recovery path has
  likely changed resulting in changed parse trees after
  recovery.

- Synched with HEAD.

- The parser is able to handle all the sample & test programs
  and snippets in the jsr308 distribution.

Next steps: 

- Still no automated tests - need to take this up on priority
  basis now.

- Type annotations are not accepted before the type in the class
  literal expressions. Changing the grammar to include optional
  annotations here send jikes parser generator into severe fits.
  parser generator.

- The specialised subclass parsers may still drop the new 
  annotations (IndexingParser, AssistParser, CompletionParser,
  MatchLocatorParser ...)

- ArrayCreation and ClassInstanceCreation productions - the annotations
  on dimensions may not be processed correctly.
Comment 14 Olivier Thomann CLA 2009-10-14 08:50:13 EDT
Nice, Srikanth.
I think for the next step the class literal needs to be handled. With this addition, all required annotations will be supported.
You might need to make the grammar more permissive in order to be LALR(1) again.
Comment 15 Srikanth Sankaran CLA 2009-10-15 07:27:46 EDT
(In reply to comment #14)
> Nice, Srikanth.
> I think for the next step the class literal needs to be handled. With this
> addition, all required annotations will be supported.
> You might need to make the grammar more permissive in order to be LALR(1)
> again.

Hi Olivier, will get to this soon, I am in the middle of putting together
automated tests for testing the parser with the new syntax and once it is
basically up and running, I can put that effort in the back burner and focus
on setting the class liretal issue straight.

(In reply to comment #10)

>     - ArrayCreation and ClassInstanceCreation productions - the annotations
>       on dimensions may not be processed correctly.

I have fixed these in my workspace. We now capture the relevant type
annotations in ArrayAllocationExpression.annotationsOnDimensions 

>     - Most likely, we may have to add annotations to NameReference too.

    See bug #292364 and the proposed patch. With that fix, we will not
need annotations on NameReferences.

>     - At this point, type annotations are attached to
>       (as per the case)
>                TypeReference.annotations,
>                TypeParameter.annotations,
>                AbstractMethodDeclaration.receiverAnnotations
>                ArrayTypeReference.annotationOnDimensions
>                ArrayQualifiedTypeReference.annotationsOnDimensions

Now also ArrayAllocationExpression.annotationsOnDimensions

>       Need to see if there is a better home for these.

    I am inclined to leave these as they are. I gave some thought to
creating new node types with Annotations leaving the existing classes
alone. This looks like too much of trouble to me (duplication, numerous 
places needing to change, type equivalence issues etc)
 
>     - When JSR308 annotations occur in places where Java 5 annotations
>       can occur, they are treated as java 5 annotations from a parse
>       tree construction stand point right now.

    I think we will also leave this behavior as it is.
Comment 16 Srikanth Sankaran CLA 2009-10-19 10:42:33 EDT
(In reply to comment #15)
> (In reply to comment #14)

> >     - At this point, type annotations are attached to
> >       (as per the case)
> >                TypeReference.annotations,
> >                TypeParameter.annotations,
> >                AbstractMethodDeclaration.receiverAnnotations
> >                ArrayTypeReference.annotationOnDimensions
> >                ArrayQualifiedTypeReference.annotationsOnDimensions
> Now also ArrayAllocationExpression.annotationsOnDimensions
> >       Need to see if there is a better home for these.
>     I am inclined to leave these as they are. I gave some thought to
> creating new node types with Annotations leaving the existing classes
> alone. This looks like too much of trouble to me (duplication, numerous 
> places needing to change, type equivalence issues etc)

We need to freeze on this, so as to stabilize on the compiler AST so
downstream work could be taken up be me/someone else.

It appears to me that in the past, folks have taken pains to avoid
type equivalence issues - (e.g why would ParameterizedSingleTypeReference
be a subclass of ArrayTypeReference instead of SingleTypeReference ?)

Adding annotations as a field with it being to set to null (and not to
Annotation[0]) seems to be the simplest way to go.  

Olivier, Do you agree ? 

> >     - When JSR308 annotations occur in places where Java 5 annotations
> >       can occur, they are treated as java 5 annotations from a parse
> >       tree construction stand point right now.
>     I think we will also leave this behavior as it is.

Again, do you agree ? Our ability to generate Runtime[In]VisibleTypeAnnotations is not compromised by this decision in anyway. This would also simplify 
DOM/AST creation (since nodes are NOT moved around during type resolution)
Comment 17 Olivier Thomann CLA 2009-10-19 10:50:32 EDT
(In reply to comment #16)
> Adding annotations as a field with it being to set to null (and not to
> Annotation[0]) seems to be the simplest way to go.  
> 
> Olivier, Do you agree ? 
Yes, adding new nodes might be much more difficult to handle. Let keep the existing nodes for now and continue. If space becomes an issue in the future, we might reconsider, but right now we should keep the code as simple as possible.

> Again, do you agree ? Our ability to generate Runtime[In]VisibleTypeAnnotations
> is not compromised by this decision in anyway. This would also simplify 
> DOM/AST creation (since nodes are NOT moved around during type resolution)
+1. This has no effect on generating the right annotations in the .class file.
Comment 18 Srikanth Sankaran CLA 2009-10-20 08:53:37 EDT
Created attachment 149970 [details]
Grammar + Parser changes v0.9


    What is new in this patch ? 

	- Type annotations are handled properly when used in
          array creation/class instance creation expressions.

	- Several automated tests have been created in
          org.eclipse.jdt.core.tests.compiler.parser.TypeAnnotationSyntaxTest

	- Need for annotations to be added to NameReferences has been
          obviated with the fix for bug #292364. So we can freeze the
          compiler AST node changes. Type annotations hang off of

	        TypeReference.annotations,
                TypeParameter.annotations,
                AbstractMethodDeclaration.receiverAnnotations
                ArrayTypeReference.annotationOnDimensions
                ArrayQualifiedTypeReference.annotationsOnDimensions
		ArrayAllocationExpression.annotationsOnDimensions
		
	- A bug in dealing with annotations in parametrized cast 
          expressions got fixed.

	- Type annotation support extended to subclass parsers (AssistParser
          CompletionParser, SelectionParser, DocumentElementParser,
          CommentRecorderParser, SourceElementParser, IndexingParser)
	  TypeAnnotationSyntaxTest.java also tests these parsers and
          validates the parse tree.

	Known Issues
        ------------

	- Annotations are not yet permitted on class literals.
        - 3 remaining failures in RunAllJDTCoreTests need to be analyzed.


	Next Steps
        ----------

	- Add class literal support.
	- Cleanup tests failures, verify test suite changes.
	- Extended testing (axxon/performance, UI tests, compliance tests ...)
	- Code review
	- Commit.

	Olivier, I believe the parser is stable enough for you to start
        work on type resolution/code generation. Let me know if you see
        any problems -- Thanks.

	I am planning to release a patch containing "preparatory" changes
        later this week (withholding all grammar changes for a latter pass
        around mid M4 after testing/code review.)
Comment 19 Srikanth Sankaran CLA 2009-10-26 03:27:42 EDT
Created attachment 150486 [details]
A patch with tests for checking new syntax

Olivier, 

As you change the grammar to fix the class literal issue, you may find
these tests useful. I am in the process of overhauling these tests
(the biggest problem being, there is no easy way to grab these programs
and feed it to javac 7 to verify that they are syntactically correct -
since these will produce several other non-syntax errors. I am changing
these so that they have no non-syntax errors, so it will be easy to
discern when javac rejects something)

Please also note, while there are several tests, the coverage should/could
be improved quite a bit.
Comment 20 Olivier Thomann CLA 2010-11-17 11:37:42 EST
jsr 308 has been moved to JDK8
Comment 21 Olivier Thomann CLA 2011-01-20 16:57:33 EST
Created attachment 187248 [details]
Patch containing original implementation done in 2010 (2011/01/20)

This doesn't include the parser binary files that need to be regenerated.
Comment 22 Srikanth Sankaran CLA 2012-06-14 04:15:52 EDT
Created attachment 217341 [details]
Up to date patch

Many thanks to Satyam & Jay for dusting up the old patch, cleaning up the 
bit rot and merging all the changes in head/master and bringing it up to 
date.

This patch fails 152 tests across 1.3 - 1.8 modes, there are 0 run time
errors/exceptions.

Here are the next steps:

	- Copyright on all files should say 2012, not 2010 or 2011.
	- We need to carefully verify that all changes made to java.g 
          file in https://bugs.eclipse.org/bugs/attachment.cgi?id=187248 
          have been correctly picked up. This is very crucial to avoid 
          headaches in future.
	- We also need to eyeball the patch above to make sure all the 
          modified files are picked up. i.e no changes are dropped.
          For example, we seem to have dropped the changes made to    
          CompilerInvocationTests.java in this patch :-( May be there
          are others too.
	- Incorporate the changes to CompilerInvocationTests.java that are  
          *relevant* from earlier patch. These are for the 5 new IProblem
          constants for 308 work. 
	- Discard the changes to ArrayTest.java - this is turning the clock
          back incorrectly.
	- Discard the changes to ScannerTest.java - this is some unrelated 
          work that has gotten mixed up.
	- Discard the changes to TryStatement.java - this is stale and 
          incorrect code
	- Discard the changes to PublicScanner.java - this is some unrelated 
          work that has gotten mixed up.
	- IProblem.java: Please get rid of first two noise diffs, change all 
          @since 3.8 to @since 4.3 for the new problem constants.
	- Compiler/Problem/message.properties:
		- please get rid of first noise diff
		- also second diff which is unrelated to current work
		- third diff, syntax error message mentions 1.7 should be
                  changed to 1.8
	- Every reference to JDK1_7 in the patch/change set should be 
          scrutinized - most would need to be JDK1_8
	- ASTNode.java:
		- please get rid of first noise diff.
		- second diff needs to discarded too since we are not using 
                  bit 30.
	- Let us release these files right away into BETA_JAVA8 as these are 
          general fixes not connected to 308 (and remove them from the patch)
		- AbstractVariableDeclaration.java:
		- AnnotationMethodDeclaration.java
		- Argument.java
		- Initializer.java
	- Initializer.java is missing JCP/JSR disclaimer. Needs to be inserted.
	- ClassFileConstants.java - Get rid of first noise diff.
	- ReadableName.props - restore copyright.
	- IAttributeNamesConstants.java: Change @since 3.8 to @since 4.3
	- In general, search for all @since 3.8 and change them to @4.3 for all 
          308 related work (only)
	- Make a pass over all to eliminate all noise diffs (white spaces, blank
          lines, etc)
	- Fix extra warnings that show up in the workspace build compared to
          master.
	- Remaster org.eclipse.jdt.core.tests.compiler.regression.JavadocTestMixed.test033() i.e 
          make the new actual output the expected output as it is equivalent 
          and correct.
	- Undo the change we have made to org.eclipse.jdt.core.tests.compiler.parser.ComplianceDiagnoseTest.test0049()
	  The actual output matches what is expected on master, but not what 
          is expected on JSR308 work. This is not influenced by JSR308 work
          anyway, so if we toss out the change made to this test, it will 
          start passing.
	
With these changes I expect to see about 120 failures (across 1.3 - 1.8 modes)
and 0 errors. There is some messed up interaction between TWR + 308 and
@SafeVarargs + 308. If this is addressed, I expect the failures to drop 
further down.
Comment 23 Markus Keller CLA 2012-06-14 05:39:54 EDT
> Change @since 3.8 to @since 4.3

Nope, change the bundle version and the @since tags to 3.9.

4.3 is the version number of the SDK feature in the Kepler release train, but this is not connected to the bundle versions any more. According to http://wiki.eclipse.org/index.php/Version_Numbering , the major version is only to be increased for breaking changes, which I don't expect here.
Comment 24 Srikanth Sankaran CLA 2012-06-14 05:54:59 EDT
(In reply to comment #23)
> > Change @since 3.8 to @since 4.3
> 
> Nope, change the bundle version and the @since tags to 3.9.

Thanks for the clarification, Markus.
Comment 25 Srikanth Sankaran CLA 2012-06-19 05:06:25 EDT
(In reply to comment #22)

>     - Let us release these files right away into BETA_JAVA8 as these are 
>           general fixes not connected to 308 (and remove them from the patch)
>         - AbstractVariableDeclaration.java:
>         - AnnotationMethodDeclaration.java
>         - Argument.java
>         - Initializer.java

Stand corrected, these files are connected to JSR308 work and have dependencies
on other parts. So this particular suggestion is invalid - sorry.
Comment 26 Jay Arthanareeswaran CLA 2012-06-19 22:35:54 EDT
Created attachment 217581 [details]
Updated patch

This patch has all the changes suggested in comment #22 except these:

- Every reference to JDK1_7 in the patch/change set should be 
          scrutinized - most would need to be JDK1_8
- Fix extra warnings that show up in the workspace build compared to
          master.

In addition this contains some code that was missed out in the previous patch, which also takes care of the point about JavadocTestMixed.test033()

Exactly 120 tests are failing with this patch.
Comment 27 Jay Arthanareeswaran CLA 2012-06-21 03:20:20 EDT
Created attachment 217668 [details]
Patch merged with recent changes in BETA_JAVA8

Also updated couple of left out @Since tags and removed reference to deprecated JLS3 constant.
Comment 28 Srikanth Sankaran CLA 2012-06-21 08:31:39 EDT
Created attachment 217685 [details]
Patch updated as of Jun 2012.


     - Fixed a bug in handling of SafeVarargs annotation.
     - Fixed some grammar rules incorrectly commented out.
     - Now we have 75 failures to deal with.
Comment 29 Srikanth Sankaran CLA 2012-06-22 08:31:29 EDT
A few grammar changes that have taken place since the time we halted
work are yet to be incorporated. These are:

    - New syntax for receiver annotations.
    - Type annotations have been declared illegal on class literals.
    - Syntax for annotations on disjunctive types (not fully specified ?)
    - Inner class annotations: @OuterAnnot Outer. @InnerAnnot Inner syntax.
Comment 30 Srikanth Sankaran CLA 2012-06-22 09:53:26 EDT
(In reply to comment #26)
> Created attachment 217581 [details]
> Updated patch
> 
> This patch has all the changes suggested in comment #22 except these:

(In reply to comment #22)

>     - Discard the changes to ArrayTest.java - this is turning the clock
>           back incorrectly.


>     - Discard the changes to ScannerTest.java - this is some unrelated 
>           work that has gotten mixed up.

>     - Compiler/Problem/message.properties:
>         - please get rid of first noise diff

These are still there, Perhaps a few others too, I'll double check
everything.
Comment 31 Srikanth Sankaran CLA 2012-06-24 08:32:51 EDT
(In reply to comment #22)

>     - ASTNode.java:

>         - second diff needs to discarded too since we are not using 
>                   bit 30.

This diff is still there in the patch.

I also noticed that some changes from Olivier's patch are missing in
what we are working with - the last chunk of code in AbstractMethodDeclaration.java for example.
Comment 32 Srikanth Sankaran CLA 2012-06-24 09:47:27 EDT
Created attachment 217786 [details]
First batch of files for review & release


   This is a shortlisted set of 79 files that are undergoing review as well
as comparison with the old patch to make sure no changes are inadvertantly
dropped.

   This consists of grammar + parser + AST construction + various infrastructure
changes.

   This is expected to be completed and released by end of Jun.
Comment 33 Srikanth Sankaran CLA 2012-06-24 09:51:23 EDT
(In reply to comment #32)
> Created attachment 217786 [details]
> First batch of files for review & release

This patch/batch also:

    - Implements the new syntax for receiver annotations.
    - Withdraws the old syntax for receiver annotations.
    - Disallows type annotations on class literals.

Nested class annotations & disjunctive types annotations are not 
implemented yet. The spec is unclear on the latter - clarifications
have been sought.
Comment 34 Jay Arthanareeswaran CLA 2012-06-26 13:39:21 EDT
Changes made to ASTConverterTestAST3_2.java as part of commit  1c79c3bc7315340e05be1209e9e7d824c9142fe5 seem to have been lost. The changes made to other files in that commit made it to the patch though. This is resulting in 3 failures.
Comment 35 Srikanth Sankaran CLA 2012-06-26 18:27:28 EDT
First batch of 80 files implementing grammar/parser, AST construction, symbol
resolution and type resolution support for JSR 308 released via:

http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?h=BETA_JAVA8&id=831d96bc5763622ed503192c35bfd6688abedd96.

One pass of review has been completed, one more pass is planned. 

A patch with test changes will be posted shortly.

All these files have been checked against the original implementation to
make sure that no changes have been lost: In this process, this checkin
fixes several merge errors in 

ParameterizedSingleTypeReference, ParameterizedQualifiedTypeReference
ASTNode, AbstractMethodDeclaration, Argument, LocalDeclaration, 
MethodDeclaration and CompletionParser

(Given this was a mega effort merging 110+ files (excluding generated files)
and bringing them up to date after 18 months, these merge errors are fewer in
number than can be expected. Thanks for a superlative effort Satyam & Jay !)

Remaining changes dealing with code generation is expected to be released
by Aug 2012.
Comment 36 Srikanth Sankaran CLA 2012-06-27 02:44:30 EDT
(In reply to comment #35)

> A patch with test changes will be posted shortly.

Test changes released via
http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?h=BETA_JAVA8&id=9cee1a1ab489dcab686c4840c5cbf41450bb5e75.

This commit disables a few tests that are failing at the moment (3 new
tests and 37 old ones) I'll raise follow up defects to address these.

The present bug will be retained as the main topic bug for JSR308 project,
but further work will happen via other bugs that will be spawned off at the
level of unit tasks/sub-projects and will be linked to here as dependencies.
Comment 37 Srikanth Sankaran CLA 2013-10-19 00:32:45 EDT
Resolving as all work items are complete for this project.