[PATCH] Allow for the use of StringRef command line options instead of cl::opt<std::string>
plotfi at apple.com
Fri Oct 4 11:12:35 PDT 2013
On Oct 4, 2013, at 10:55 AM, Chandler Carruth <chandlerc at google.com> wrote:
> On Fri, Oct 4, 2013 at 1:52 PM, Puyan Lotfi <plotfi at apple.com> wrote:
> On Oct 4, 2013, at 10:43 AM, Chandler Carruth <chandlerc at google.com> wrote:
>> On Fri, Oct 4, 2013 at 1:39 PM, Puyan Lotfi <plotfi at apple.com> wrote:
>> I like this solution but do you think there should be some way to tell the library (optional parameter, or a separate API function) to use the argv data directly instead of allocating memory on the context. Seems like it would be good to not hurt performance of the common case.
>> I don't think there is a common case where performance is a concern. Why?
>> 1) We're parsing commandline flags in an inherently race-prone way, and for flags which are explicitly for debugging and custom manipulations only.
>> 2) It can only happen once per LLVMContext spin up / tear down
> I mean the common case where ParseCommandLineOptions is being called from main and being passed argv directly. Wouldn’t it be better to not allocate duplicates inside LLVMContext in this case? But yeah, otherwise I like the idea of putting strings into LLVMContext.
> The question to ask here is what problem does it solve.
> If it is being called from main, it will be called exactly once per execution. That means that it will do at most a number of allocations corresponding to the number of command line arguments passed to the binary. I think that malloc overhead is not worth *any* complexity in the system, and certainly not a bug-prone interface.
It still seems to me that there should be the option of passing argv directly without having copies. Most of the tools don’t need duplicates managed and a lot of clang at the moment is leaking the -mllvm args as well so that’s the same case. I think it would be good to be able to call ParseCommandLineOptioms(argc, argv, “Foo tool\n”, false /* makePersistentCopies=false */); (where the default behavior is to make dupes) and just not have any dupes generated in the current context as an option.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits