Public review of rearchitected front-end type annotations pipeline

[Werner Dietl]

Hi Eric,

thanks for integrating this huge refactoring!
I've worked on adapting the Checker Framework to this; there are only
small code changes I needed to perform, but a few tests started
failing.

I do notice that test case

test/tools/javac/processing/model/type/BasicAnnoTests.java

is now failing for even the simple examples that previously worked.
That is, the Type for a Tree is apparently not set correctly.

To further show the problem, run javadoc on this example:

===
import java.lang.annotation.*;

public class JavaDoc {
    public @TA Object foo();
}

@Documented
@Target(ElementType.TYPE_USE)
@interface TA { }
===

Before the refactoring and with JDK 8, @TA was visible on the return
type. It is now no longer present.
What do you make of this changed behavior?

Best regards,
cu, WMD.



On Tue, Jun 3, 2014 at 10:24 AM, Eric McCorkle <eric.mccorkle at oracle.com> wrote:
> Since I last posted a revision, there has been a significant amount of
> review activity by the javac team.
>
> I have, at this point, merged what I had intended to be the third patch
> in the series, and created a combined patch.  I have also fixed a few
> issues that were found in our review.
>
> The current revision can be found here:
> http://cr.openjdk.java.net/~emc/8027262/webrev.02/
>
> Note that since this incorporates what was to be the third patch in the
> series, there will not be a third patch anymore.  It should be possible
> to test Checkers at this point.  It is also likely that this will be
> integrated very shortly.
>
> At this point, I will begin evaluating this entire body of work for
> inclusion in the upcoming 8u20 update release.  One thing I would like
> to do is a "before/after" evaluation with as many "real-world" uses of
> type annotations as possible.  To facilitate this, I will shortly post a
> large patch which combines all of my work thus far, which should be
> applied against the 8-update repo.  When that patch is published, I
> would very much appreciate if anyone who is actually using type
> annotations at this point could evaluate and report on the correctness
> of the 8-update repo with and without the patch.
>
> As a final note, we are currently putting plans together for 8u40 work.
>  My tentative plans at this point focus on the javax.lang.model API and
> internal javac API's, with particular focus on ensuring that Type's
> always have the annotations they should, and on using that to facilitate
> cleaning up some code in the backend (mainly Gen), though that is not
> set in stone at this point.
>
> On 05/15/14 12:40, Eric McCorkle wrote:
>> I have integrated a number of edits to the patch, from both the public
>> and internal reviews, and I have fixed a few issues I found.  A new
>> version has been posted here:
>>
>> http://cr.openjdk.java.net/~emc/8027262/webrev.01/
>>
>> It is likely that this change will be integrated later today, or
>> tomorrow.  At that point, I will post the third and final patch in the
>> series.
>>
>> On 05/09/14 15:49, Eric McCorkle wrote:
>>> Hello,
>>>
>>> This is the public review of the second in my series of patches dealing
>>> with type annotations.
>>>
>>> http://cr.openjdk.java.net/~emc/8027262/
>>>
>>> This patch rearchitects the type annotations pipeline, integrating
>>> handling of type annotations directly into the javac
>>> MemberEnter/Annotate/Attr pipeline.  It represents the majority of the
>>> work I have been doing regarding type annotations for 8u20.
>>>
>>> The handling of type annotations is now dispatched by the MemberEnter or
>>> Attr visitors and uses information from those visitors.  Most of the
>>> actual functionality is now implemented in Annotate.
>>>
>>> The new test Stress.java is the test for this patch.  Stress.java will
>>> cause 8-release javac to fail with an assertion failure.  Its addition
>>> to the test suite demonstrates that this change fixes those cases.
>>>
>>> This patch addresses a number of JBS issues:
>>> https://bugs.openjdk.java.net/browse/JDK-8027262
>>> https://bugs.openjdk.java.net/browse/JDK-8027261
>>> https://bugs.openjdk.java.net/browse/JDK-8027258
>>> https://bugs.openjdk.java.net/browse/JDK-8027182
>>> and possibly others as well.
>>>
>>> Note: this patch does not attempt to remove code made obsolete; however,
>>> any such code is very clearly marked as deprecated.  Removal of dead
>>> code will be done in the last of the series.  This patch also does not
>>> attempt to re-enable tests which were previously disabled.  That will be
>>> done as a separate patch as well.
>>>



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