RFR: JDK-8193367: Annotated type variable bounds crash javac

[Maurizio Cimadamore]

On 24/01/2019 15:49, B. Blaser wrote:
> On Thu, 24 Jan 2019 at 15:30, Maurizio Cimadamore
> <maurizio.cimadamore at oracle.com> wrote:
>> I'm a bit confused (but it could also be because I'm misremembering) - wasn't the whole point of the exercise to have cloned type variables to piggy back on the clonee's getUpperBound?
>>
>> In the patch I see this:
>>
>> @Override
>> 1640         public TypeVar cloneWithMetadata(TypeMetadata md) {
>> 1641             return new TypeVar(tsym, getUpperBound(), lower, md) {
>> 1642                 @Override
>> 1643                 public Type baseType() { return TypeVar.this.baseType(); }
>> 1644             };
>> 1645         }
>>
>>
>> Which doesn't seem better than the current code - e.g. the upper bound is fetched when the cloned var is created, so, if the bound is updated after the fact, such changes will not be reflected in the clones var?
> The current getUpperBound() sets the clone's bound to the clonee's
> bound (tsym.type.getUpperBound()) at every invocation while still
> unset (null or tag=NONE):
>
> http://hg.openjdk.java.net/jdk/jdk/file/525f212f1bda/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Type.java#l1659
>
> which is exactly what we need in our issue (see Types::getBounds).
> Overriding the accessor in cloneWithMetadata() is then no more
> necessary.
>
> This fixes the issue and tests are clean.

Urgh - I forgot that getUpperBound was side-effecting things.

That said, this side-effect thing really does come from the very first 
type annotation push back in 2013! Back then the compiler was using a 
different architecture for supporting type annotations (there was an 
explicit AnnotatedType) - so I believe this could be a leftover of that era.

Now that you have properly encapsulated access to the bound, I bet that 
this hack can go away, and you should be able to mimic current behavior 
by overriding the 'getUpperBound' in the cloned type variable.

Maurizio

>
> Do we agree?
>
> Bernard