reducing use of TypeAnnotations constructor

[Jonathan Gibbons]

Oops.  Actually, I simplified the code even more.  Here is the latest 
version of that segment of code. Note, this is all just refactoring to 
remove direct use of AnnotatedType.  The intent is not to be changing 
overall functionality, and I'm not changing names or anything like that.

-- Jon



--- old/src/share/classes/com/sun/tools/javac/code/TypeAnnotations.java 
2013-08-27 11:39:25.966283140 -0700
+++ new/src/share/classes/com/sun/tools/javac/code/TypeAnnotations.java 
2013-08-27 11:39:25.898283140 -0700
@@ -360,25 +360,15 @@
                  return type;
              }
              if (type.hasTag(TypeTag.ARRAY)) {
+                Type.ArrayType arType = (Type.ArrayType) 
type.unannotatedType();
+                Type.ArrayType tomodify = new Type.ArrayType(null, 
arType.tsym);
                  Type toreturn;
-                Type.ArrayType tomodify;
-                Type.ArrayType arType;
-                {
-                    Type touse = type;
-                    if (type.isAnnotated()) {
-                        Type.AnnotatedType atype = 
(Type.AnnotatedType)type;
-                        toreturn = new 
Type.AnnotatedType(atype.underlyingType);
- ((Type.AnnotatedType)toreturn).typeAnnotations = atype.typeAnnotations;
-                        touse = atype.underlyingType;
-                        arType = (Type.ArrayType) touse;
-                        tomodify = new Type.ArrayType(null, arType.tsym);
- ((Type.AnnotatedType)toreturn).underlyingType = tomodify;
-                    } else {
-                        arType = (Type.ArrayType) touse;
-                        tomodify = new Type.ArrayType(null, arType.tsym);
-                        toreturn = tomodify;
-                    }
+                if (type.isAnnotated()) {
+                    toreturn = 
tomodify.annotatedType(type.getAnnotationMirrors());
+                } else {
+                    toreturn = tomodify;
                  }
+
                  JCArrayTypeTree arTree = arrayTypeTree(typetree);

                  ListBuffer<TypePathEntry> depth = ListBuffer.lb();


On 08/27/2013 11:31 AM, Werner Dietl wrote:
> Jon,
>
> did you forget to attach a patch in that last mail?
> I will try to work through all your comments in the afternoon today.
>
> cu, WMD.
>
>
> On Tue, Aug 27, 2013 at 11:12 AM, Jonathan Gibbons
> <jonathan.gibbons at oracle.com> wrote:
>> 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
>>
>>
>
>