valitydev/thrift
14
0
Fork 0
mirror of https://github.com/valitydev/thrift.git synced 2024-11-07 10:48:51 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

THRIFT-1270. compiler: add --allow-neg-keys argument to allow

Browse Source 
This switch allows the user to specify explicit negative field IDs in Thrift IDL in order to avoid breaking wire protocol.

Patch: Dave Watson

git-svn-id: https://svn.apache.org/repos/asf/thrift/trunk@1158967 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Bryan Duxbury 2011-08-17 23:17:04 +00:00
parent ff1c69bc1c
commit c7206a4011
4 changed files with 70 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
compiler/cpp/src/globals.h
View File

@ -114,4 +114,19 @@ extern char* g_doctext;
*/
extern int g_doctext_lineno;
/**
* Whether or not negative field keys are accepted.
*
* When a field does not have a user-specified key, thrift automatically
* assigns a negative value. However, this is fragile since changes to the
* file may unintentionally change the key numbering, resulting in a new
* protocol that is not backwards compatible.
*
* When g_allow_neg_field_keys is enabled, users can explicitly specify
* negative keys. This way they can write a .thrift file with explicitly
* specified keys that is still backwards compatible with older .thrift files
* that did not specify key values.
*/
extern int g_allow_neg_field_keys;
#endif

10
compiler/cpp/src/main.cc
View File

@ -151,6 +151,11 @@ char* g_doctext;
*/
int g_doctext_lineno;
/**
* Whether or not negative field keys are accepted.
*/
int g_allow_neg_field_keys;
/**
* Flags to control code generation
*/
@ -639,6 +644,9 @@ void usage() {
fprintf(stderr, " -v[erbose] Verbose mode\n");
fprintf(stderr, " -r[ecurse] Also generate included files\n");
fprintf(stderr, " -debug Parse debug trace to stdout\n");
fprintf(stderr, " --allow-neg-keys Allow negative field keys (Used to "
"preserve protocol\n");
fprintf(stderr, " compatibility with older .thrift files)\n");
fprintf(stderr, " --gen STR Generate code with a dynamically-registered generator.\n");
fprintf(stderr, " STR has the form language[:key1=val1[,key2,[key3=val3]]].\n");
fprintf(stderr, " Keys and values are options passed to the generator.\n");
@ -970,6 +978,8 @@ int main(int argc, char** argv) {
g_verbose = 1;
} else if (strcmp(arg, "-r") == 0 || strcmp(arg, "-recurse") == 0 ) {
gen_recurse = true;
} else if (strcmp(arg, "-allow-neg-keys") == 0) {
g_allow_neg_field_keys = true;
} else if (strcmp(arg, "-gen") == 0) {
arg = argv[++i];
if (arg == NULL) {

9
compiler/cpp/src/parse/t_field.h
View File

@ -150,4 +150,13 @@ class t_field : public t_doc {
};
/**
* A simple struct for the parser to use to store a field ID, and whether or
* not it was specified by the user or automatically chosen.
*/
struct t_field_id {
int64_t value;
bool auto_assigned;
};
#endif

43
compiler/cpp/src/thrifty.yy
View File

@ -71,6 +71,7 @@ const int struct_is_union = 1;
char* dtext;
t_field::e_req ereq;
t_annotation* tannot;
t_field_id tfieldid;
}
/**
@ -174,7 +175,7 @@ const int struct_is_union = 1;
%type<tannot> TypeAnnotation
%type<tfield> Field
%type<iconst> FieldIdentifier
%type<tfieldid> FieldIdentifier
%type<ereq> FieldRequiredness
%type<ttype> FieldType
%type<tconstv> FieldValue
@ -870,14 +871,14 @@ Field:
CaptureDocText FieldIdentifier FieldRequiredness FieldType tok_identifier FieldValue XsdOptional XsdNillable XsdAttributes TypeAnnotations CommaOrSemicolonOptional
{
pdebug("tok_int_constant : Field -> FieldType tok_identifier");
if ($2 < 0) {
if ($2.auto_assigned) {
pwarning(1, "No field key specified for %s, resulting protocol may have conflicts or not be backwards compatible!\n", $5);
if (g_strict >= 192) {
yyerror("Implicit field keys are deprecated and not allowed with -strict");
exit(1);
}
}
$$ = new t_field($4, $5, $2);
$$ = new t_field($4, $5, $2.value);
$$->set_req($3);
if ($6 != NULL) {
g_scope->resolve_const_value($6, $4);
@ -902,14 +903,42 @@ FieldIdentifier:
tok_int_constant ':'
{
if ($1 <= 0) {
pwarning(1, "Nonpositive value (%d) not allowed as a field key.\n", $1);
$1 = y_field_val--;
if (g_allow_neg_field_keys) {
/*
* g_allow_neg_field_keys exists to allow users to add explicitly
* specified key values to old .thrift files without breaking
* protocol compatibility.
*/
if ($1 != y_field_val) {
/*
* warn if the user-specified negative value isn't what
* thrift would have auto-assigned.
*/
pwarning(1, "Negative field key (%d) differs from what would be "
"auto-assigned by thrift (%d).\n", $1, y_field_val);
}
/*
* Leave $1 as-is, and update y_field_val to be one less than $1.
* The FieldList parsing will catch any duplicate key values.
*/
y_field_val = $1 - 1;
$$.value = $1;
$$.auto_assigned = false;
} else {
pwarning(1, "Nonpositive value (%d) not allowed as a field key.\n",
$1);
$$.value = y_field_val--;
$$.auto_assigned = true;
}
} else {
$$.value = $1;
$$.auto_assigned = false;
}
$$ = $1;
}
|
{
$$ = y_field_val--;
$$.value = y_field_val--;
$$.auto_assigned = true;
}
FieldRequiredness: