[llvm] r215784 - [Support] Promote cl::StringSaver to a separate utility
Rafael Auler
rafaelauler at gmail.com
Fri Aug 15 16:41:01 PDT 2014
A saveStringRef() member would be helpful.
On Fri, Aug 15, 2014 at 8: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/20140815/f27931cc/attachment.html>
More information about the llvm-commits
mailing list