[Debug Info PATCH] patches to generate unique identifier for external C++ types

Manman Ren manman.ren at gmail.com
Thu Aug 29 15:36:41 PDT 2013


On Thu, Aug 29, 2013 at 2:48 PM, David Blaikie <dblaikie at gmail.com> wrote:

> Please include test case changes along with whichever patch requires
> them, generally that's a bit easier to keep track of rather than
> having to pair a given change with a given patch.
>
> The Clang change could use the code review feedback I gave earlier
> today - have the unique string function return a SmallString & rely on
> NRVO, rather than passing by reference.

It seems that the other parts of the compiler pass a SmallString & and
return void, so I am not sure
whether we should depend on NRVO :)


> (also, you have a FIXME in
> there about objective C++ - is there any reason you aren't just
> checking both language values now? Or checking the language setting in
> LangOpts (in there, CPlusPlus is true for any C++ variant, including
> Objective C++) - but I'm happy to leave it out if you've got concerns
> about Objective-C types when used in Objective C++, I know nothing
> about such things)
>
I want to separate C++ from Objective C++, I was thinking about enabling
Objective C++ in a later patch.


>
> The comment in DIBuilder::finalize might use some more detail. Two
> different nodes will be added to the retained types list, but the
> client (Clang in this instance) deduplicates those after the fact,
> thus leaving the retained types list with duplicate nodes. Perhaps:
>
> // Declarations and definitions of the same type may be retained. Some
> clients RAUW these pairs, leaving duplicates in the retained types
> list. Use a set to remove the duplicates while we transform the
> TrackingVHs back into Values.
>
Will do.

Thanks again for the quick review,
Manman

>
> With those changes, please go ahead & commit these patches.
>
> On Thu, Aug 29, 2013 at 2:33 PM, Manman Ren <manman.ren at gmail.com> wrote:
> > Hi,
> >
> > I think it is a good idea to start a new thread :)
> > Attached please find 3 patches: one for llvm (update DIBuilder to retain
> > types with unique
> > identifier), one for clang (to generate the unique identifier), the last
> for
> > updating testing cases.
> >
> > Thanks,
> > Manman
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130829/6af6a2dc/attachment.html>


More information about the llvm-commits mailing list