reducing use of TypeAnnotations constructor

[Jonathan Gibbons]

On 08/26/2013 09:55 PM, Jonathan Gibbons wrote:
> On 08/26/2013 07:23 PM, Jonathan Gibbons wrote:
>> Werner,
>>
>> As you know, we're looking to improve the representation of annotated 
>> types in javac, perhaps by replacing AnnotatedType, in order to 
>> reduce "those pesky calls of unannotatedType".
>>
>> I've attached a patch for your consideration to replace direct use of 
>> the AnnotatedType constructor with a factory method on the underlying 
>> type to be annotated.
>>
>> This isn't the whole story about replacing AnnotatedType: just think 
>> of it as clearing out the easy bits first.  I do feel this is a step 
>> in the right direction, and will allow us to focus on the remaining 
>> uses of AnnotatedType.  As such, I may push this to TL soon enough, 
>> but you might want to preview it in the type-annotations forest 
>> before then.
>>
>> And, if you're interested, I'd be interested to hear any suggestions 
>> you may have on further reducing direct use of the TypeAnnotations 
>> type, by replacing such usages with virtual methods on Type, with 
>> appropriate impls in Type and AnnotatedType.
>>
>> -- Jon
>>
>>
>>
>>
>
> Werner,
>
> The next stage in reducing/eliminating the use of AnnotatedType is to 
> work on the two fields it provides, replacing direct access with 
> "reasonable" methods on Type. As far I can see, this is 3/4 easy.
>
> The two fields are typeAnnotations and underlyingType.
>
> Read access to typeAnnotations is available via 
> Type.getAnnotationMirrors() and read access to underlyingType is 
> available via Type.unannotatedType().  Yes, I know we want to get rid 
> of that method eventually, but for now it is convenient to use it. (We 
> can eliminate the method, eventually, when it is effectively a no-op 
> for all types)  So that takes care of read access to the fields of 
> AnnotatedType.
>
> Write access to trickier. It is reasonable to image we could have a 
> method on Type to set the annotations on a type. Such a method can 
> fail with IllegalStateException if the instance does not support 
> mutable annotations (e.g. if it is a shared Type object.) So that one 
> is OK, and that completes my 3 out of 4.
>
> That leaves 7 or so occurrences where you assign directly to 
> AnnotatedType.underlyingType.  I haven't yet been able to figure out 
> the abstract operation you are doing on a type when you change the 
> underlying type of an AnnotatedType -- are you conceptually creating a 
> new annotated type with a different underlying type? Is it important 
> that you mutate the instance?  For example, how hard would it be for 
> you to make the "underlyingType" field final, and fixup the code 
> accordingly?
>
> -- Jon
>
>
>


Werner,

Here's another simplification for you to consider, in TypeAnnotations.  
This is in the typeWithAnnotations method.  I created this cleanup by 
replacing direct use of AnnotatedType with appropriate methods on Type, 
simplified the code a bit, and hoisted identical code out of the then 
and else clauses, to get something that looks a lot cleaner, and doesn't 
not directly reference the AnnotatedType class.     With the 
refactoring, all the jtreg tests pass (of course.)

What do you think?

-- Jon