<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br>
Message: 3<br>
Date: Fri, 1 Jul 2011 12:09:04 -0400<br>
From: Zach Wheeler <<a href="mailto:zachw.foss@gmail.com">zachw.foss@gmail.com</a>><br>
Subject: Re: [cfe-commits] [PATCH] StringRef'ize<br>
        clang::driver::Option::getName()<br>
To: Chris Lattner <<a href="mailto:clattner@apple.com">clattner@apple.com</a>><br>
Cc: <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
Message-ID: <<a href="mailto:BANLkTimoXHdSP9Vz0mStwt_xHtORoPtqwQ@mail.gmail.com">BANLkTimoXHdSP9Vz0mStwt_xHtORoPtqwQ@mail.gmail.com</a>><br>
Content-Type: text/plain; charset="iso-8859-1"<br>
<br>
I forgot to initialize NameLen. :-/<br>
<br>
I stored 'Name' as start+size because I saw that pattern in<br>
lib/Basic/DiagnosticID.h, and assumed it was expected. Storing a StringRef<br>
makes more sense to me. Here's an updated patch. Clang builds successfully,<br>
and hopefully the tests will pass.<br>
<br>
ZJ<br>
<br>
On Thu, Jun 30, 2011 at 4:04 PM, Chris Lattner <<a href="mailto:clattner@apple.com">clattner@apple.com</a>> wrote:<br>
<br>
><br>
> On Jun 30, 2011, at 11:47 AM, Zach Wheeler wrote:<br>
><br>
> > This patch changes the return type of Option::getName() from const char*<br>
> to StringRef, doing away with the need for several calls to strlen.<br>
> > Clang builds fine with these changes.<br>
><br>
> THis patch looks reasonable, except that it completely breaks the testsuite<br>
> :).  Please track down the bug and resubmit a patch.  Also, why not just<br>
> embed a 'Name' StringRef in Option.h instead of storing the start+size?<br>
><br>
> -Chris<br>
><br>
><br>
-------------- next part --------------<br>
An HTML attachment was scrubbed...<br>
URL: <a href="http://lists.cs.uiuc.edu/pipermail/cfe-commits/attachments/20110701/782f855a/attachment-0001.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/cfe-commits/attachments/20110701/782f855a/attachment-0001.html</a><br>

-------------- next part --------------<br>
A non-text attachment was scrubbed...<br>
Name: StringRef-ize--driver.Option.getName-2.patch<br>
Type: application/octet-stream<br>
Size: 5033 bytes<br>
Desc: not available<br>
Url : <a href="http://lists.cs.uiuc.edu/pipermail/cfe-commits/attachments/20110701/782f855a/attachment-0001.obj" target="_blank">http://lists.cs.uiuc.edu/pipermail/cfe-commits/attachments/20110701/782f855a/attachment-0001.obj</a><br>
</blockquote></div><br>Hi Zach,<br><br>The pattern in DiagnosticIDs.cpp comes from Argyrios :)<br><br>I tried using StringRef first, unfortunately it led to a massive explosion of the compilation time of the file because the compiler had to emit *lots* of constructor calls, while "bare" C types are trivial to initialize. Argyrios tracked the problem down and came up with this solution, it's a bit gross I think (and unfortunate) but noone could propose a better solution that did not involve calls to strlen at runtime.<br>
<br>This pattern is an optimization, and not meant to be reproduced. Sorry for it being misleading :/<br><br>-- Matthieu<br><br>