Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8227617

Inconsistent handling of short between AnnotationValue.toString and Elements.getConstantExpression

    Details

    • Type: Bug
    • Status: Open
    • Priority: P4
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: tools
    • Labels:
      None

      Description

      The handling of byte in AnnotationValue.toString [1] and Elements.getConstantExpression [2] is consistent, it is always represented as `String.format("(byte)0x%02x", b)`.

      On the other hand, `short` is represented as `String.format("(short)%d", s)` in AnnotationValue.toString, and `String.valueOf(s)` in Elements.getConstantExpression.

      This is allowed by the spec, which says getConstantExpression returns text "in a form suitable for representing the value in source code", while AnnotationValue.toString returns text "form suitable for representing this value *in the source code of an annotation*". It's safe to omit the casts for shorts and bytes that appear as constants inside annotations.

      Nevertheless, the different handling of byte and short seems slightly inconsistent and surprising. Would it makes to always add the cast to `(short)` for consistency?

      [1] https://docs.oracle.com/en/java/javase/11/docs/api/java.compiler/javax/lang/model/element/AnnotationValue.html#toString()
      [2] https://docs.oracle.com/en/java/javase/11/docs/api/java.compiler/javax/lang/model/util/Elements.html#getConstantExpression(java.lang.Object)

      Repro:

      ```
      import java.util.Set;
      import javax.annotation.processing.AbstractProcessor;
      import javax.annotation.processing.RoundEnvironment;
      import javax.annotation.processing.SupportedAnnotationTypes;
      import javax.lang.model.SourceVersion;
      import javax.lang.model.element.Element;
      import javax.lang.model.element.TypeElement;
      import javax.lang.model.util.Elements;

      @interface A {
        short s();
      }

      @interface B {
        byte b();
      }

      @A(s = 1)
      class One {}

      @B(b = 2)
      class Two {}

      @SupportedAnnotationTypes("A")
      public class P extends AbstractProcessor {

        @Override
        public SourceVersion getSupportedSourceVersion() {
          return SourceVersion.latestSupported();
        }

        @Override
        public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment roundEnv) {
          if (roundEnv.processingOver()) {
            return false;
          }
          Element e = roundEnv.getElementsAnnotatedWith(A.class).iterator().next();
          System.err.println(
              "AnnotationValue short = "
                  + e.getAnnotationMirrors().get(0).getElementValues().values().iterator().next());

          e = roundEnv.getElementsAnnotatedWith(B.class).iterator().next();
          System.err.println(
              "AnnotationValue byte = "
                  + e.getAnnotationMirrors().get(0).getElementValues().values().iterator().next());

          Elements elements = processingEnv.getElementUtils();
          System.err.println("getConstantExpression short = " + elements.getConstantExpression((short) 1));
          System.err.println("getConstantExpression byte = " + elements.getConstantExpression((byte) 2));

          return false;
        }
      }
      ```

      $ javac -processor P -fullversion P.java
      javac full version "14-ea+5-129"
      AnnotationValue short = 1
      AnnotationValue byte = (byte)0x02
      getConstantExpression short = (short)1
      getConstantExpression byte = (byte)0x02


      The patch to treat short consistently is something like:

      ```
      diff --git a/src/java.base/share/classes/sun/reflect/annotation/AnnotationInvocationHandler.java b/src/java.base/share/classes/sun/reflect/annotation/AnnotationInvocationHandler.java
      --- a/src/java.base/share/classes/sun/reflect/annotation/AnnotationInvocationHandler.java
      +++ b/src/java.base/share/classes/sun/reflect/annotation/AnnotationInvocationHandler.java
      @@ -186,6 +186,8 @@ class AnnotationInvocationHandler implem
                       return toSourceString((long) value);
                   else if (type == Byte.class)
                       return toSourceString((byte) value);
      + else if (type == Short.class)
      + return toSourceString((short) value);
                   else
                       return value.toString();
               } else {
      @@ -300,6 +302,10 @@ class AnnotationInvocationHandler implem
               return String.format("(byte)0x%02x", b);
           }
       
      + private static String toSourceString(short s) {
      + return String.format("(short)%d", s);
      + }
      +
           private static String toSourceString(long ell) {
               return String.valueOf(ell) + "L";
           }
      diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Constants.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Constants.java
      --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Constants.java
      +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Constants.java
      @@ -68,6 +68,7 @@ public class Constants {
               case FLOAT: return formatFloat((Float) value);
               case DOUBLE: return formatDouble((Double) value);
               case CHAR: return formatChar((Character) value);
      + case SHORT: return formatShort((Short) value);
               }
               if (value instanceof String)
                   return formatString((String) value);
      ```

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                cushon Liam Miller-Cushon
                Reporter:
                cushon Liam Miller-Cushon
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated: