[llvm] r215784 - [Support] Promote cl::StringSaver to a separate utility
Reid Kleckner
rnk at google.com
Fri Aug 22 10:42:10 PDT 2014
Looks good. Originally I made it virtual to try to handle the different
ownership models of LLVM and Clangs argument parsing, but it's less trouble
to just unify them.
On Fri, Aug 15, 2014 at 4:18 PM, Sean Silva <chisophugis at gmail.com> wrote:
> 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]));
> }
> }
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140822/66ef2988/attachment.html>
More information about the llvm-commits
mailing list