-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Description
Does the feature exist in the most recent commit?
No.
Why do we need this feature?
I have been going about in Chromium, trying to make clang's -Wexit-time-destructors
a default enabled warning, in order to reduce the risk with destruction time ordering issues. Upon further investigation, I noticed that some of the test that are coming back with warnings happen to trip the warning whenever they use TYPED_TEST_SUITE_P
. This macro looks like this:
// The variables defined in the type-parameterized test macros are
// static as typically these macros are used in a .h file that can be
// #included in multiple translation units linked together.
#define TYPED_TEST_SUITE_P(SuiteName) \
static ::testing::internal::TypedTestSuitePState \
GTEST_TYPED_TEST_SUITE_P_STATE_(SuiteName)
And ::testing::internal::TypedTestSuitePState
looks like this:
// State of the definition of a type-parameterized test suite.
class GTEST_API_ TypedTestSuitePState {
public:
TypedTestSuitePState() : registered_(false) {}
// Adds the given test name to defined_test_names_ and return true
// if the test suite hasn't been registered; otherwise aborts the
// program.
bool AddTestName(const char* file, int line, const char* case_name,
const char* test_name) {
if (registered_) {
fprintf(stderr,
"%s Test %s must be defined before "
"REGISTER_TYPED_TEST_SUITE_P(%s, ...).\n",
FormatFileLocation(file, line).c_str(), test_name, case_name);
fflush(stderr);
posix::Abort();
}
registered_tests_.emplace(test_name, CodeLocation(file, line));
return true;
}
bool TestExists(const std::string& test_name) const {
return registered_tests_.count(test_name) > 0;
}
const CodeLocation& GetCodeLocation(const std::string& test_name) const {
RegisteredTestsMap::const_iterator it = registered_tests_.find(test_name);
GTEST_CHECK_(it != registered_tests_.end());
return it->second;
}
// Verifies that registered_tests match the test names in
// defined_test_names_; returns registered_tests if successful, or
// aborts the program otherwise.
const char* VerifyRegisteredTestNames(const char* test_suite_name,
const char* file, int line,
const char* registered_tests);
private:
typedef ::std::map<std::string, CodeLocation, std::less<>> RegisteredTestsMap;
bool registered_;
RegisteredTestsMap registered_tests_;
};
A global static for this type requires non-trivial destructors, and although the use by gtest is safe here, that ends up interfering with enforcing this warning elsewhere.
Describe the proposal.
There are two ways to make googletest not interfere with this warning:
a) We make these static objects use some sort of NoDestructor type, similar to chromium's base::NoDestructor
or abseil's NoDestructor
. Notice provides an attribute for this too, namely, [[clang::no_destroy]]
(https://clang.llvm.org/docs/AttributeReference.html#no-destroy)
b) We add a macro with for clang, with a compile pragma, blessing in particular TYPED_TEST_SUITE_P
and other similar macros.
I think b is more realistic, and I could even help implementing it, if that's something that would be acceptable.
Is the feature specific to an operating system, compiler, or build system version?
It is specific to clang with -Wexit-time-destructors
.