[cfe-commits] cfe-commits Digest, Vol 49, Issue 1

[Matthieu Monrocq]

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

Hi Zach,

The pattern in DiagnosticIDs.cpp comes from Argyrios :)

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.

This pattern is an optimization, and not meant to be reproduced. Sorry for
it being misleading :/

-- Matthieu
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110701/0e2b66e7/attachment.html>