nemequ / hedley

A C/C++ header to help move #ifdefs out of your code
https://nemequ.github.io/hedley/
Creative Commons Zero v1.0 Universal
774 stars 51 forks source link

Using a trailing semicolon after some macros causes warnings on some platforms #49

Open aharrison24 opened 3 years ago

aharrison24 commented 3 years ago

Some macros, such as HEDLEY_STATIC_ASSERT expand to nothing when they are not supported by a particular platform. This can result in hard-to-fix warnings when using -Wpedantic on clang or gcc.

Specifically, for C++11 and above, the following requires a semicolon at the end:

HEDLEY_STATIC_ASSERT(true, "");

See https://godbolt.org/z/4dYWveMaj

However, under -Wpedantic, the same line will raise a warning on C++98 because the macro expands to nothing, leaving a lone semicolon on the line. See https://godbolt.org/z/Wbhcjsqvz

I wonder if it would be helpful to have the 'empty' version of the macro actually expand to something the requires a semi-colon at the end, so that it behaves the same as the normal version.

Looking at other libraries, such as Doctest, the preferred technique seems to be to define a typedef with a unique name, which is effectively a no-op.

Something along the lines of this:

#define HEDLEY_STATIC_ASSERT(expr, message) \
  typedef int HEDLEY_UNIQUE_NAME(HEDLEY_ANON_FOR_SEMICOLON_)

where HEDLEY_UNIQUE_NAME is a macro which expands to a unique string:

#ifdef __COUNTER__
#  define HEDLEY_UNIQUE_NAME(name) HEDLEY_CONCAT(name, __COUNTER__)
#else
#  define HEDLEY_UNIQUE_NAME(name) HEDLEY_CONCAT(name, __LINE__)
#endif

Here's an example of what it does: https://godbolt.org/z/ddn4bjPjM

The use of __COUNTER__ if it is available is helpful for cases where the macro appears more than once on a single line, because C does not allow repeated typedefs. They're apparently OK in C++ though.

nemequ commented 3 years ago

Nice, thanks. I don't think I was aware of the typedef trick; I've always seen static assertions implemented as either the standard one, or using the trick to declare an array with a negative length on failure. That semi-colon has been annoying me for a while…

The drawback here is that this won't work quite everywhere _Static_assert does, but I suspect it's okay. I'll do a survey of some projects using HEDLEY_STATIC_ASSERT to make sure making this change wouldn't break anyone's code; obviously I can't check proprietary software, but open-source should at least give an indication of the likelihood it would break anyone's code.

aharrison24 commented 3 years ago

The drawback here is that this won't work quite everywhere _Static_assert does...

The C++ standard mentions that static_assert can be used inside anonymous unions, but typedefs cannot, so that's one of those places. I'm curious to know if there are any other places. The only thing I've found is this stack overflow question that suggests some very bizarre options that it's hard to imagine anyone actually needing!

...or using the trick to declare an array with a negative length on failure

Well I guess if you were willing to consider an unused typedef then you might as well go the whole hog and fold in the negative-sized-array trick, as in https://stackoverflow.com/a/6766023

One of the commenters points out that an unused local typedef can raise a warning when -Wunused-local-typedef is in force, which probably scuppers this whole approach, anyway. I believe Doctest only uses the typedef trick for macros that appear at global scope, so it can get away with it.

FWIW there's a discussion of various approaches at https://stackoverflow.com/questions/35530850/how-to-require-a-semicolon-after-a-macro, but none of them ticks all of the boxes, unfortunately.

I'm happy for this issue to be closed, given that there don't seem to be any viable solutions!