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

[Eric McCorkle]

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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: eric_mccorkle.vcf
Type: text/x-vcard
Size: 314 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/type-annotations-dev/attachments/20140508/59ecf9f1/eric_mccorkle.vcf>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: <http://mail.openjdk.java.net/pipermail/type-annotations-dev/attachments/20140508/59ecf9f1/signature.asc>