PSA: -Wundef and including gmock.h and gtest.h
Karl Wiberg
tl;dr:
- Preprocessor constants will no longer default to 0.
- #include webrtc/test/gmock.h and webrtc/test/gtest.h.
This CL enables the -Wundef flag for clang. This means that testing the value of a preprocessor constant with #if etc. will now require that it’s actually defined. If it isn’t, clang will issue a warning, which gets turned into an error because we also use -Werror.
This is a good thing, because it allows us to replace this sort of error-prone construct:
struct Foo {
int x;
#ifdef FOO_DEBUG
int y;
#endif
};
with this:
struct Foo {
int x;
#if FOO_DEBUG
int y;
#endif
};
So why is it better? Because the second construct will give three different results depending on if you explicitly set FOO_DEBUG to 1 (the debug stuff gets enabled), explicitly set FOO_DEBUG to 0 (the debug stuff gets disabled), or don’t set it (compilation error); whereas the first construction (and the second, if we didn’t use -Wundef) treats not setting FOO_DEBUG the same as explicitly disabling it. This can lead to hilarious and time-consuming bugs if this is in a header file that gets included in multiple build targets, and some build targets forget to set FOO_DEBUG (see the bug linked from the CL).
You can of course still use #ifdef and #if defined(...) to test whether a preprocessor constant is defined or not. But now that you can rely on clang to complain about undefined preprocessor constants instead of defaulting them to 0, using plain #if is often a better choice.
It may come as a shocking surprise to you that not all of the third-party headers we include can handle being compiled with -Wundef. At least some protobuf-generated headers—and a small number of others—require this beautiful annotation:
RTC_PUSH_IGNORING_WUNDEF()
#include "webrtc/modules/audio_processing/debug.pb.h"
RTC_POP_IGNORING_WUNDEF()
gmock.h and gtest.h accounted for the overwhelming majority of the places where this was necessary, so I made wrappers for them; instead of including them from testing/gtest/include/gtest/ and testing/gmock/include/gmock/, you should now include them from webrtc/test/. Henrik Kjellander immediately jumped on this and asked me to also change the DEPS files to implement a general prohibition against including anything from testing/—so you’re not just supposed to use the gmock.h and gtest.h files from webrtc/test/, you’re required to.