From 1ee89fc432ded040593e59844efa621b4189a7c3 Mon Sep 17 00:00:00 2001 From: Yaron Keren Date: Tue, 17 Mar 2015 09:51:17 +0000 Subject: [PATCH] Teach Twine to support SmallString. Enable removing .str() member calls for these frequent cases. http://reviews.llvm.org/D6372 llvm-svn: 232465 --- clang/include/clang/Sema/CodeCompleteConsumer.h | 15 +----- clang/lib/Sema/CodeCompleteConsumer.cpp | 17 +++--- llvm/include/llvm/ADT/Twine.h | 71 +++++++++++++++---------- llvm/include/llvm/MC/MCContext.h | 4 +- llvm/include/llvm/Option/ArgList.h | 19 ++++--- llvm/lib/MC/MCContext.cpp | 15 +++--- llvm/lib/Option/ArgList.cpp | 9 +--- llvm/lib/Support/Twine.cpp | 14 ++--- llvm/unittests/ADT/TwineTest.cpp | 10 ++++ 9 files changed, 86 insertions(+), 88 deletions(-) diff --git a/clang/include/clang/Sema/CodeCompleteConsumer.h b/clang/include/clang/Sema/CodeCompleteConsumer.h index 647eb8b..d24a925 100644 --- a/clang/include/clang/Sema/CodeCompleteConsumer.h +++ b/clang/include/clang/Sema/CodeCompleteConsumer.h @@ -499,20 +499,7 @@ public: class CodeCompletionAllocator : public llvm::BumpPtrAllocator { public: /// \brief Copy the given string into this allocator. - const char *CopyString(StringRef String); - - /// \brief Copy the given string into this allocator. - const char *CopyString(Twine String); - - // \brief Copy the given string into this allocator. - const char *CopyString(const char *String) { - return CopyString(StringRef(String)); - } - - /// \brief Copy the given string into this allocator. - const char *CopyString(const std::string &String) { - return CopyString(StringRef(String)); - } + const char *CopyString(const Twine &String); }; /// \brief Allocator for a cached set of global code completions. diff --git a/clang/lib/Sema/CodeCompleteConsumer.cpp b/clang/lib/Sema/CodeCompleteConsumer.cpp index 92d2aeb..69ae4f0 100644 --- a/clang/lib/Sema/CodeCompleteConsumer.cpp +++ b/clang/lib/Sema/CodeCompleteConsumer.cpp @@ -251,19 +251,16 @@ const char *CodeCompletionString::getTypedText() const { return nullptr; } -const char *CodeCompletionAllocator::CopyString(StringRef String) { - char *Mem = (char *)Allocate(String.size() + 1, 1); - std::copy(String.begin(), String.end(), Mem); - Mem[String.size()] = 0; - return Mem; -} - -const char *CodeCompletionAllocator::CopyString(Twine String) { +const char *CodeCompletionAllocator::CopyString(const Twine &String) { + SmallString<128> Data; + StringRef Ref = String.toStringRef(Data); // FIXME: It would be more efficient to teach Twine to tell us its size and // then add a routine there to fill in an allocated char* with the contents // of the string. - SmallString<128> Data; - return CopyString(String.toStringRef(Data)); + char *Mem = (char *)Allocate(Ref.size() + 1, 1); + std::copy(Ref.begin(), Ref.end(), Mem); + Mem[Ref.size()] = 0; + return Mem; } StringRef CodeCompletionTUInfo::getParentName(const DeclContext *DC) { diff --git a/llvm/include/llvm/ADT/Twine.h b/llvm/include/llvm/ADT/Twine.h index c986cf3..51bb18d 100644 --- a/llvm/include/llvm/ADT/Twine.h +++ b/llvm/include/llvm/ADT/Twine.h @@ -10,6 +10,7 @@ #ifndef LLVM_ADT_TWINE_H #define LLVM_ADT_TWINE_H +#include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/DataTypes.h" #include "llvm/Support/ErrorHandling.h" @@ -17,9 +18,6 @@ #include namespace llvm { - template - class SmallVectorImpl; - class StringRef; class raw_ostream; /// Twine - A lightweight data structure for efficiently representing the @@ -100,6 +98,9 @@ namespace llvm { /// A pointer to a StringRef instance. StringRefKind, + /// A pointer to a SmallString instance. + SmallStringKind, + /// A char value reinterpreted as a pointer, to render as a character. CharKind, @@ -136,6 +137,7 @@ namespace llvm { const char *cString; const std::string *stdString; const StringRef *stringRef; + const SmallVectorImpl *smallString; char character; unsigned int decUI; int decI; @@ -183,32 +185,32 @@ namespace llvm { /// when concatenating might cause undefined behavior or stack corruptions Twine &operator=(const Twine &Other) = delete; - /// isNull - Check for the null twine. + /// Check for the null twine. bool isNull() const { return getLHSKind() == NullKind; } - /// isEmpty - Check for the empty twine. + /// Check for the empty twine. bool isEmpty() const { return getLHSKind() == EmptyKind; } - /// isNullary - Check if this is a nullary twine (null or empty). + /// Check if this is a nullary twine (null or empty). bool isNullary() const { return isNull() || isEmpty(); } - /// isUnary - Check if this is a unary twine. + /// Check if this is a unary twine. bool isUnary() const { return getRHSKind() == EmptyKind && !isNullary(); } - /// isBinary - Check if this is a binary twine. + /// Check if this is a binary twine. bool isBinary() const { return getLHSKind() != NullKind && getRHSKind() != EmptyKind; } - /// isValid - Check if this is a valid twine (satisfying the invariants on + /// Check if this is a valid twine (satisfying the invariants on /// order and number of arguments). bool isValid() const { // Nullary twines always have Empty on the RHS. @@ -234,16 +236,16 @@ namespace llvm { return true; } - /// getLHSKind - Get the NodeKind of the left-hand side. + /// Get the NodeKind of the left-hand side. NodeKind getLHSKind() const { return LHSKind; } - /// getRHSKind - Get the NodeKind of the right-hand side. + /// Get the NodeKind of the right-hand side. NodeKind getRHSKind() const { return RHSKind; } - /// printOneChild - Print one child from a twine. + /// Print one child from a twine. void printOneChild(raw_ostream &OS, Child Ptr, NodeKind Kind) const; - /// printOneChildRepr - Print the representation of one child from a twine. + /// Print the representation of one child from a twine. void printOneChildRepr(raw_ostream &OS, Child Ptr, NodeKind Kind) const; @@ -288,6 +290,13 @@ namespace llvm { assert(isValid() && "Invalid twine!"); } + /// Construct from a SmallString. + /*implicit*/ Twine(const SmallVectorImpl &Str) + : LHSKind(SmallStringKind), RHSKind(EmptyKind) { + LHS.smallString = &Str; + assert(isValid() && "Invalid twine!"); + } + /// Construct from a char. explicit Twine(char Val) : LHSKind(CharKind), RHSKind(EmptyKind) { @@ -385,14 +394,14 @@ namespace llvm { /// @name Predicate Operations /// @{ - /// isTriviallyEmpty - Check if this twine is trivially empty; a false - /// return value does not necessarily mean the twine is empty. + /// Check if this twine is trivially empty; a false return value does not + /// necessarily mean the twine is empty. bool isTriviallyEmpty() const { return isNullary(); } - /// isSingleStringRef - Return true if this twine can be dynamically - /// accessed as a single StringRef value with getSingleStringRef(). + /// Return true if this twine can be dynamically accessed as a single + /// StringRef value with getSingleStringRef(). bool isSingleStringRef() const { if (getRHSKind() != EmptyKind) return false; @@ -401,6 +410,7 @@ namespace llvm { case CStringKind: case StdStringKind: case StringRefKind: + case SmallStringKind: return true; default: return false; @@ -417,15 +427,14 @@ namespace llvm { /// @name Output & Conversion. /// @{ - /// str - Return the twine contents as a std::string. + /// Return the twine contents as a std::string. std::string str() const; - /// toVector - Write the concatenated string into the given SmallString or - /// SmallVector. + /// Write the concatenated string into the given SmallString or SmallVector. void toVector(SmallVectorImpl &Out) const; - /// getSingleStringRef - This returns the twine as a single StringRef. This - /// method is only valid if isSingleStringRef() is true. + /// This returns the twine as a single StringRef. This method is only valid + /// if isSingleStringRef() is true. StringRef getSingleStringRef() const { assert(isSingleStringRef() &&"This cannot be had as a single stringref!"); switch (getLHSKind()) { @@ -434,18 +443,24 @@ namespace llvm { case CStringKind: return StringRef(LHS.cString); case StdStringKind: return StringRef(*LHS.stdString); case StringRefKind: return *LHS.stringRef; + case SmallStringKind: + return StringRef(LHS.smallString->data(), LHS.smallString->size()); } } - /// toStringRef - This returns the twine as a single StringRef if it can be + /// This returns the twine as a single StringRef if it can be /// represented as such. Otherwise the twine is written into the given /// SmallVector and a StringRef to the SmallVector's data is returned. - StringRef toStringRef(SmallVectorImpl &Out) const; + StringRef toStringRef(SmallVectorImpl &Out) const { + if (isSingleStringRef()) + return getSingleStringRef(); + toVector(Out); + return StringRef(Out.data(), Out.size()); + } - /// toNullTerminatedStringRef - This returns the twine as a single null - /// terminated StringRef if it can be represented as such. Otherwise the - /// twine is written into the given SmallVector and a StringRef to the - /// SmallVector's data is returned. + /// This returns the twine as a single null terminated StringRef if it + /// can be represented as such. Otherwise the twine is written into the + /// given SmallVector and a StringRef to the SmallVector's data is returned. /// /// The returned StringRef's size does not include the null terminator. StringRef toNullTerminatedStringRef(SmallVectorImpl &Out) const; diff --git a/llvm/include/llvm/MC/MCContext.h b/llvm/include/llvm/MC/MCContext.h index fc2ce2b..20fad95 100644 --- a/llvm/include/llvm/MC/MCContext.h +++ b/llvm/include/llvm/MC/MCContext.h @@ -15,6 +15,7 @@ #include "llvm/ADT/SmallString.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringMap.h" +#include "llvm/ADT/Twine.h" #include "llvm/MC/MCDwarf.h" #include "llvm/MC/SectionKind.h" #include "llvm/Support/Allocator.h" @@ -36,8 +37,6 @@ namespace llvm { class MCRegisterInfo; class MCLineSection; class SMLoc; - class StringRef; - class Twine; class MCSectionMachO; class MCSectionELF; class MCSectionCOFF; @@ -230,7 +229,6 @@ namespace llvm { /// return it. If not, create a forward reference and return it. /// /// @param Name - The symbol name, which must be unique across all symbols. - MCSymbol *GetOrCreateSymbol(StringRef Name); MCSymbol *GetOrCreateSymbol(const Twine &Name); MCSymbol *getOrCreateSectionSymbol(const MCSectionELF &Section); diff --git a/llvm/include/llvm/Option/ArgList.h b/llvm/include/llvm/Option/ArgList.h index 29371cc..ab3f95d 100644 --- a/llvm/include/llvm/Option/ArgList.h +++ b/llvm/include/llvm/Option/ArgList.h @@ -11,7 +11,9 @@ #define LLVM_OPTION_ARGLIST_H #include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringRef.h" +#include "llvm/ADT/Twine.h" #include "llvm/Option/OptSpecifier.h" #include "llvm/Option/Option.h" #include @@ -277,16 +279,13 @@ public: /// @name Arg Synthesis /// @{ - /// MakeArgString - Construct a constant string pointer whose + /// Construct a constant string pointer whose /// lifetime will match that of the ArgList. - virtual const char *MakeArgString(StringRef Str) const = 0; - const char *MakeArgString(const char *Str) const { - return MakeArgString(StringRef(Str)); + virtual const char *MakeArgStringRef(StringRef Str) const = 0; + const char *MakeArgString(const Twine &Str) const { + SmallString<256> Buf; + return MakeArgStringRef(Str.toStringRef(Buf)); } - const char *MakeArgString(std::string Str) const { - return MakeArgString(StringRef(Str)); - } - const char *MakeArgString(const Twine &Str) const; /// \brief Create an arg string for (\p LHS + \p RHS), reusing the /// string at \p Index if possible. @@ -336,7 +335,7 @@ public: unsigned MakeIndex(StringRef String0, StringRef String1) const; using ArgList::MakeArgString; - const char *MakeArgString(StringRef Str) const override; + const char *MakeArgStringRef(StringRef Str) const override; /// @} }; @@ -374,7 +373,7 @@ public: void AddSynthesizedArg(Arg *A); using ArgList::MakeArgString; - const char *MakeArgString(StringRef Str) const override; + const char *MakeArgStringRef(StringRef Str) const override; /// AddFlagArg - Construct a new FlagArg for the given option \p Id and /// append it to the argument list. diff --git a/llvm/lib/MC/MCContext.cpp b/llvm/lib/MC/MCContext.cpp index 82f1da2..608d958 100644 --- a/llvm/lib/MC/MCContext.cpp +++ b/llvm/lib/MC/MCContext.cpp @@ -98,13 +98,15 @@ void MCContext::reset() { // Symbol Manipulation //===----------------------------------------------------------------------===// -MCSymbol *MCContext::GetOrCreateSymbol(StringRef Name) { - assert(!Name.empty() && "Normal symbols cannot be unnamed!"); +MCSymbol *MCContext::GetOrCreateSymbol(const Twine &Name) { + SmallString<128> NameSV; + StringRef NameRef = Name.toStringRef(NameSV); - MCSymbol *&Sym = Symbols[Name]; + assert(!NameRef.empty() && "Normal symbols cannot be unnamed!"); + MCSymbol *&Sym = Symbols[NameRef]; if (!Sym) - Sym = CreateSymbol(Name); + Sym = CreateSymbol(NameRef); return Sym; } @@ -168,11 +170,6 @@ MCSymbol *MCContext::createTempSymbol(const Twine &Name) { return CreateSymbol(NameSV); } -MCSymbol *MCContext::GetOrCreateSymbol(const Twine &Name) { - SmallString<128> NameSV; - return GetOrCreateSymbol(Name.toStringRef(NameSV)); -} - MCSymbol *MCContext::CreateLinkerPrivateTempSymbol() { SmallString<128> NameSV; raw_svector_ostream(NameSV) diff --git a/llvm/lib/Option/ArgList.cpp b/llvm/lib/Option/ArgList.cpp index 2de55ce..f2547d6 100644 --- a/llvm/lib/Option/ArgList.cpp +++ b/llvm/lib/Option/ArgList.cpp @@ -285,11 +285,6 @@ void ArgList::ClaimAllArgs() const { (*it)->claim(); } -const char *ArgList::MakeArgString(const Twine &T) const { - SmallString<256> Str; - return MakeArgString(T.toStringRef(Str)); -} - const char *ArgList::GetOrMakeJoinedArgString(unsigned Index, StringRef LHS, StringRef RHS) const { @@ -334,7 +329,7 @@ unsigned InputArgList::MakeIndex(StringRef String0, return Index0; } -const char *InputArgList::MakeArgString(StringRef Str) const { +const char *InputArgList::MakeArgStringRef(StringRef Str) const { return getArgString(MakeIndex(Str)); } @@ -345,7 +340,7 @@ DerivedArgList::DerivedArgList(const InputArgList &BaseArgs) DerivedArgList::~DerivedArgList() {} -const char *DerivedArgList::MakeArgString(StringRef Str) const { +const char *DerivedArgList::MakeArgStringRef(StringRef Str) const { return BaseArgs.MakeArgString(Str); } diff --git a/llvm/lib/Support/Twine.cpp b/llvm/lib/Support/Twine.cpp index 56ed964..d2cc75b 100644 --- a/llvm/lib/Support/Twine.cpp +++ b/llvm/lib/Support/Twine.cpp @@ -28,13 +28,6 @@ void Twine::toVector(SmallVectorImpl &Out) const { print(OS); } -StringRef Twine::toStringRef(SmallVectorImpl &Out) const { - if (isSingleStringRef()) - return getSingleStringRef(); - toVector(Out); - return StringRef(Out.data(), Out.size()); -} - StringRef Twine::toNullTerminatedStringRef(SmallVectorImpl &Out) const { if (isUnary()) { switch (getLHSKind()) { @@ -72,6 +65,9 @@ void Twine::printOneChild(raw_ostream &OS, Child Ptr, case Twine::StringRefKind: OS << *Ptr.stringRef; break; + case Twine::SmallStringKind: + OS << *Ptr.smallString; + break; case Twine::CharKind: OS << Ptr.character; break; @@ -122,6 +118,10 @@ void Twine::printOneChildRepr(raw_ostream &OS, Child Ptr, OS << "stringref:\"" << Ptr.stringRef << "\""; break; + case Twine::SmallStringKind: + OS << "smallstring:\"" + << *Ptr.smallString << "\""; + break; case Twine::CharKind: OS << "char:\"" << Ptr.character << "\""; break; diff --git a/llvm/unittests/ADT/TwineTest.cpp b/llvm/unittests/ADT/TwineTest.cpp index 39d3b56..9683e97 100644 --- a/llvm/unittests/ADT/TwineTest.cpp +++ b/llvm/unittests/ADT/TwineTest.cpp @@ -29,6 +29,7 @@ TEST(TwineTest, Construction) { EXPECT_EQ("hi", Twine(StringRef("hi")).str()); EXPECT_EQ("hi", Twine(StringRef(std::string("hi"))).str()); EXPECT_EQ("hi", Twine(StringRef("hithere", 2)).str()); + EXPECT_EQ("hi", Twine(SmallString<4>("hi")).str()); } TEST(TwineTest, Numbers) { @@ -62,6 +63,10 @@ TEST(TwineTest, Concat) { repr(Twine("hi").concat(Twine()))); EXPECT_EQ("(Twine cstring:\"hi\" empty)", repr(Twine().concat(Twine("hi")))); + EXPECT_EQ("(Twine smallstring:\"hi\" empty)", + repr(Twine().concat(Twine(SmallString<5>("hi"))))); + EXPECT_EQ("(Twine smallstring:\"hey\" cstring:\"there\")", + repr(Twine(SmallString<7>("hey")).concat(Twine("there")))); // Concatenation of unary ropes. EXPECT_EQ("(Twine cstring:\"a\" cstring:\"b\")", @@ -72,6 +77,8 @@ TEST(TwineTest, Concat) { repr(Twine("a").concat(Twine("b")).concat(Twine("c")))); EXPECT_EQ("(Twine cstring:\"a\" rope:(Twine cstring:\"b\" cstring:\"c\"))", repr(Twine("a").concat(Twine("b").concat(Twine("c"))))); + EXPECT_EQ("(Twine cstring:\"a\" rope:(Twine smallstring:\"b\" cstring:\"c\"))", + repr(Twine("a").concat(Twine(SmallString<3>("b")).concat(Twine("c"))))); } TEST(TwineTest, toNullTerminatedStringRef) { @@ -79,6 +86,9 @@ TEST(TwineTest, toNullTerminatedStringRef) { EXPECT_EQ(0, *Twine("hello").toNullTerminatedStringRef(storage).end()); EXPECT_EQ(0, *Twine(StringRef("hello")).toNullTerminatedStringRef(storage).end()); + EXPECT_EQ(0, *Twine(SmallString<11>("hello")) + .toNullTerminatedStringRef(storage) + .end()); } // I suppose linking in the entire code generator to add a unit test to check -- 2.7.4