Teach Twine to support SmallString to save on .str() calls and later remove str() usage for these frequent cases.
Broke from http://reviews.llvm.org/D6336 for easier review.
Details
- Reviewers
dblaikie chfast dexonsmith
Diff Detail
Event Timeline
include/llvm/MC/MCContext.h | ||
---|---|---|
236–238 | If Twine can handle SmallString now, wouldn't it be better to reduce the number of overloads in this case? |
include/llvm/MC/MCContext.h | ||
---|---|---|
236–238 | The preferred function is GetOrCreateSymbol(StringRef) in which Name directly indexes Symbols[]. GetOrCreateSymbol(Twine) is a slower convenience function, using a temporary buffer to print out the Twine into a SmallString and then call the StringRef version. With MakeArgString the (StringRef) function is virtual and all other versions defer work to it. |
lib/MC/MCContext.cpp | ||
---|---|---|
176–177 | Do you think it is worth to check if Name isSingleStringRef? |
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.
tools/clang/lib/Sema/CodeCompleteConsumer.cpp | ||
---|---|---|
254 ↗ | (On Diff #22010) | I liked the argument name String better than T. I know the type has changed but CopyString copies strings. |
If Twine can handle SmallString now, wouldn't it be better to reduce the number of overloads in this case?