<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div><br></div><div>It wouldn’t be the common case. But it would reduce string copies resulting from parser::parse().</div><br><div><div>On Aug 15, 2013, at 5:01 PM, Reid Kleckner <<a href="mailto:rnk@google.com">rnk@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div dir="ltr">If CommandLine owns the strings, then why not just use cl::opt<std::string>?</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Aug 15, 2013 at 4:56 PM, Puyan Lotfi <span dir="ltr"><<a href="mailto:plotfi@apple.com" target="_blank">plotfi@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
This sounds like a workable compromise.<br>
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?<br>
<span class="HOEnZb"><font color="#888888"><br>
-Puyan<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
On Aug 15, 2013, at 1:47 PM, Chris Lattner <<a href="mailto:clattner@apple.com">clattner@apple.com</a>> wrote:<br>
<br>
><br>
> On Aug 14, 2013, at 4:06 PM, Puyan Lotfi <<a href="mailto:plotfi@apple.com">plotfi@apple.com</a>> wrote:<br>
><br>
>> Oh wow. I see that.<br>
>><br>
>> I am not completely sure how the char* arrays would get handled this way.<br>
>> I did not make any clang modifications in this patch (although since clang is using the cl library it is affected) but I noticed that the use of cl:: in the llvm source tree made use of global cl::opt variables that specify the opt type.<br>
>> I don’t see that in the files handling the -mllvm options; I’ll have to look at the CommandLine.cpp code a little more closely to be sure in this case that the char*’s aren’t being implicitly put into StringRefs that might have their data pointers deallocated after cl::ParseCommandLineOptions() is called.<br>
><br>
> After thinking about it for a bit, how about we go with this approach:<br>
><br>
> 1. Document ParseCommandLineOptions/ParseEnvironmentOptions to only work with strings that live forever in CommandLine.h<br>
> 2. Change clang to leak (or store globally) the strings that are affected.<br>
><br>
> I don't think it is worth designing a context system or anything like that to handle this case.<br>
><br>
> -Chris<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>
</blockquote></div><br></body></html>