Implemented Paweł suggestion to use isSingleStringRef() to reduce the number of
overloads. GetOrCreateSymbol, CopyString and MakeArgString accept only Twine
now, virtual MakeArgString(StringRef) is still required and was renamed to
MakeArgStringRef to avoid ambiguity with the Twine version.
isSingleStringRef() is used through the toStringRef(), which was inlined to
give the compiler a chance at optimizing out the Twine
construction-isSingleStringRef testing-getSingleStringRef code sequence for
simple arguments.
Removed function names from Twine.h comments.
http://reviews.llvm.org/D6372
Files:
include/llvm/ADT/Twine.h
include/llvm/MC/MCContext.h
include/llvm/Option/ArgList.h
lib/MC/MCContext.cpp
lib/Option/ArgList.cpp
lib/Support/Twine.cpp
tools/clang/include/clang/Sema/CodeCompleteConsumer.h
tools/clang/lib/Sema/CodeCompleteConsumer.cpp
unittests/ADT/TwineTest.cpp
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
Index: include/llvm/ADT/Twine.h
===================================================================
--- include/llvm/ADT/Twine.h
+++ include/llvm/ADT/Twine.h
@@ -10,16 +10,14 @@
#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"
#include <cassert>
#include <string>
namespace llvm {
- template <typename T>
- class SmallVectorImpl;
- class StringRef;
class raw_ostream;
/// Twine - A lightweight data structure for efficiently representing the
@@ -100,6 +98,9 @@
/// 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 @@
const char *cString;
const std::string *stdString;
const StringRef *stringRef;
+ const SmallVectorImpl<char> *smallString;
char character;
unsigned int decUI;
int decI;
@@ -183,32 +185,32 @@
/// 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 @@
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 @@
assert(isValid() && "Invalid twine!");
}
+ /// Construct from a SmallString.
+ /*implicit*/ Twine(const SmallVectorImpl<char> &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,22 +394,23 @@
/// @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;
switch (getLHSKind()) {
case EmptyKind:
case CStringKind:
case StdStringKind:
case StringRefKind:
+ case SmallStringKind:
return true;
default:
return false;
@@ -417,35 +427,40 @@
/// @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<char> &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()) {
default: llvm_unreachable("Out of sync with isSingleStringRef");
case EmptyKind: return StringRef();
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<char> &Out) const;
+ StringRef toStringRef(SmallVectorImpl<char> &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<char> &Out) const;
Index: include/llvm/MC/MCContext.h
===================================================================
--- include/llvm/MC/MCContext.h
+++ 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 @@
class MCRegisterInfo;
class MCLineSection;
class SMLoc;
- class StringRef;
- class Twine;
class MCSectionMachO;
class MCSectionELF;
class MCSectionCOFF;
@@ -230,7 +229,6 @@
/// 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);
Index: include/llvm/Option/ArgList.h
===================================================================
--- include/llvm/Option/ArgList.h
+++ 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 <list>
@@ -277,16 +279,13 @@
/// @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 @@
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 @@
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.
Index: lib/MC/MCContext.cpp
===================================================================
--- lib/MC/MCContext.cpp
+++ lib/MC/MCContext.cpp
@@ -98,13 +98,15 @@
// 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 @@
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)
Index: lib/Option/ArgList.cpp
===================================================================
--- lib/Option/ArgList.cpp
+++ lib/Option/ArgList.cpp
@@ -285,11 +285,6 @@
(*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 @@
return Index0;
}
-const char *InputArgList::MakeArgString(StringRef Str) const {
+const char *InputArgList::MakeArgStringRef(StringRef Str) const {
return getArgString(MakeIndex(Str));
}
@@ -346,7 +341,7 @@
DerivedArgList::~DerivedArgList() {}
-const char *DerivedArgList::MakeArgString(StringRef Str) const {
+const char *DerivedArgList::MakeArgStringRef(StringRef Str) const {
return BaseArgs.MakeArgString(Str);
}
Index: lib/Support/Twine.cpp
===================================================================
--- lib/Support/Twine.cpp
+++ lib/Support/Twine.cpp
@@ -28,13 +28,6 @@
print(OS);
}
-StringRef Twine::toStringRef(SmallVectorImpl<char> &Out) const {
- if (isSingleStringRef())
- return getSingleStringRef();
- toVector(Out);
- return StringRef(Out.data(), Out.size());
-}
-
StringRef Twine::toNullTerminatedStringRef(SmallVectorImpl<char> &Out) const {
if (isUnary()) {
switch (getLHSKind()) {
@@ -72,6 +65,9 @@
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 @@
OS << "stringref:\""
<< Ptr.stringRef << "\"";
break;
+ case Twine::SmallStringKind:
+ OS << "smallstring:\""
+ << *Ptr.smallString << "\"";
+ break;
case Twine::CharKind:
OS << "char:\"" << Ptr.character << "\"";
break;
Index: unittests/ADT/TwineTest.cpp
===================================================================
--- unittests/ADT/TwineTest.cpp
+++ unittests/ADT/TwineTest.cpp
@@ -29,6 +29,7 @@
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 @@
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,13 +77,18 @@
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) {
SmallString<8> storage;
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
Index: tools/clang/include/clang/Sema/CodeCompleteConsumer.h
===================================================================
--- tools/clang/include/clang/Sema/CodeCompleteConsumer.h
+++ tools/clang/include/clang/Sema/CodeCompleteConsumer.h
@@ -499,20 +499,7 @@
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 &T);
};
/// \brief Allocator for a cached set of global code completions.
Index: tools/clang/lib/Sema/CodeCompleteConsumer.cpp
===================================================================
--- tools/clang/lib/Sema/CodeCompleteConsumer.cpp
+++ tools/clang/lib/Sema/CodeCompleteConsumer.cpp
@@ -251,21 +251,18 @@
return nullptr;
}
-const char *CodeCompletionAllocator::CopyString(StringRef String) {
+const char *CodeCompletionAllocator::CopyString(const Twine &T) {
+ SmallString<128> Data;
+ StringRef String = T.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.
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) {
- // 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));
-}
-
StringRef CodeCompletionTUInfo::getParentName(const DeclContext *DC) {
const NamedDecl *ND = dyn_cast<NamedDecl>(DC);
if (!ND)
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits