[llvm] r215784 - [Support] Promote cl::StringSaver to a separate utility
Sean Silva
chisophugis at gmail.com
Fri Aug 15 16:18:33 PDT 2014
Author: silvas
Date: Fri Aug 15 18:18:33 2014
New Revision: 215784
URL: http://llvm.org/viewvc/llvm-project?rev=215784&view=rev
Log:
[Support] Promote cl::StringSaver to a separate utility
This class is generally useful.
In breaking it out, the primary change is that it has been made
non-virtual. It seems like being abstract led to there being 3 different
(2 in llvm + 1 in clang) concrete implementations which disagreed about
the ownership of the saved strings (see the manual call to free() in the
unittest StrDupSaver; yes this is different from the CommandLine.cpp
StrDupSaver which owns the stored strings; which is different from
Clang's StringSetSaver which just holds a reference to a
std::set<std::string> which owns the strings).
I've identified 2 other places in the
codebase that are open-coding this pattern:
memcpy(Alloc.Allocate<char>(strlen(S)+1), S, strlen(S)+1)
I'll be switching them over. They are
* llvm::sys::Process::GetArgumentVector
* The StringAllocator member of YAMLIO's Input class
This also will allow simplifying Clang's driver.cpp quite a bit.
Let me know if there are any other places that could benefit from
StringSaver. I'm also thinking of adding a saveStringRef member for
getting a stable StringRef.
Added:
llvm/trunk/include/llvm/Support/StringSaver.h
Modified:
llvm/trunk/include/llvm/Support/CommandLine.h
llvm/trunk/lib/Support/CommandLine.cpp
llvm/trunk/unittests/Support/CommandLineTest.cpp
Modified: llvm/trunk/include/llvm/Support/CommandLine.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/CommandLine.h?rev=215784&r1=215783&r2=215784&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/CommandLine.h (original)
+++ llvm/trunk/include/llvm/Support/CommandLine.h Fri Aug 15 18:18:33 2014
@@ -24,6 +24,7 @@
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/Twine.h"
#include "llvm/Support/Compiler.h"
+#include "llvm/Support/StringSaver.h"
#include <cassert>
#include <climits>
#include <cstdarg>
@@ -1772,15 +1773,6 @@ void getRegisteredOptions(StringMap<Opti
// Standalone command line processing utilities.
//
-/// \brief Saves strings in the inheritor's stable storage and returns a stable
-/// raw character pointer.
-class StringSaver {
- virtual void anchor();
-public:
- virtual const char *SaveString(const char *Str) = 0;
- virtual ~StringSaver() {}; // Pacify -Wnon-virtual-dtor.
-};
-
/// \brief Tokenizes a command line that can contain escapes and quotes.
//
/// The quoting rules match those used by GCC and other tools that use
Added: llvm/trunk/include/llvm/Support/StringSaver.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/StringSaver.h?rev=215784&view=auto
==============================================================================
--- llvm/trunk/include/llvm/Support/StringSaver.h (added)
+++ llvm/trunk/include/llvm/Support/StringSaver.h Fri Aug 15 18:18:33 2014
@@ -0,0 +1,33 @@
+//===- llvm/Support/StringSaver.h - Stable storage for strings --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_SUPPORT_STRINGSAVER_H
+#define LLVM_SUPPORT_STRINGSAVER_H
+
+#include "llvm/Support/Allocator.h"
+#include <cstring>
+
+namespace llvm {
+
+/// \brief Saves strings in stable storage that it owns.
+class StringSaver {
+ BumpPtrAllocator Alloc;
+
+public:
+ const char *saveCStr(const char *CStr) {
+ auto Len = std::strlen(CStr) + 1; // Don't forget the NUL!
+ char *Buf = Alloc.Allocate<char>(Len);
+ std::memcpy(Buf, CStr, Len);
+ return Buf;
+ }
+};
+
+} // end namespace llvm
+
+#endif
Modified: llvm/trunk/lib/Support/CommandLine.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/CommandLine.cpp?rev=215784&r1=215783&r2=215784&view=diff
==============================================================================
--- llvm/trunk/lib/Support/CommandLine.cpp (original)
+++ llvm/trunk/lib/Support/CommandLine.cpp Fri Aug 15 18:18:33 2014
@@ -76,7 +76,6 @@ void parser<double>::anchor() {}
void parser<float>::anchor() {}
void parser<std::string>::anchor() {}
void parser<char>::anchor() {}
-void StringSaver::anchor() {}
//===----------------------------------------------------------------------===//
@@ -509,7 +508,7 @@ void cl::TokenizeGNUCommandLine(StringRe
// End the token if this is whitespace.
if (isWhitespace(Src[I])) {
if (!Token.empty())
- NewArgv.push_back(Saver.SaveString(Token.c_str()));
+ NewArgv.push_back(Saver.saveCStr(Token.c_str()));
Token.clear();
continue;
}
@@ -520,7 +519,7 @@ void cl::TokenizeGNUCommandLine(StringRe
// Append the last token after hitting EOF with no whitespace.
if (!Token.empty())
- NewArgv.push_back(Saver.SaveString(Token.c_str()));
+ NewArgv.push_back(Saver.saveCStr(Token.c_str()));
}
/// Backslashes are interpreted in a rather complicated way in the Windows-style
@@ -593,7 +592,7 @@ void cl::TokenizeWindowsCommandLine(Stri
if (State == UNQUOTED) {
// Whitespace means the end of the token.
if (isWhitespace(Src[I])) {
- NewArgv.push_back(Saver.SaveString(Token.c_str()));
+ NewArgv.push_back(Saver.saveCStr(Token.c_str()));
Token.clear();
State = INIT;
continue;
@@ -625,7 +624,7 @@ void cl::TokenizeWindowsCommandLine(Stri
}
// Append the last token after hitting EOF with no whitespace.
if (!Token.empty())
- NewArgv.push_back(Saver.SaveString(Token.c_str()));
+ NewArgv.push_back(Saver.saveCStr(Token.c_str()));
}
static bool ExpandResponseFile(const char *FName, StringSaver &Saver,
@@ -691,25 +690,6 @@ bool cl::ExpandResponseFiles(StringSaver
return AllExpanded;
}
-namespace {
- class StrDupSaver : public StringSaver {
- std::vector<char*> Dups;
- public:
- ~StrDupSaver() {
- for (std::vector<char *>::iterator I = Dups.begin(), E = Dups.end();
- I != E; ++I) {
- char *Dup = *I;
- free(Dup);
- }
- }
- const char *SaveString(const char *Str) override {
- char *Dup = strdup(Str);
- Dups.push_back(Dup);
- return Dup;
- }
- };
-}
-
/// ParseEnvironmentOptions - An alternative entry point to the
/// CommandLine library, which allows you to read the program's name
/// from the caller (as PROGNAME) and its command-line arguments from
@@ -729,8 +709,8 @@ void cl::ParseEnvironmentOptions(const c
// Get program's "name", which we wouldn't know without the caller
// telling us.
SmallVector<const char *, 20> newArgv;
- StrDupSaver Saver;
- newArgv.push_back(Saver.SaveString(progName));
+ StringSaver Saver;
+ newArgv.push_back(Saver.saveCStr(progName));
// Parse the value of the environment variable into a "command line"
// and hand it off to ParseCommandLineOptions().
@@ -754,7 +734,7 @@ void cl::ParseCommandLineOptions(int arg
SmallVector<const char *, 20> newArgv;
for (int i = 0; i != argc; ++i)
newArgv.push_back(argv[i]);
- StrDupSaver Saver;
+ StringSaver Saver;
ExpandResponseFiles(Saver, TokenizeGNUCommandLine, newArgv);
argv = &newArgv[0];
argc = static_cast<int>(newArgv.size());
Modified: llvm/trunk/unittests/Support/CommandLineTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/CommandLineTest.cpp?rev=215784&r1=215783&r2=215784&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/CommandLineTest.cpp (original)
+++ llvm/trunk/unittests/Support/CommandLineTest.cpp Fri Aug 15 18:18:33 2014
@@ -146,26 +146,19 @@ TEST(CommandLineTest, UseOptionCategory)
"Category.";
}
-class StrDupSaver : public cl::StringSaver {
- const char *SaveString(const char *Str) override {
- return strdup(Str);
- }
-};
-
-typedef void ParserFunction(StringRef Source, llvm::cl::StringSaver &Saver,
+typedef void ParserFunction(StringRef Source, StringSaver &Saver,
SmallVectorImpl<const char *> &NewArgv);
void testCommandLineTokenizer(ParserFunction *parse, const char *Input,
const char *const Output[], size_t OutputSize) {
SmallVector<const char *, 0> Actual;
- StrDupSaver Saver;
+ StringSaver Saver;
parse(Input, Saver, Actual);
EXPECT_EQ(OutputSize, Actual.size());
for (unsigned I = 0, E = Actual.size(); I != E; ++I) {
if (I < OutputSize)
EXPECT_STREQ(Output[I], Actual[I]);
- free(const_cast<char *>(Actual[I]));
}
}
More information about the llvm-commits
mailing list