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.

Sign up for GitHub

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

Fix Wpedantic warning 'extra ;' for c++ #10385

Merged
merged 10 commits into from
Aug 17, 2022
Merged

Fix Wpedantic warning 'extra ;' for c++ #10385

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b136ee3
Fix Wpedantic warning 'extra ;' for c++
marjoleinheyndrickx Aug 10, 2022
8177cd0
Merge branch 'protocolbuffers:main' into main
marjoleinheyndrickx Aug 11, 2022
71af720
Use protoc version for --version
deannagarcia Aug 10, 2022
8fee1de
Update to have two distinct methods
deannagarcia Aug 10, 2022
f36b3c2
Disabling upb tests in python_cpp builds (#10392)
mkruskal-google Aug 11, 2022
4c3ab6c
Reverting changes to ruby aarch64 build. (#10394)
mkruskal-google Aug 11, 2022
7df94e2
Upgrade third_party/googletest submodule to current main branch (#10393)
acozzette Aug 12, 2022
9450259
Fixing php proto generation script to work properly during sync (#10395)
mkruskal-google Aug 12, 2022
def602d
change macro to avoid pedantic warning
marjoleinheyndrickx Aug 12, 2022
8335ad0
Merge branch 'protocolbuffers:main' into main
marjoleinheyndrickx Aug 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions src/google/protobuf/descriptor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix this macro to not end with a ; instead

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@marjoleinheyndrickx marjoleinheyndrickx Aug 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could change the empty definition to

do {} while (false)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can change them to static_assert(true, "") which will be valid at all scopes.


// Describes a single field of a message. To get the descriptor for a given
// field, first get the Descriptor for the message in which it is defined,
Expand Down Expand Up @@ -992,7 +992,7 @@ class PROTOBUF_EXPORT FieldDescriptor : private internal::SymbolBase {
GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(FieldDescriptor);
};

PROTOBUF_INTERNAL_CHECK_CLASS_SIZE(FieldDescriptor, 72);
PROTOBUF_INTERNAL_CHECK_CLASS_SIZE(FieldDescriptor, 72)

// Describes a oneof defined in a message type.
class PROTOBUF_EXPORT OneofDescriptor : private internal::SymbolBase {
Expand Down Expand Up @@ -1073,7 +1073,7 @@ class PROTOBUF_EXPORT OneofDescriptor : private internal::SymbolBase {
GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(OneofDescriptor);
};

PROTOBUF_INTERNAL_CHECK_CLASS_SIZE(OneofDescriptor, 40);
PROTOBUF_INTERNAL_CHECK_CLASS_SIZE(OneofDescriptor, 40)

// Describes an enum type defined in a .proto file. To get the EnumDescriptor
// for a generated enum type, call TypeName_descriptor(). Use DescriptorPool
Expand Down Expand Up @@ -1244,7 +1244,7 @@ class PROTOBUF_EXPORT EnumDescriptor : private internal::SymbolBase {
GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(EnumDescriptor);
};

PROTOBUF_INTERNAL_CHECK_CLASS_SIZE(EnumDescriptor, 72);
PROTOBUF_INTERNAL_CHECK_CLASS_SIZE(EnumDescriptor, 72)

// Describes an individual enum constant of a particular type. To get the
// EnumValueDescriptor for a given enum value, first get the EnumDescriptor
Expand Down Expand Up @@ -1329,7 +1329,7 @@ class PROTOBUF_EXPORT EnumValueDescriptor : private internal::SymbolBaseN<0>,
GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(EnumValueDescriptor);
};

PROTOBUF_INTERNAL_CHECK_CLASS_SIZE(EnumValueDescriptor, 32);
PROTOBUF_INTERNAL_CHECK_CLASS_SIZE(EnumValueDescriptor, 32)

// Describes an RPC service. Use DescriptorPool to construct your own
// descriptors.
Expand Down Expand Up @@ -1412,7 +1412,7 @@ class PROTOBUF_EXPORT ServiceDescriptor : private internal::SymbolBase {
GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(ServiceDescriptor);
};

PROTOBUF_INTERNAL_CHECK_CLASS_SIZE(ServiceDescriptor, 48);
PROTOBUF_INTERNAL_CHECK_CLASS_SIZE(ServiceDescriptor, 48)

// Describes an individual service method. To obtain a MethodDescriptor given
// a service, first get its ServiceDescriptor, then call
Expand Down Expand Up @@ -1501,7 +1501,7 @@ class PROTOBUF_EXPORT MethodDescriptor : private internal::SymbolBase {
GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(MethodDescriptor);
};

PROTOBUF_INTERNAL_CHECK_CLASS_SIZE(MethodDescriptor, 64);
PROTOBUF_INTERNAL_CHECK_CLASS_SIZE(MethodDescriptor, 64)

// Describes a whole .proto file. To get the FileDescriptor for a compiled-in
// file, get the descriptor for something defined in that file and call
Expand Down Expand Up @@ -1709,7 +1709,7 @@ class PROTOBUF_EXPORT FileDescriptor : private internal::SymbolBase {
GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(FileDescriptor);
};

PROTOBUF_INTERNAL_CHECK_CLASS_SIZE(FileDescriptor, 152);
PROTOBUF_INTERNAL_CHECK_CLASS_SIZE(FileDescriptor, 152)

// ===================================================================

Expand Down
2 changes: 1 addition & 1 deletion src/google/protobuf/parse_context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,7 @@ RotRight7AndReplaceLowByte(uint64_t res, const char& byte) {
res |= 0xFF & byte;
#endif
return res;
};
}

inline PROTOBUF_ALWAYS_INLINE
const char* ReadTagInlined(const char* ptr, uint32_t* out) {
Expand Down