Fix Wpedantic warning 'extra ;' for c++ #10385
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
I signed the CLA now. I can not seem to add labels but would like to add "c++" and probably "release notes:no". |
src/google/protobuf/descriptor.h
Outdated
| @@ -615,7 +615,7 @@ class PROTOBUF_EXPORT Descriptor : private internal::SymbolBase { | |||
| GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(Descriptor); | |||
| }; | |||
|
|
|||
| PROTOBUF_INTERNAL_CHECK_CLASS_SIZE(Descriptor, 136); | |||
| PROTOBUF_INTERNAL_CHECK_CLASS_SIZE(Descriptor, 136) | |||
There was a problem hiding this comment.
please fix this macro to not end with a ; instead
There was a problem hiding this comment.
The macro is defined empty:
#if !defined(PROTOBUF_INTERNAL_CHECK_CLASS_SIZE) #define PROTOBUF_INTERNAL_CHECK_CLASS_SIZE(t, expected) #endif
which means the double ; comes from the fact that we have a regular statement before the macro, then emptyness (the macro) and then another ; after that.
There was a problem hiding this comment.
I can see that the removal of the ; from my patch would be problematic for anyone who has defined PROTOBUF_INTERNAL_CHECK_CLASS_SIZE as something which has content but doesn't end in ;
If it is the case that this macro is actually used, this pull request should probably be rejected or the macro definition there adapted.
There was a problem hiding this comment.
you could change the empty definition to
do {} while (false)
There was a problem hiding this comment.
since the macro's are used outside of a function scope, a regular function calling or piece of code won't compile there ("expected constructor, destructor, or type conversion before ';' token"). Putting an empty class or struct definition is also not possible since the macro is used multiple times so then you get redefinition errors.
The macro seems like something used internally by google, is that correct?
There was a problem hiding this comment.
you can change them to static_assert(true, "") which will be valid at all scopes.
* Disabling upb tests in python_cpp builds * Adding comment about upb
* Reverting changes to ruby aarch64 build. * Adding IN_DOCKER environment variable to prevent codegen * Pass the cmake-built protoc to the ruby runner
…ocolbuffers#10393) * Upgrade third_party/googletest submodule to current main branch We can finally do this upgrade now that we have dropped our autotools build. Googletest recommends living at head, so let's go straight to the most recent commit on main. For some reason the googletest archive is not present in the Bazel build mirror, so I removed that entry and just left the GitHub download link in our WORKSPACE file. Googletest now requires C++14, so I updated all the C++11 flags I could find to C++14 instead. I added a .bazelrc file to add -std=c++14 for all our Bazel builds. * Delete the empty //src/google/protobuf:protobuf_test target * Avoid building C++ unit tests in aarch64 jobs for Python and Ruby
marjoleinheyndrickx
force-pushed
the
main
branch
from
7726d76 to
9450259
Compare
|
I changed the macro as suggested |
|
Thanks for the PR. I will merge this after our CI system processes it. |
|
can somebody please explain what the purpose is of this empty macro, it seems rather strange, and added value for now is negative it creates problems, and why are there even still macros in use ? Wouldn't it be better if such a bad habit would be removed completely ? |
|
@killerbot242 the macro is not empty inside google's codebase. We use it to verify invariants that we don't externalize |
Fix Wpedantic warning 'extra ;' for c++
Fixing a few "extra ‘;’ [-Wpedantic]" I got when compiling with gcc 10.3.0. This is simply removal of a few extra semicolons, does not affect the functionality of anything.
This is in c++