Better encapsulation for AnnotatedType

[Joel Borggren-Franck]

Hi Werner,

In general I think this looks good.

On 2013-09-15, Werner Dietl wrote:
> Jon, all,
> 
> as you suggested, I encapsulated the typeAnnotations and underlyingType
> fields in com.sun.tools.javac.code.Type.AnnotatedType
> 
> This actually made the code simpler in a few places.

> There is one new failure:
> langtools/test/tools/javac/processing/model/type/BasicAnnoTests.java
> 
> However, I think the test case is wrong: it uses a "normal" annotation
> processor and therefore runs too early. It should be changed to be a
> "type annotation processor", i.e. it should run after FLOW.
> I suggest we create a subclass/alternative to
> JavacTestingAbstractProcessor and use that to test type annotations.

Annotation processing will occur before flow. Eventually we need to move
to a model were we trigger stuff that annotation processing depends on
before attr. Some type anno stuff needs to be done there, and some can
be delayed. For example there is this bug:
https://bugs.openjdk.java.net/browse/JDK-8014016 that is assigned to Jan
in the javac team.

I'm not sure the concept of an after flow processor makes sense at all.

However it seems a bit strange that your encapsulation introduced an
error like this.

> 
> I'm not sure whether this had an effect on javadoc - if so, we should
> revisit when the typeAnnotations queue is flushed.
> 
> Please review this changeset:
> 
> http://hg.openjdk.java.net/type-annotations/type-annotations/langtools/rev/6f595e036db7
> 
> If this first step is OK, we can next introduce the TypeMetadata
> abstraction.
> 
> 
> Regarding SymbolMetadata, I find it a bit strange that the methods that
> access SymbolMetadata in Symbol are still called "xxxAnnotations", e.g.
> com.sun.tools.javac.code.Symbol.getAnnotations()
> Should these also be renamed?
> 

I'm working towards removing those altogether (but probably in the 9
timeframe). There was I reason I didn't want Andreas to rename them, but
I can't recall it now :)

> I have one more change that I just pushed:
> 
> http://hg.openjdk.java.net/type-annotations/type-annotations/langtools/rev/e1772bcdf768
> 
> I changed com.sun.tools.javac.tree.TreeCopier.visitLabeledStatement
> to take the copied body instead of the original body.
> Charlie Garrett discovered this problem after painful debugging of a
> Checker Framework problem that depended on whether a label is present or
> not.
> I cannot present a simple test case for the error, but the change
> doesn't break any existing test cases.
> Also note that this was the only "unused variable" warning in
> TreeCopier. Warnings are useful.
> 

Seems reasonable.

cheers
/Joel