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

[Maurizio Cimadamore]

On 24/01/2019 17:48, B. Blaser wrote:
> On Thu, 24 Jan 2019 at 16:56, Maurizio Cimadamore
> <maurizio.cimadamore at oracle.com> wrote:
>> 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.
> Yes, as here under (only Type.java, the other files are identical to webrev.00).
> Issue and tests are OK, seems good?
> Bernard

Yep - this seems like a good cleanup!

Maurizio

>
> diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Type.java
> b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Type.java
> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Type.java
> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Type.java
> @@ -896,7 +896,7 @@
>               if (moreInfo && bound != null && !isPrintingBound)
>                   try {
>                       isPrintingBound = true;
> -                    s.append("{:").append(bound.bound).append(":}");
> +                    s.append("{:").append(bound.getUpperBound()).append(":}");
>                   } finally {
>                       isPrintingBound = false;
>                   }
> @@ -1607,7 +1607,7 @@
>            *  itself. Furthermore, the erasure_field of the class
>            *  points to the first class or interface bound.
>            */
> -        public Type bound = null;
> +        private Type _bound = null;
>
>           /** The lower bound of this type variable.
>            *  TypeVars don't normally have a lower bound, so it is normally set
> @@ -1620,7 +1620,7 @@
>               super(null, TypeMetadata.EMPTY);
>               Assert.checkNonNull(lower);
>               tsym = new TypeVariableSymbol(0, name, this, owner);
> -            this.bound = null;
> +            this.setUpperBound(null);
>               this.lower = lower;
>           }
>
> @@ -1632,15 +1632,20 @@
>                          TypeMetadata metadata) {
>               super(tsym, metadata);
>               Assert.checkNonNull(lower);
> -            this.bound = bound;
> +            this.setUpperBound(bound);
>               this.lower = lower;
>           }
>
>           @Override
>           public TypeVar cloneWithMetadata(TypeMetadata md) {
> -            return new TypeVar(tsym, bound, lower, md) {
> +            return new TypeVar(tsym, getUpperBound(), lower, md) {
>                   @Override
>                   public Type baseType() { return TypeVar.this.baseType(); }
> +
> +                @Override @DefinedBy(Api.LANGUAGE_MODEL)
> +                public Type getUpperBound() { return
> TypeVar.this.getUpperBound(); }
> +
> +                public void setUpperBound(Type bound) {
> TypeVar.this.setUpperBound(bound); }
>               };
>           }
>
> @@ -1655,12 +1660,9 @@
>           }
>
>           @Override @DefinedBy(Api.LANGUAGE_MODEL)
> -        public Type getUpperBound() {
> -            if ((bound == null || bound.hasTag(NONE)) && this != tsym.type) {
> -                bound = tsym.type.getUpperBound();
> -            }
> -            return bound;
> -        }
> +        public Type getUpperBound() { return _bound; }
> +
> +        public void setUpperBound(Type bound) { this._bound = bound; }
>
>           int rank_field = -1;
>
> @@ -1709,7 +1711,7 @@
>                               WildcardType wildcard) {
>               super(name, owner, lower);
>               this.lower = Assert.checkNonNull(lower);
> -            this.bound = upper;
> +            this.setUpperBound(upper);
>               this.wildcard = wildcard;
>           }
>
> @@ -1725,9 +1727,14 @@
>
>           @Override
>           public CapturedType cloneWithMetadata(TypeMetadata md) {
> -            return new CapturedType(tsym, bound, bound, lower, wildcard, md) {
> +            return new CapturedType(tsym, getUpperBound(),
> getUpperBound(), lower, wildcard, md) {
>                   @Override
>                   public Type baseType() { return CapturedType.this.baseType(); }
> +
> +                @Override @DefinedBy(Api.LANGUAGE_MODEL)
> +                public Type getUpperBound() { return
> CapturedType.this.getUpperBound(); }
> +
> +                public void setUpperBound(Type bound) {
> CapturedType.this.setUpperBound(bound); }
>               };
>           }
>
> @@ -1832,7 +1839,7 @@
>
>           public void complete() {
>               for (List<Type> l = tvars; l.nonEmpty(); l = l.tail) {
> -                ((TypeVar)l.head).bound.complete();
> +                ((TypeVar)l.head).getUpperBound().complete();
>               }
>               qtype.complete();
>           }