Warning/Error on (EXPECT|ASSERT)_EQ(char*, char*)

9 views
Skip to first unread message

Zentaro Kavanagh

unread,
Mar 17, 2022, 5:43:15 PMMar 17
to cxx
This is usually unintentional when someone calls this test macro in google test on 2 c-string pointers. However it will often still "work" due to string interning when both strings are literals.

We recently saw an example where it showed up later in an ASAN build where presumably a side effect of ASANs instrumentation is to disable the string interning which made the error show up.

I tried the simple thing to just use static_assert in that function - unfortunately there are a couple of places that are relying on this (for a span and an iterator test). I wanted to at least try and get it to generate a warning so that I could try and build all the code to find if there are any other unintentional examples of this, but I don't really know how to make it generate a warning. Is  #pragma warning a thing in clang?

Or is this something we can implement with a linter or in a way that we can only enable it on the chromium code.

Dumb version (is there a cleaner way to do this?)...

diff --git a/googletest/include/gtest/gtest.h b/googletest/include/gtest/gtest.h
index 0c6e75c6..e3b45d27 100644
--- a/googletest/include/gtest/gtest.h
+++ b/googletest/include/gtest/gtest.h
@@ -1368,6 +1368,15 @@ class EqHelper {
   static AssertionResult Compare(const char* lhs_expression,
                                  const char* rhs_expression, const T1& lhs,
                                  const T2& rhs) {
+    constexpr bool are_both_c_strings = (
+        std::is_same<typename std::remove_cv<T1>::type, const char*>::value  ||
+        std::is_same<typename std::remove_cv<T1>::type, char*>::value) && (
+        std::is_same<typename std::remove_cv<T2>::type, const char*>::value ||
+        std::is_same<typename std::remove_cv<T2>::type, char*>::value);
+    static_assert(
+        !are_both_c_strings,
+        "This compares pointers not the string contents."
+        " Use (ASSERT|EXPECT)_STREQ instead.");
     return CmpHelperEQ(lhs_expression, rhs_expression, lhs, rhs);
   }



Reply all
Reply to author
Forward
0 new messages