RFR: JDK-8193367: Annotated type variable bounds crash javac
Maurizio Cimadamore
maurizio.cimadamore at oracle.com
Thu Jan 24 18:20:57 UTC 2019
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();
> }
More information about the compiler-dev
mailing list