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

David Blaikie dblaikie at gmail.com
Wed Aug 14 14:52:15 PDT 2013


On Wed, Aug 14, 2013 at 11:41 AM, Chris Lattner <clattner at apple.com> wrote:
>
> On Aug 13, 2013, at 9:55 AM, Puyan Lotfi <plotfi at apple.com> wrote:
>
>> Oops, forgot to reattach the patch file.
>
> I like this a lot, this seems like clearly the right way to go.  One concern: "ParseCommandLineOptions" is most commonly called on argc/argv coming into main, but that isn't always the case.  Clang for example, turns the -mllvm options into an an array and parses them (see clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp and clang/lib/CodeGen/BackendUtil.cpp).
>
> In this case, what keeps the memory from being deallocated underneath the StringRefs?

The former (ExecuteCompileRInvocation.cpp) seems to just leak the
strings, if I'm reading that right. The latter - yes, that seems to be
slightly problematic for this change & will need to be addressed.

Personally I'd like to see the caller have to own/persist the string
data as long as the API calls into LLVM - given the options are
globals anyway, there's no clear owner that LLVM could provide where
the strings could be given to (the LLVMContext, etc - are
insufficient).

>
> -Chris
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits




More information about the llvm-commits mailing list