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

Warning message for literal shift amounts outside the canonical domain

XMLWordPrintable

    • Icon: CSR CSR
    • Resolution: Unresolved
    • Icon: P4 P4
    • tbd
    • tools
    • None
    • behavioral
    • low
    • As with any new warning, existing code that triggers the new warning and is compiled with `-Werror` will no longer build and will therefore need to be addressed by either fixing the code or suppressing the warning.
    • add/remove/modify command line option
    • JDK

      Summary

      Add a new compiler warning for out-of-range bit shifts, i.e., an int bit shift not in the range [0...31] or a long bit shift not in the range [0...63].

      Problem

      When bit shifting an int or long value by an amount X, all but the last 5 or 6 (respectively) bits of X are ignored.

      This can create a trap for the unwary, as in this example:

      public long readLongBigEndian(byte[] buf, int offset) {
          return ((buf[offset + 0] & 0xff) << 56)   // BUG HERE
               | ((buf[offset + 1] & 0xff) << 48)   // BUG HERE
               | ((buf[offset + 2] & 0xff) << 40)   // BUG HERE
               | ((buf[offset + 3] & 0xff) << 32)   // BUG HERE
               | ((buf[offset + 4] & 0xff) << 24)
               | ((buf[offset + 5] & 0xff) << 16)
               | ((buf[offset + 6] & 0xff) << 8)
               | ((buf[offset + 7] & 0xff);
      }

      This is a prime opportunity for the compiler to alert the developer to a potential problem by emitting a warning.

      Solution

      Emit a lint warning when the compiler is able to detect this situation, i.e., when the shift amount is a known constant value and out of range for the target type. The new warning will be added in the lossy-conversions lint category.

      The lossy-conversions lint category is disabled by default, so the new warning will not occur without the -Xlint, -Xlint:all, or `-Xlint:lossy-conversions command line flag.

      Choice Of Lint Category

      The lossy-conversions category currently contains only one other warning ("implicit cast from {0} to {1} in compound assignment is possibly lossy") and so people immediately associate it with that type of lossy conversion. However using an out-of-range shift value is also a "lossy conversion" because bits are being thrown away to coerce a 32 or 64 bit value into a 5 or 6 bit value (i.e., the shift amount), so this lint category is appropriate. In any case, no other existing lint category would make sense, and creating an entirely new lint category for just this one warning would be overkill.

      Specification

      Given this input:

      public class Simple {
          public long multiplyByFourGiga(int x) {
              return x << 32;
          }
      }

      and assuming lint category lossy-conversions is enabled, the compiler would output this warning:

      Simple.java:3: warning: [lossy-conversions] shifting int by 32 bits is equivalent to shifting by 0 bit(s)
              return x << 32;
                          ^
      1 warning

      Note: In the message template, the first shift amount can never be equal to 1, so "bits" is used, whereas the second shift amount can possibly be 1 so "bit(s)" is used instead.

      Here is the Javadoc diff in module-info.java for the compiler:

      --- i/src/jdk.compiler/share/classes/module-info.java
      +++ w/src/jdk.compiler/share/classes/module-info.java
      @@ -166,7 +166,7 @@
        * <tr><th scope="row">{@code finally}              <td>{@code finally} clauses that do not terminate normally
        * <tr><th scope="row">{@code identity}             <td>use of a value-based class where an identity class is expected
        * <tr><th scope="row">{@code incubating}           <td>use of incubating modules
      - * <tr><th scope="row">{@code lossy-conversions}    <td>possible lossy conversions in compound assignment
      + * <tr><th scope="row">{@code lossy-conversions}    <td>possible lossy conversions in compound assignment or bit shift
        * <tr><th scope="row">{@code missing-explicit-ctor} <td>missing explicit constructors in public and protected classes
        *                                                      in exported packages
        * <tr><th scope="row">{@code module}               <td>module system related issues

      Here is the diff of the output of javac --help-lint:

      @@ -17,7 +17,7 @@
                                effects. Users are encouraged to use the 'identity' category for all future
                                and existing uses of 'synchronization'.
           incubating           Warn about use of incubating modules.
      -    lossy-conversions    Warn about possible lossy conversions in compound assignment.
      +    lossy-conversions    Warn about possible lossy conversions in compound assignment and bit shift operations.
           missing-explicit-ctor Warn about missing explicit constructors in public and protected classes in exported packages.
           module               Warn about module system related issues.
           opens                Warn about issues regarding module opens.

      Here's the man page diff:

      @@ -592,7 +592,7 @@
                     • incubating: Warns about the use of incubating modules.
      
                     • lossy‐conversions: Warns about possible lossy  conversions  in
      -                compound assignment.
      +                compound assignment and bit shift operations.
      
                     • missing‐explicit‐ctor:  Warns about missing explicit construc‐
                       tors in public and protected classes in exported packages.

            acobbs Archie Cobbs
            duke J. Duke
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

              Created:
              Updated: