You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by dr...@apache.org on 2008/06/30 22:45:48 UTC

svn commit: r672907 - in /incubator/thrift/trunk: compiler/cpp/src/parse/t_const_value.h compiler/cpp/src/thriftl.ll compiler/cpp/src/thrifty.yy test/BrokenConstants.thrift

Author: dreiss
Date: Mon Jun 30 13:45:47 2008
New Revision: 672907

URL: http://svn.apache.org/viewvc?rev=672907&view=rev
Log:
Partial handlling of 64-bit integer constants.

- Parse integer constants in Thrift files as 64-bit ints.
- Die if an overflow occurs.
- Warn if an enum value cannot fit in 32 bits.
- Add a simple test for the above.

I ran all of the generators over BrokenConstants.thrift before adding the
overflow, and they appeared to work.  However, the code generated was not
always valid (for example, the 64-bit constant must have an LL suffix in C++).

Added:
    incubator/thrift/trunk/test/BrokenConstants.thrift
Modified:
    incubator/thrift/trunk/compiler/cpp/src/parse/t_const_value.h
    incubator/thrift/trunk/compiler/cpp/src/thriftl.ll
    incubator/thrift/trunk/compiler/cpp/src/thrifty.yy

Modified: incubator/thrift/trunk/compiler/cpp/src/parse/t_const_value.h
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/compiler/cpp/src/parse/t_const_value.h?rev=672907&r1=672906&r2=672907&view=diff
==============================================================================
--- incubator/thrift/trunk/compiler/cpp/src/parse/t_const_value.h (original)
+++ incubator/thrift/trunk/compiler/cpp/src/parse/t_const_value.h Mon Jun 30 13:45:47 2008
@@ -8,6 +8,7 @@
 #define T_CONST_VALUE_H
 
 #include "t_const.h"
+#include <stdint.h>
 #include <map>
 #include <vector>
 
@@ -30,7 +31,7 @@
 
   t_const_value() {}
 
-  t_const_value(int val) {
+  t_const_value(int64_t val) {
     set_integer(val);
   }
 
@@ -47,12 +48,12 @@
     return stringVal_;
   }
 
-  void set_integer(int val) {
+  void set_integer(int64_t val) {
     valType_ = CV_INTEGER;
     intVal_ = val;
   }
 
-  int get_integer() const {
+  int64_t get_integer() const {
     return intVal_;
   }
 
@@ -97,7 +98,7 @@
   std::map<t_const_value*, t_const_value*> mapVal_;
   std::vector<t_const_value*> listVal_;
   std::string stringVal_;
-  int intVal_;
+  int64_t intVal_;
   double doubleVal_;
 
   t_const_value_type valType_;

Modified: incubator/thrift/trunk/compiler/cpp/src/thriftl.ll
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/compiler/cpp/src/thriftl.ll?rev=672907&r1=672906&r2=672907&view=diff
==============================================================================
--- incubator/thrift/trunk/compiler/cpp/src/thriftl.ll (original)
+++ incubator/thrift/trunk/compiler/cpp/src/thriftl.ll Mon Jun 30 13:45:47 2008
@@ -15,6 +15,8 @@
 
 %{
 
+#include <errno.h>
+
 #include "main.h"
 #include "globals.h"
 #include "parse/t_program.h"
@@ -30,6 +32,11 @@
   exit(1);
 }
 
+void integer_overflow(char* text) {
+  yyerror("This integer is too big: \"%s\"\n", text);
+  exit(1);
+}
+
 %}
 
 /**
@@ -181,12 +188,20 @@
 "yield"              { thrift_reserved_keyword(yytext); }
 
 {intconstant} {
-  yylval.iconst = atoi(yytext);
+  errno = 0;
+  yylval.iconst = strtoll(yytext, NULL, 10);
+  if (errno == ERANGE) {
+    integer_overflow(yytext);
+  }
   return tok_int_constant;
 }
 
 {hexconstant} {
-  sscanf(yytext+2, "%x", &yylval.iconst);
+  errno = 0;
+  yylval.iconst = strtoll(yytext+2, NULL, 16);
+  if (errno == ERANGE) {
+    integer_overflow(yytext);
+  }
   return tok_int_constant;
 }
 

Modified: incubator/thrift/trunk/compiler/cpp/src/thrifty.yy
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/compiler/cpp/src/thrifty.yy?rev=672907&r1=672906&r2=672907&view=diff
==============================================================================
--- incubator/thrift/trunk/compiler/cpp/src/thrifty.yy (original)
+++ incubator/thrift/trunk/compiler/cpp/src/thrifty.yy Mon Jun 30 13:45:47 2008
@@ -13,7 +13,10 @@
  * @author Mark Slee <mc...@facebook.com>
  */
 
+#define __STDC_LIMIT_MACROS
+#define __STDC_FORMAT_MACROS
 #include <stdio.h>
+#include <inttypes.h>
 #include "main.h"
 #include "globals.h"
 #include "parse/t_program.h"
@@ -35,7 +38,7 @@
  */
 %union {
   char*          id;
-  int            iconst;
+  int64_t        iconst;
   double         dconst;
   bool           tbool;
   t_doc*         tdoc;
@@ -496,6 +499,9 @@
       if ($4 < 0) {
         pwarning(1, "Negative value supplied for enum %s.\n", $2);
       }
+      if ($4 > INT_MAX) {
+        pwarning(1, "64-bit value supplied for enum %s.\n", $2);
+      }
       $$ = new t_enum_value($2, $4);
       if ($1 != NULL) {
         $$->set_doc($1);
@@ -569,6 +575,9 @@
       pdebug("ConstValue => tok_int_constant");
       $$ = new t_const_value();
       $$->set_integer($1);
+      if ($1 < INT32_MIN || $1 > INT32_MAX) {
+        pwarning(1, "64-bit constant \"%"PRIi64"\" may not work in all languages.\n", $1);
+      }
     }
 | tok_dub_constant
     {

Added: incubator/thrift/trunk/test/BrokenConstants.thrift
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/test/BrokenConstants.thrift?rev=672907&view=auto
==============================================================================
--- incubator/thrift/trunk/test/BrokenConstants.thrift (added)
+++ incubator/thrift/trunk/test/BrokenConstants.thrift Mon Jun 30 13:45:47 2008
@@ -0,0 +1,6 @@
+const i64 myint = 68719476736
+const i64 broken = 9876543210987654321  // A little over 2^63
+
+enum foo {
+  bar = 68719476736
+}