Future of 'type-annotations/type-annotations' forest

[Werner Dietl]

Hi Eric, all,

thanks for all your recent improvements to type annotations!
I have integrated all jdk9 changes into type-annotations without any issue.
I then updated the Checker Framework to the new API and all unit tests
and running checkers on Daikon pass again. So things are looking good
from my side.
(I might still be using "deprecated" APIs though. Do you have an ETA
for when the clean-up patch is going out?)

Thanks for integrating the re-ordering fix with:
http://hg.openjdk.java.net/jdk9/dev/langtools/rev/9c2c24a6785d
I think a reference to my original contribution from:
http://hg.openjdk.java.net/type-annotations/type-annotations/langtools/rev/7e9c983565ac
would have been appropriate.

In type-annotations, I made sure that the BasicAnnoTest at:
test/tools/javac/processing/model/type/BasicAnnoTests.java
compiles correctly.
However, the generics/arrays/exceptions test cases at the end do not work yet.
Is this something you are aware of? I thought one reason for your
refactorings was exactly to make these tests work.

There is one difference between jdk9 and type-annotations that is
required for the Checker Framework to work:
adding the "copy" method to
com/sun/tools/javac/code/TypeAnnotationPosition.java. An alternative
design would be to have factory methods that create new TAPs from an
existing TAP, changing the position.

Another difference that would make compatibility between jdk8 and jkd9
easier is if field "exception_index" remained public to allow direct
access.

A few tests in jdk9 are disabled, even though they pass in type-annotations.
The other differences between jdk9 and type-annotations are small
fixes to javadoc, performance, and imports.
See my message from May 8 for details:
http://mail.openjdk.java.net/pipermail/type-annotations-dev/2014-May/001722.html

Are you working on integrating these changes? How can I help?

Best regards,
cu, WMD.




On Thu, May 8, 2014 at 4:57 PM, Eric McCorkle <eric.mccorkle at oracle.com> wrote:
> On 05/08/14 15:54, Werner Dietl wrote:
>> Alex, all,
>>
>> thanks for this update.
>>
>> Type annotations in the jdk9 repo are currently broken for serious
>> use. On Feb 7 I identified the invalid re-ordering performed in
>> ClassReader:
>>
>> http://mail.openjdk.java.net/pipermail/type-annotations-dev/2014-February/001592.html
>>
>> This problem still exists in jdk9 - the problematic changeset wasn't
>> rolled back, nor was the fix applied. I have fixed this problem in
>> type-annotations.
>> I was waiting to hear an update on this before pursuing the other
>> outstanding changes.
>
> I am aware of the fix that was pushed into the type annotations forest.
>  As we discussed when you developed the fix, I cannot push it into
> jdk9-dev as-is, because the development practices of the langtools team
> require that any changeset that introduces behavioral changes be
> accompanied by a test.
>
> I will take care of the test this time, so that you can run checkers on
> the contents of jdk9.  However, in the future, we ask that patches
> submitted against jdk9 conform to the langtools group's development
> practices.
>
>> You say that after the new changes type-annotations will be seriously
>> out of date. I don't see why this would be the case. I'm currently
>> keeping type-annotations in sync with jdk9, making sure that the
>> Checker Framework correctly works with type-annotations. Once these
>> further refactorings go into jdk9, they should be easy to pull into
>> type-annotations.
>
> I am posting a series of three (maybe just two) patches against jdk9,
> which rearchitect a significant portion of the frontend implementation.
>  The end result is that it may not be trivial to rebase all the changes
> you list below.
>
>
>> All the interesting changes are against jdk9/langtools:
>>
>> - com/sun/tools/javac/code/TypeAnnotationPosition.java:
>>   I added a "copy" method to make it possible to create an updated
>> TAP. Alternatively, constructors that give a new TAP with a changed
>> location would be possible. A mechanism like this is required to allow
>> the Checker Framework to build TAPs for defaulted locations.
>
> That sounds fine.  Also, it shouldn't be affected by anything I'm doing.
>
>>   One issue I run into is making the Checker Framework work with both
>> JDK 8 and JDK 9 - the APIs for things like TAP changed significantly
>> and I now need to access many things reflectively to correctly access
>> the right API. One simple change I made was to make field
>> "exception_index" public to allow direct access from JDK 8 and JDK 9.
>> As the upcoming changes will require further changes in the Checker
>> Framework, making this change is not necessary.
>
> My changes affect exception_index in nontrivial ways...  I am now
> storing catch-types and class ids, as well as exception indexes in a
> private field.  Exception indexes are accessed via a getter.
>
> However, this is a temporary measure.  My tentative agenda for 8u40 is
> to rework the way we do exception parameters and eliminate.
>
>>
>> - com/sun/tools/javac/code/TypeAnnotations.java: initializes an
>> Options object that is unused. I removed that code.
>
> Most if not all of TypeAnnotations.java is going away anyway.  I've
> moved most of the core functionality into Annotate.
>
>> - com/sun/tools/javac/comp/Check.java: the call to Assert.checkNonNull
>> always constructs a toString of an annotation - this was slow and
>> annoying when debugging some problems. Other uses of Assert also use
>> simple error messages, so I adapted this one.
>
> A good change.  We may actually want to file a blanket issue about this
> as part of langtools code cleanup projects.
>
>> - com/sun/tools/javac/jvm/ClassReader.java: the jdk 9 code incorrectly
>> reads the location of type annotations, which results in a compiler
>> crash. type-annotations fixed this.
>
> Discussed above.
>
> (skipped a bunch that won't be affected by my work)
>
>> - test/tools/javac/annotations/typeAnnotations/failures/AnnotatedImport.java
>> is being ignored in jdk9, even though it is working. Same for
>> AnnotatedPackage1 and CantAnnotateStaticClass3.
>
> The positions patch will probably fix a few of the @ignore'd tests.  So
> I'd planned to go through, figure out what's working, and turn them back on.
>
> (The rest should be unaffected by my work)
>
>> How should I go about getting these changes into jdk9/jdk and jdk9/langtools?
>> Is it enough to send the above descriptions to compiler-dev and point
>> interested reviewers to a diff between the two repos? Should I attach
>> a diff?
>
> I have three major patches planned: remove AnnotatedType, which is
> already out for public review, the "positions patch", which is about to
> be, and a patch to remove code obsoleted by the positions patch.
>
> Here's what I'll do: we want to get the first two in ASAP, but the third
> is mostly for code cleanliness, and it may cause trouble rebasing some
> of those changes.  I'll hold off on the cleanup patch, and we'll look at
> bringing the changes over after.
>
> As for the reorder fix, I will commit that to jdk9 (with a test) before
> I commit the two major changes, so that you can run checkers with them.



-- 
http://www.google.com/profiles/wdietl