[PATCH] Allow for the use of StringRef command line options instead of cl::opt<std::string>

[Puyan Lotfi]

Should this be put on hold until we have a better solution than replacing cl::opt<std::string> with cl::opt<StringRef>? I took another look at all the places that where ParseCommandLineOptions is not called with argc/argv from main. I found:

clang/tools/driver/cc1as_main.cpp
clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/Tooling/CommonOptionsParser.cpp

Among these cc1as_main.cpp and ExecuteCompilerInvocation.cpp leak memory regardless of the changes. CommonOptionsParser.cpp uses ParseCommandLineOptions but is called from unittests I already know are passing. Finally BackendUtil.cpp is still of concern if the options parsed are used by EmitAssemblyHelper::CreateTargetMachine()'s callers. I think a solution to this could be to move SmallVector<const char *, 16> BackendArgs; into the global scope as a work around and clear it out every time it is used when calling ParseCommandLineOptions.

Outside of this, it could just be a better solution to work out a way to not rely on static ctors to begin with.

Let me know your thoughts. Thanks.

-Puyan


On Aug 16, 2013, at 8:54 AM, David Blaikie <dblaikie at gmail.com> wrote:

> On Fri, Aug 16, 2013 at 8:43 AM, Chris Lattner <clattner at apple.com> wrote:
>> 
>> On Aug 15, 2013, at 10:57 PM, David Blaikie <dblaikie at gmail.com> wrote:
>> 
>>> On Thu, Aug 15, 2013 at 4:56 PM, Puyan Lotfi <plotfi at apple.com> wrote:
>>>> 
>>>> This sounds like a workable compromise.
>>>> To avoid leaking how about a CommandLine.cpp handled string pool that allocates copies on demand (specified maybe by an optional flag to ParseCommandLineOptions/ParseEnvironmentOptions) but also has the option of flushing/deallocating the pool?
>>> 
>>> I'm with Chris - I think it should be simpler to have
>>> ParseCommandLineOptions/CommandLine in general not be concerned with
>>> task of owning the strings. That should be the responsibility of the
>>> caller.
>> 
>> Also, I don't know if switching from cl::opt<std::string> to cl::opt<StringRef> is enough, but I'd really like for the cl::opts scattered through llvm to stop requiring static constructor/destructors.  This seems like a useful step along the way.
> 
> True - it shouldn't be hard to make StringRef's defoult ctor constexpr
> under C++11 - not sure what else is in a cl::opt that might present
> challenges to that kind of solution.