[llvm] r211244 - PR10140 - StringPool's PooledStringPtr has non-const operator== causing bad OR-result.

[Nikola Smiljanic]

Thanks, I'm still cloning the repo from a VM :P


On Thu, Jun 19, 2014 at 10:51 AM, David Blaikie <dblaikie at gmail.com> wrote:

> r211245
>
> On Wed, Jun 18, 2014 at 5:49 PM, David Blaikie <dblaikie at gmail.com> wrote:
> > I think I've got it - it's just some excess (& inefficient) operator==
> > overloads, by the looks of things.
> >
> > On Wed, Jun 18, 2014 at 5:47 PM, Nikola Smiljanic <popizdeh at gmail.com>
> wrote:
> >> I'm panicking :) but it seems to be only in tools/extra so I'll try to
> fix
> >> this quickly.
> >>
> >>
> >> On Thu, Jun 19, 2014 at 10:46 AM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >>>
> >>> Hm,m not even in clang, I think - just clang-tools-extra.
> >>>
> >>> On Wed, Jun 18, 2014 at 5:45 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >>> > On Wed, Jun 18, 2014 at 5:26 PM, Nikola Smiljanic <
> popizdeh at gmail.com>
> >>> > wrote:
> >>> >> Author: nikola
> >>> >> Date: Wed Jun 18 19:26:49 2014
> >>> >> New Revision: 211244
> >>> >>
> >>> >> URL: http://llvm.org/viewvc/llvm-project?rev=211244&view=rev
> >>> >> Log:
> >>> >> PR10140 - StringPool's PooledStringPtr has non-const operator==
> causing
> >>> >> bad OR-result.
> >>> >>
> >>> >> Mark conversion operator explicit and const qualify comparison
> >>> >> operators.
> >>> >
> >>> > Looks like there's some breakage in Clang due to this (looks like
> >>> > making the bool conversion explicit found some more bugs). Not sure
> if
> >>> > you can/want to fix them quickly, or revert then fix.
> >>> >
> >>> > - David
> >>> >
> >>> >>
> >>> >> Added:
> >>> >>     llvm/trunk/unittests/Support/StringPool.cpp
> >>> >> Modified:
> >>> >>     llvm/trunk/include/llvm/Support/StringPool.h
> >>> >>     llvm/trunk/unittests/Support/CMakeLists.txt
> >>> >>
> >>> >> Modified: llvm/trunk/include/llvm/Support/StringPool.h
> >>> >> URL:
> >>> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/StringPool.h?rev=211244&r1=211243&r2=211244&view=diff
> >>> >>
> >>> >>
> ==============================================================================
> >>> >> --- llvm/trunk/include/llvm/Support/StringPool.h (original)
> >>> >> +++ llvm/trunk/include/llvm/Support/StringPool.h Wed Jun 18 19:26:49
> >>> >> 2014
> >>> >> @@ -128,10 +128,10 @@ namespace llvm {
> >>> >>      }
> >>> >>
> >>> >>      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
> >>> >>
> >>> >> Modified: llvm/trunk/unittests/Support/CMakeLists.txt
> >>> >> URL:
> >>> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/CMakeLists.txt?rev=211244&r1=211243&r2=211244&view=diff
> >>> >>
> >>> >>
> ==============================================================================
> >>> >> --- llvm/trunk/unittests/Support/CMakeLists.txt (original)
> >>> >> +++ llvm/trunk/unittests/Support/CMakeLists.txt Wed Jun 18 19:26:49
> >>> >> 2014
> >>> >> @@ -30,6 +30,7 @@ add_llvm_unittest(SupportTests
> >>> >>    ProgramTest.cpp
> >>> >>    RegexTest.cpp
> >>> >>    SourceMgrTest.cpp
> >>> >> +  StringPool.cpp
> >>> >>    SwapByteOrderTest.cpp
> >>> >>    ThreadLocalTest.cpp
> >>> >>    TimeValueTest.cpp
> >>> >>
> >>> >> Added: llvm/trunk/unittests/Support/StringPool.cpp
> >>> >> URL:
> >>> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/StringPool.cpp?rev=211244&view=auto
> >>> >>
> >>> >>
> ==============================================================================
> >>> >> --- llvm/trunk/unittests/Support/StringPool.cpp (added)
> >>> >> +++ llvm/trunk/unittests/Support/StringPool.cpp Wed Jun 18 19:26:49
> >>> >> 2014
> >>> >> @@ -0,0 +1,31 @@
> >>> >> +//===- 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 {
> >>> >> +
> >>> >> +TEST(PooledStringPtrTest, OperatorEquals) {
> >>> >> +  StringPool pool;
> >>> >> +  const PooledStringPtr a = pool.intern("a");
> >>> >> +  const PooledStringPtr b = pool.intern("b");
> >>> >> +  EXPECT_FALSE(a == b);
> >>> >> +}
> >>> >> +
> >>> >> +TEST(PooledStringPtrTest, OperatorNotEquals) {
> >>> >> +  StringPool pool;
> >>> >> +  const PooledStringPtr a = pool.intern("a");
> >>> >> +  const PooledStringPtr b = pool.intern("b");
> >>> >> +  EXPECT_TRUE(a != b);
> >>> >> +}
> >>> >> +
> >>> >> +}
> >>> >>
> >>> >>
> >>> >> _______________________________________________
> >>> >> llvm-commits mailing list
> >>> >> llvm-commits at cs.uiuc.edu
> >>> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >>
> >>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140619/17d74fa3/attachment.html>