[PATCH] First set of patches for type uniquing

[David Blaikie]

On Tue, Aug 27, 2013 at 2:28 PM, Reid Kleckner <rnk at google.com> wrote:
> Why are we using the Itanium mangler for uuidof exprs?  Can we not do that?

We have a test case (see
clang/test/CodeGenCXX/debug-info-template.cpp) because we seem to
support the language feature on itanium platforms. If we shouldn't
allow that language extension in that case, we should disable it &
move this test out to a separate case.

(+David Majnemer who implemented this test/debug info support)

> If not, feel free to grab the approach used in the MS mangler and pretend to
> mangle a global variable called _GUID_1234....

I'm OK-ish with this, but deferring to you & David if you want to
avoid these language extensions cropping up outside of win32
platforms.

>
>
> On Tue, Aug 27, 2013 at 1:46 PM, Eric Christopher <echristo at gmail.com>
> wrote:
>>
>> On Tue, Aug 27, 2013 at 1:12 PM, Manman Ren <manman.ren at gmail.com> wrote:
>> >
>> >
>> >
>> > On Tue, Aug 27, 2013 at 1:04 PM, David Blaikie <dblaikie at gmail.com>
>> > wrote:
>> >>
>> >> On Tue, Aug 27, 2013 at 12:57 PM, Manman Ren <manman.ren at gmail.com>
>> >> wrote:
>> >> >
>> >> >
>> >> >
>> >> > On Tue, Aug 27, 2013 at 12:41 PM, David Blaikie <dblaikie at gmail.com>
>> >> > wrote:
>> >> >>
>> >> >> On Tue, Aug 27, 2013 at 11:23 AM, Manman Ren <manman.ren at gmail.com>
>> >> >> wrote:
>> >> >>
>> >> >> >> 1) add identifier from Clang (DIBuilder support, plus passing in
>> >> >> >> the
>> >> >> >> identifier from Clang), updating lots of Clang test cases
>> >> >>
>> >> >> I assume you're going to have to update a lot of test cases if you
>> >> >> make the change as you've written it - as we'll start producing
>> >> >> empty
>> >> >> strings ("") for the identifier field instead of null. Is that
>> >> >> correct?
>> >> >
>> >> > The verifier currently accepts both null and a StringRef, so there is
>> >> > no
>> >> > verification error with the existing
>> >> > llvm testing cases. Yes, we are now producing "" instead of null. To
>> >> > make
>> >> > sure the existing llvm testing
>> >> > cases follow what DIBuilder emits, we can update the testing cases or
>> >> > use a
>> >> > null value when the
>> >> > string is empty as suggested below.
>> >>
>> >> It's the Clang test cases I'm concerned about - if you just commit
>> >> this change, the Clang build will break, won't it?
>> >
>> >
>> > I attached a patch for updating testing cases earlier:
>> > it basically changed from null to the actual mangled name.
>> > One example:
>> >
>> > -// CHECK: [[C:![0-9]*]] = {{.*}} metadata [[C_MEM:![0-9]*]], i32 0,
>> > metadata [[C]], null, null} ; [ DW_TAG_structure_type ] [C] {{.*}} [def]
>> >
>> > +// CHECK: [[C:![0-9]*]] = {{.*}} metadata [[C_MEM:![0-9]*]], i32 0,
>> > metadata [[C]], null, metadata !"_ZTS1C"} ; [ DW_TAG_structure_type ]
>> > [C]
>> > {{.*}} [def]
>> >
>> >
>> > By applying the 3 patches (clang, llvm, clang-test), I still see one
>> > failure: CodeGenCXX/debug-info-template.cpp fails with error: cannot yet
>> > mangle expression type CXXUuidoofExpr.
>> >
>> > This can be fixed by modifying ItaniumMangler
>> > (CXXNameMangler::mangleExpression), it currently has
>> > FIXME: invent manglings for all these.
>> >
>>
>> So.. Reid... :)
>>
>> -eric
>
>