From b2fa9f7b8e9dd0d82cb0ba2663617549e593d68c Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Mon, 7 Nov 2016 20:34:16 +0000 Subject: [PATCH 1/2] Disallow StringRef assignment from temporary std::strings. Similar to r283798, this prevents accidentally referring to temporary storage that goes out of scope by the end of the statement: someStringRef = getStringByValue(); someStringRef = (Twine("-") + otherString).str(); Note that once again the constructor still has this problem: StringRef someStringRef = getStringByValue(); because once again we occasionally rely on this in calls: takesStringRef(getStringByValue()); takesStringRef(Twine("-") + otherString); Still, it's a step. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@286139 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/ADT/StringRef.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/include/llvm/ADT/StringRef.h b/include/llvm/ADT/StringRef.h index a053b414a10..2c1883e5671 100644 --- a/include/llvm/ADT/StringRef.h +++ b/include/llvm/ADT/StringRef.h @@ -226,6 +226,15 @@ namespace llvm { return Data[Index]; } + /// Disallow accidental assignment from a temporary std::string. + /// + /// The declaration here is extra complicated so that `stringRef = {}` + /// and `stringRef = "abc"` continue to select the move assignment operator. + template + typename std::enable_if::value, + StringRef>::type & + operator=(T &&Str) = delete; + /// @} /// @name Type Conversions /// @{ From 25442627822d58f69d01945d65337dbb776f41d2 Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Mon, 7 Nov 2016 20:40:16 +0000 Subject: [PATCH 2/2] Add tests for r286139. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@286141 91177308-0d34-0410-b5e6-96231b3b80d8 --- unittests/ADT/StringRefTest.cpp | 36 +++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/unittests/ADT/StringRefTest.cpp b/unittests/ADT/StringRefTest.cpp index ca6b011d695..9c65a4b3589 100644 --- a/unittests/ADT/StringRefTest.cpp +++ b/unittests/ADT/StringRefTest.cpp @@ -32,6 +32,34 @@ std::ostream &operator<<(std::ostream &OS, } +// Check that we can't accidentally assign a temporary std::string to a +// StringRef. (Unfortunately we can't make use of the same thing with +// constructors.) +// +// Disable this check under MSVC; even MSVC 2015 isn't consistent between +// std::is_assignable and actually writing such an assignment. +#if !defined(_MSC_VER) +static_assert( + !std::is_assignable::value, + "Assigning from prvalue std::string"); +static_assert( + !std::is_assignable::value, + "Assigning from xvalue std::string"); +static_assert( + std::is_assignable::value, + "Assigning from lvalue std::string"); +static_assert( + std::is_assignable::value, + "Assigning from prvalue C string"); +static_assert( + std::is_assignable::value, + "Assigning from xvalue C string"); +static_assert( + std::is_assignable::value, + "Assigning from lvalue C string"); +#endif + + namespace { TEST(StringRefTest, Construction) { EXPECT_EQ("", StringRef()); @@ -40,6 +68,14 @@ TEST(StringRefTest, Construction) { EXPECT_EQ("hello", StringRef(std::string("hello"))); } +TEST(StringRefTest, EmptyInitializerList) { + StringRef S = {}; + EXPECT_TRUE(S.empty()); + + S = {}; + EXPECT_TRUE(S.empty()); +} + TEST(StringRefTest, Iteration) { StringRef S("hello"); const char *p = "hello";