eson/googletest
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Googletest export

Browse Source 
Fix emission of -Wzero-as-null-pointer-constant when comparing integers.

The following code fails to compile:

#pragma clang diagnostic error "-Wzero-as-null-pointer-constant"
void foo() {
  EXPECT_EQ(0, 0);
}

This happens because gtest checks the first argument to EXPECT_EQ and
ASSERT_EQ is a null pointer constant. The magic it does to do this causes the
warning to be emitted.

This patch removes that check. It replaces the explicit check with a Compare
overload that can only be selected when 0 or nullptr is passed on the LHS
with a pointer on the right.

This patch does not suppress -Wzero-as-null-pointer-constant when users
are actually using it as NULL.

PiperOrigin-RevId: 236654634
This commit is contained in:
Abseil Team 2019-03-04 11:10:22 -05:00 committed by Gennadiy Civil
parent a1dd07786b
commit 3dd2e841c3
3 changed files with 65 additions and 97 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

63
googletest/include/gtest/gtest.h
View File

@ -52,9 +52,11 @@
#ifndef GTEST_INCLUDE_GTEST_GTEST_H_
#define GTEST_INCLUDE_GTEST_GTEST_H_
#include <cstddef>
#include <limits>
#include <memory>
#include <ostream>
#include <type_traits>
#include <vector>
#include "gtest/internal/gtest-internal.h"
@ -1532,18 +1534,17 @@ GTEST_API_ AssertionResult CmpHelperEQ(const char* lhs_expression,
BiggestInt lhs,
BiggestInt rhs);
// The helper class for {ASSERT|EXPECT}_EQ. The template argument
// lhs_is_null_literal is true iff the first argument to ASSERT_EQ()
// is a null pointer literal. The following default implementation is
// for lhs_is_null_literal being false.
template <bool lhs_is_null_literal>
class EqHelper {
public:
// This templatized version is for the general case.
template <typename T1, typename T2>
template <
typename T1, typename T2,
// Disable this overload for cases where one argument is a pointer
// and the other is the null pointer constant.
typename std::enable_if<!std::is_integral<T1>::value ||
!std::is_pointer<T2>::value>::type* = nullptr>
static AssertionResult Compare(const char* lhs_expression,
const char* rhs_expression,
const T1& lhs,
const char* rhs_expression, const T1& lhs,
const T2& rhs) {
return CmpHelperEQ(lhs_expression, rhs_expression, lhs, rhs);
}
@ -1560,44 +1561,12 @@ class EqHelper {
BiggestInt rhs) {
return CmpHelperEQ(lhs_expression, rhs_expression, lhs, rhs);
}
};
// This specialization is used when the first argument to ASSERT_EQ()
// is a null pointer literal, like NULL, false, or 0.
template <>
class EqHelper<true> {
public:
// We define two overloaded versions of Compare(). The first
// version will be picked when the second argument to ASSERT_EQ() is
// NOT a pointer, e.g. ASSERT_EQ(0, AnIntFunction()) or
// EXPECT_EQ(false, a_bool).
template <typename T1, typename T2>
static AssertionResult Compare(
const char* lhs_expression, const char* rhs_expression, const T1& lhs,
const T2& rhs,
// The following line prevents this overload from being considered if T2
// is not a pointer type. We need this because ASSERT_EQ(NULL, my_ptr)
// expands to Compare("", "", NULL, my_ptr), which requires a conversion
// to match the Secret* in the other overload, which would otherwise make
// this template match better.
typename EnableIf<!std::is_pointer<T2>::value>::type* = nullptr) {
return CmpHelperEQ(lhs_expression, rhs_expression, lhs, rhs);
}
// This version will be picked when the second argument to ASSERT_EQ() is a
// pointer, e.g. ASSERT_EQ(NULL, a_pointer).
template <typename T>
static AssertionResult Compare(
const char* lhs_expression,
const char* rhs_expression,
// We used to have a second template parameter instead of Secret*. That
// template parameter would deduce to 'long', making this a better match
// than the first overload even without the first overload's EnableIf.
// Unfortunately, gcc with -Wconversion-null warns when "passing NULL to
// non-pointer argument" (even a deduced integral argument), so the old
// implementation caused warnings in user code.
Secret* /* lhs (NULL) */,
T* rhs) {
const char* lhs_expression, const char* rhs_expression,
// Handle cases where '0' is used as a null pointer literal.
std::nullptr_t /* lhs */, T* rhs) {
// We already know that 'lhs' is a null pointer.
return CmpHelperEQ(lhs_expression, rhs_expression, static_cast<T*>(nullptr),
rhs);
@ -2046,9 +2015,7 @@ class TestWithParam : public Test, public WithParamInterface<T> {
// ASSERT_GT(records.size(), 0) << "There is no record left.";
#define EXPECT_EQ(val1, val2) \
EXPECT_PRED_FORMAT2(::testing::internal:: \
EqHelper<GTEST_IS_NULL_LITERAL_(val1)>::Compare, \
val1, val2)
EXPECT_PRED_FORMAT2(::testing::internal::EqHelper::Compare, val1, val2)
#define EXPECT_NE(val1, val2) \
EXPECT_PRED_FORMAT2(::testing::internal::CmpHelperNE, val1, val2)
#define EXPECT_LE(val1, val2) \
@ -2061,9 +2028,7 @@ class TestWithParam : public Test, public WithParamInterface<T> {
EXPECT_PRED_FORMAT2(::testing::internal::CmpHelperGT, val1, val2)
#define GTEST_ASSERT_EQ(val1, val2) \
ASSERT_PRED_FORMAT2(::testing::internal:: \
EqHelper<GTEST_IS_NULL_LITERAL_(val1)>::Compare, \
val1, val2)
ASSERT_PRED_FORMAT2(::testing::internal::EqHelper::Compare, val1, val2)
#define GTEST_ASSERT_NE(val1, val2) \
ASSERT_PRED_FORMAT2(::testing::internal::CmpHelperNE, val1, val2)
#define GTEST_ASSERT_LE(val1, val2) \

31
googletest/include/gtest/internal/gtest-internal.h
View File

@ -124,37 +124,6 @@ class IgnoredValue {
IgnoredValue(const T& /* ignored */) {} // NOLINT(runtime/explicit)
};
// The only type that should be convertible to Secret* is nullptr.
// The other null pointer constants are not of a type that is convertible to
// Secret*. Only the literal with the right value is.
template <typename T>
using TypeIsValidNullptrConstant = std::integral_constant<
bool, std::is_same<typename std::decay<T>::type, std::nullptr_t>::value ||
!std::is_convertible<T, Secret*>::value>;
// Two overloaded helpers for checking at compile time whether an
// expression is a null pointer literal (i.e. NULL or any 0-valued
// compile-time integral constant). These helpers have no
// implementations, as we only need their signatures.
//
// Given IsNullLiteralHelper(x), the compiler will pick the first
// version if x can be implicitly converted to Secret*, and pick the
// second version otherwise. Since Secret is a secret and incomplete
// type, the only expression a user can write that has type Secret* is
// a null pointer literal. Therefore, we know that x is a null
// pointer literal if and only if the first version is picked by the
// compiler.
std::true_type IsNullLiteralHelper(Secret*, std::true_type);
std::false_type IsNullLiteralHelper(IgnoredValue, std::false_type);
std::false_type IsNullLiteralHelper(IgnoredValue, std::true_type);
// A compile-time bool constant that is true if and only if x is a null pointer
// literal (i.e. nullptr, NULL or any 0-valued compile-time integral constant).
#define GTEST_IS_NULL_LITERAL_(x) \
decltype(::testing::internal::IsNullLiteralHelper( \
x, \
::testing::internal::TypeIsValidNullptrConstant<decltype(x)>()))::value
// Appends the user-supplied message to the Google-Test-generated message.
GTEST_API_ std::string AppendUserMessage(
const std::string& gtest_msg, const Message& user_msg);

68
googletest/test/gtest_unittest.cc
View File

@ -515,22 +515,23 @@ TEST_F(FormatEpochTimeInMillisAsIso8601Test, PrintsEpochStart) {
# pragma option push -w-ccc -w-rch
# endif
// Tests that GTEST_IS_NULL_LITERAL_(x) is true when x is a null
// pointer literal.
TEST(NullLiteralTest, IsTrueForNullLiterals) {
EXPECT_TRUE(GTEST_IS_NULL_LITERAL_(NULL)); // NOLINT
EXPECT_TRUE(GTEST_IS_NULL_LITERAL_(0)); // NOLINT
EXPECT_TRUE(GTEST_IS_NULL_LITERAL_(0u)); // NOLINT
EXPECT_TRUE(GTEST_IS_NULL_LITERAL_(nullptr));
}
// Tests that the LHS of EXPECT_EQ or ASSERT_EQ can be used as a null literal
// when the RHS is a pointer type.
TEST(NullLiteralTest, LHSAllowsNullLiterals) {
EXPECT_EQ(0, static_cast<void*>(nullptr)); // NOLINT
ASSERT_EQ(0, static_cast<void*>(nullptr)); // NOLINT
EXPECT_EQ(NULL, static_cast<void*>(nullptr)); // NOLINT
ASSERT_EQ(NULL, static_cast<void*>(nullptr)); // NOLINT
EXPECT_EQ(nullptr, static_cast<void*>(nullptr));
ASSERT_EQ(nullptr, static_cast<void*>(nullptr));
// Tests that GTEST_IS_NULL_LITERAL_(x) is false when x is not a null
// pointer literal.
TEST(NullLiteralTest, IsFalseForNonNullLiterals) {
EXPECT_FALSE(GTEST_IS_NULL_LITERAL_(1));
EXPECT_FALSE(GTEST_IS_NULL_LITERAL_(0.0));
EXPECT_FALSE(GTEST_IS_NULL_LITERAL_('a'));
EXPECT_FALSE(GTEST_IS_NULL_LITERAL_(static_cast<void*>(nullptr)));
const int* const p = nullptr;
EXPECT_EQ(0, p); // NOLINT
ASSERT_EQ(0, p); // NOLINT
EXPECT_EQ(NULL, p); // NOLINT
ASSERT_EQ(NULL, p); // NOLINT
EXPECT_EQ(nullptr, p);
ASSERT_EQ(nullptr, p);
}
struct ConvertToAll {
@ -540,6 +541,13 @@ struct ConvertToAll {
}
};
struct ConvertToPointer {
template <class T>
operator T*() const { // NOLINT
return nullptr;
}
};
struct ConvertToAllButNoPointers {
template <typename T,
typename std::enable_if<!std::is_pointer<T>::value, int>::type = 0>
@ -548,11 +556,37 @@ struct ConvertToAllButNoPointers {
}
};
struct MyType {};
inline bool operator==(MyType const&, MyType const&) { return true; }
TEST(NullLiteralTest, ImplicitConversion) {
EXPECT_FALSE(GTEST_IS_NULL_LITERAL_(ConvertToAll{}));
EXPECT_FALSE(GTEST_IS_NULL_LITERAL_(ConvertToAllButNoPointers{}));
EXPECT_EQ(ConvertToPointer{}, static_cast<void*>(nullptr));
#if !defined(__GNUC__) || defined(__clang__)
// Disabled due to GCC bug gcc.gnu.org/PR89580
EXPECT_EQ(ConvertToAll{}, static_cast<void*>(nullptr));
#endif
EXPECT_EQ(ConvertToAll{}, MyType{});
EXPECT_EQ(ConvertToAllButNoPointers{}, MyType{});
}
#ifdef __clang__
#pragma clang diagnostic push
#if __has_warning("-Wzero-as-null-pointer-constant")
#pragma clang diagnostic error "-Wzero-as-null-pointer-constant"
#endif
#endif
TEST(NullLiteralTest, NoConversionNoWarning) {
// Test that gtests detection and handling of null pointer constants
// doesn't trigger a warning when '0' isn't actually used as null.
EXPECT_EQ(0, 0);
ASSERT_EQ(0, 0);
}
#ifdef __clang__
#pragma clang diagnostic pop
#endif
# ifdef __BORLANDC__
// Restores warnings after previous "#pragma option push" suppressed them.
# pragma option pop