-
Notifications
You must be signed in to change notification settings - Fork 15.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make C++ code generation of string oneof field compatible with C++11 #113
Conversation
CLAs look good, thanks! |
printer->Print("const "); | ||
} | ||
field_generators_.get(field).GeneratePrivateMembers(printer); | ||
} | ||
} | ||
|
||
printer->Print("}* $name$_default_oneof_instance_ = NULL;\n", | ||
printer->Print("} const* $name$_default_oneof_instance_ = NULL;\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you add this const?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's to retain constness of the generated string fields.
I wondered why the original code attributed const to string fields as well as
to message fields.
And it seemed to me it was to prevent the other part of the generated code
from calling non-const methods of those supposed-to-be-immune default instances'
fields by mistake.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding "const" here forces the initialization code of this struct to do "const_cast" everywhere. Doesn't seem to be a win to me. Can you remove this "const" and also these "const_cast"s?
Preprocessor token right after string literal without whitespace results in parse failure in C++11.
string oneof field was generated into "const ArenaStringPtr" field inside "default_oneof_instance_" struct (of name "<class name>OneofInstance"). On the other hand, in C++11, const field of type with trivial default constructor causes enclosing type's implicit default constructor to be deleted. Since ArenaStringPtr has tirvial default constructor, this caused default constructor of "default_oneof_instance_" struct to be deleted, making the constructor call inside generated code invalid and fail to compile.
To be clear, first two commits are unchanged and the third commit is modified according to your request. |
Make C++ code generation of string oneof field compatible with C++11
Support json parsing for wrapper values
Groups are a deprecated feature, only available in proto2. Now, when an incoming payload eventually contains a group, the decoder no longer crashes, it will skip ahead until the group is over and read the remaining fields normally. https://developers.google.com/protocol-buffers/docs/proto#groups https://developers.google.com/protocol-buffers/docs/reference/proto2-spec#group_field
This PR doesn't introduce any C++11 specific feature, but only fixes the behavior with C++11 so that it works fine in the same way as with C++03.
You can reproduce the "problem" by
make check
configured withCXXFLAGS=-std=c++11
.Applying first commit will reveal the
oneof
part.Tested with clang 3.5.0 and gcc 4.9.2 with and without
-std=c++11
.