PR10140 - StringPool's PooledStringPtr has non-const operator== causing bad OR-result

[dblaikie]

I suspect the primary bug here is that operator bool should be
explicit. Could you send a review for that too? (& verify that it
would've made the example given in PR10140 fail to compile, instead of
comparing booleans? (without the const patch applied))

In any case, I'll sign off on this change to add "const" - please go
ahead and commit.

[nikola]

Tests fail with original code. Tests don't compile if only conversion operator is marked as explicit. Const qualifying comparison operators gets the tests to compile and pass.

[dblaikie]

Oh, you don't need to create a separate class for the tests, just using "TEST(Grouping, Testname)" will work fine.

Please commit after you've made that change (removed "PooledStringPtrTest" class and changed from TEST_F to TEST macros).

Thanks for the work. For future reference I was suggesting that "adding explicit" could be done as a separate step, but it's fine together too. Thanks again.

[nikola]

What file should I use, they all seem to be very specialized?

[nikola]

I thought you were talking about file, not class. Ignore me, I'm stupid :)

[nikola]

r211244. Thanks.

include/llvm/Support/StringPool.h

     }
 
     inline const char *operator*() const { return begin(); }
-    inline operator bool() const { return S != nullptr; }
+    inline explicit operator bool() const { return S != nullptr; }
 
-    inline bool operator==(const PooledStringPtr &That) { return S == That.S; }
-    inline bool operator!=(const PooledStringPtr &That) { return S != That.S; }
+    inline bool operator==(const PooledStringPtr &That) const { return S == That.S; }
+    inline bool operator!=(const PooledStringPtr &That) const { return S != That.S; }
   };
 
 } // End llvm namespace

unittests/Support/CMakeLists.txt

   ProgramTest.cpp
   RegexTest.cpp
   SourceMgrTest.cpp
+  StringPool.cpp
   SwapByteOrderTest.cpp
   ThreadLocalTest.cpp
   TimeValueTest.cpp

unittests/Support/StringPool.cpp

+//===- llvm/unittest/Support/ThreadLocalTest.cpp - Therad Local tests   ---===//
+//
+//           The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Support/StringPool.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+namespace {
+
+class PooledStringPtrTest : public ::testing::Test {
+};
+
+TEST_F(PooledStringPtrTest, OperatorEquals) {
+  StringPool pool;
+  const PooledStringPtr a = pool.intern("a");
+  const PooledStringPtr b = pool.intern("b");
+  EXPECT_FALSE(a == b);
+}
+
+TEST_F(PooledStringPtrTest, OperatorNotEquals) {
+  StringPool pool;
+  const PooledStringPtr a = pool.intern("a");
+  const PooledStringPtr b = pool.intern("b");
+  EXPECT_TRUE(a != b);
+}
+
+}