Skip to content
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

Merged
merged 3 commits into from
Dec 12, 2014

Conversation

nsuke
Copy link
Contributor

@nsuke nsuke commented Nov 30, 2014

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 with CXXFLAGS=-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.

@googlebot
Copy link

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",
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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?

nsuke added 3 commits December 3, 2014 23:05
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.
@nsuke
Copy link
Contributor Author

nsuke commented Dec 7, 2014

To be clear, first two commits are unchanged and the third commit is modified according to your request.

xfxyjwf added a commit that referenced this pull request Dec 12, 2014
Make C++ code generation of string oneof field compatible with C++11
@xfxyjwf xfxyjwf merged commit 001e82a into protocolbuffers:master Dec 12, 2014
TeBoring pushed a commit to TeBoring/protobuf that referenced this pull request Jan 19, 2019
Support json parsing for wrapper values
yordis pushed a commit to yordis/protobuf that referenced this pull request Dec 8, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants