Ryan Greenberg created THRIFT-3751:
--------------------------------------

             Summary: Compiler allows field ids that are too large for 
generated code
                 Key: THRIFT-3751
                 URL: https://issues.apache.org/jira/browse/THRIFT-3751
             Project: Thrift
          Issue Type: Bug
         Environment: thrift 0.9.3
            Reporter: Ryan Greenberg
         Attachments: big_field_ids.thrift, big_field_ids_max32.thrift

Shorts are used to represent field ids in generated code for some languages 
(e.g. Java), however using a field id greater than max short does not produce 
an error and instead produced incorrect generated code.

Compiling the attached big_field_ids_max32.thrift succeeds and produces this 
Java:

{code}
@SuppressWarnings({"cast", "rawtypes", "serial", "unchecked"})
@Generated(value = "Autogenerated by Thrift Compiler (0.9.3)", date = 
"2016-03-22")
public class BigFieldIds implements org.apache.thrift.TBase<BigFieldIds, 
BigFieldIds._Fields>, java.io.Serializable, Cloneable, Comparable<BigFieldIds> {
  private static final org.apache.thrift.protocol.TStruct STRUCT_DESC = new 
org.apache.thrift.protocol.TStruct("BigFieldIds");

  private static final org.apache.thrift.protocol.TField MAX1_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max1", 
org.apache.thrift.protocol.TType.STRING, (short)1);
  private static final org.apache.thrift.protocol.TField MAX2_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max2", 
org.apache.thrift.protocol.TType.STRING, (short)3);
  private static final org.apache.thrift.protocol.TField MAX3_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max3", 
org.apache.thrift.protocol.TType.STRING, (short)7);
  private static final org.apache.thrift.protocol.TField MAX4_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max4", 
org.apache.thrift.protocol.TType.STRING, (short)15);
  private static final org.apache.thrift.protocol.TField MAX5_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max5", 
org.apache.thrift.protocol.TType.STRING, (short)31);
  private static final org.apache.thrift.protocol.TField MAX6_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max6", 
org.apache.thrift.protocol.TType.STRING, (short)63);
  private static final org.apache.thrift.protocol.TField MAX7_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max7", 
org.apache.thrift.protocol.TType.STRING, (short)127);
  private static final org.apache.thrift.protocol.TField MAX8_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max8", 
org.apache.thrift.protocol.TType.STRING, (short)255);
  private static final org.apache.thrift.protocol.TField MAX9_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max9", 
org.apache.thrift.protocol.TType.STRING, (short)511);
  private static final org.apache.thrift.protocol.TField MAX10_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max10", 
org.apache.thrift.protocol.TType.STRING, (short)1023);
  private static final org.apache.thrift.protocol.TField MAX11_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max11", 
org.apache.thrift.protocol.TType.STRING, (short)2047);
  private static final org.apache.thrift.protocol.TField MAX12_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max12", 
org.apache.thrift.protocol.TType.STRING, (short)4095);
  private static final org.apache.thrift.protocol.TField MAX13_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max13", 
org.apache.thrift.protocol.TType.STRING, (short)8191);
  private static final org.apache.thrift.protocol.TField MAX14_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max14", 
org.apache.thrift.protocol.TType.STRING, (short)16383);
  private static final org.apache.thrift.protocol.TField MAX15_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max15", 
org.apache.thrift.protocol.TType.STRING, (short)32767);
  private static final org.apache.thrift.protocol.TField MAX16_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max16", 
org.apache.thrift.protocol.TType.STRING, (short)65535);
  private static final org.apache.thrift.protocol.TField MAX17_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max17", 
org.apache.thrift.protocol.TType.STRING, (short)131071);
  private static final org.apache.thrift.protocol.TField MAX18_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max18", 
org.apache.thrift.protocol.TType.STRING, (short)262143);
  private static final org.apache.thrift.protocol.TField MAX19_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max19", 
org.apache.thrift.protocol.TType.STRING, (short)524287);
  private static final org.apache.thrift.protocol.TField MAX20_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max20", 
org.apache.thrift.protocol.TType.STRING, (short)1048575);
  private static final org.apache.thrift.protocol.TField MAX21_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max21", 
org.apache.thrift.protocol.TType.STRING, (short)2097151);
  private static final org.apache.thrift.protocol.TField MAX22_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max22", 
org.apache.thrift.protocol.TType.STRING, (short)4194303);
  private static final org.apache.thrift.protocol.TField MAX23_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max23", 
org.apache.thrift.protocol.TType.STRING, (short)8388607);
  private static final org.apache.thrift.protocol.TField MAX24_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max24", 
org.apache.thrift.protocol.TType.STRING, (short)16777215);
  private static final org.apache.thrift.protocol.TField MAX25_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max25", 
org.apache.thrift.protocol.TType.STRING, (short)33554431);
  private static final org.apache.thrift.protocol.TField MAX26_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max26", 
org.apache.thrift.protocol.TType.STRING, (short)67108863);
  private static final org.apache.thrift.protocol.TField MAX27_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max27", 
org.apache.thrift.protocol.TType.STRING, (short)134217727);
  private static final org.apache.thrift.protocol.TField MAX28_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max28", 
org.apache.thrift.protocol.TType.STRING, (short)268435455);
  private static final org.apache.thrift.protocol.TField MAX29_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max29", 
org.apache.thrift.protocol.TType.STRING, (short)536870911);
  private static final org.apache.thrift.protocol.TField MAX30_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max30", 
org.apache.thrift.protocol.TType.STRING, (short)1073741823);
  private static final org.apache.thrift.protocol.TField MAX31_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max31", 
org.apache.thrift.protocol.TType.STRING, (short)2147483647);
  private static final org.apache.thrift.protocol.TField MAX32_FIELD_DESC = new 
org.apache.thrift.protocol.TField("max32", 
org.apache.thrift.protocol.TType.STRING, (short)-1);
...
{code}

Casting numbers greater than 32767 to short causes a compiler error:

{code}
javac -cp libthrift-0.9.3.jar:log4j-over-slf4j-1.7.7.jar:slf4j-api-1.7.7.jar 
BigFieldIds.java
BigFieldIds.java:2987: error: incompatible types: possible lossy conversion 
from int to short
          case 65535: // MAX16
               ^
BigFieldIds.java:2995: error: incompatible types: possible lossy conversion 
from int to short
          case 131071: // MAX17
               ^
BigFieldIds.java:3003: error: incompatible types: possible lossy conversion 
from int to short
          case 262143: // MAX18
               ^
BigFieldIds.java:3011: error: incompatible types: possible lossy conversion 
from int to short
          case 524287: // MAX19
               ^
BigFieldIds.java:3019: error: incompatible types: possible lossy conversion 
from int to short
          case 1048575: // MAX20
               ^
BigFieldIds.java:3027: error: incompatible types: possible lossy conversion 
from int to short
          case 2097151: // MAX21
               ^
BigFieldIds.java:3035: error: incompatible types: possible lossy conversion 
from int to short
          case 4194303: // MAX22
               ^
BigFieldIds.java:3043: error: incompatible types: possible lossy conversion 
from int to short
          case 8388607: // MAX23
               ^
BigFieldIds.java:3051: error: incompatible types: possible lossy conversion 
from int to short
          case 16777215: // MAX24
               ^
BigFieldIds.java:3059: error: incompatible types: possible lossy conversion 
from int to short
          case 33554431: // MAX25
               ^
BigFieldIds.java:3067: error: incompatible types: possible lossy conversion 
from int to short
          case 67108863: // MAX26
               ^
BigFieldIds.java:3075: error: incompatible types: possible lossy conversion 
from int to short
          case 134217727: // MAX27
               ^
BigFieldIds.java:3083: error: incompatible types: possible lossy conversion 
from int to short
          case 268435455: // MAX28
               ^
BigFieldIds.java:3091: error: incompatible types: possible lossy conversion 
from int to short
          case 536870911: // MAX29
               ^
BigFieldIds.java:3099: error: incompatible types: possible lossy conversion 
from int to short
          case 1073741823: // MAX30
               ^
BigFieldIds.java:3107: error: incompatible types: possible lossy conversion 
from int to short
          case 2147483647: // MAX31
               ^
16 errors
{code}

There are two corollary bugs:
- Compiler converts field ids that are larger than max *int* (i.e. > 2**31) to 
-1, and subsequently errors if there are multiple such fields.

{code}
$ thrift --gen java big_field_ids.thrift
"-1: max33" - field identifier/name has already been used
{code}

- Compiler does error when a field id is too large for its internal 
representation (i.e. > 2**63)

{code}
struct BigFieldIds {
  9223372036854775808: optional string max63
}

$ thrift --gen java big_field_ids.thrift
[ERROR:/Users/ryangreenberg/workspace/source/big_field_ids.thrift:2] (last 
token was '9223372036854775808')
This integer is too big: "9223372036854775808"
{code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to