[PATCH] First set of patches for type uniquing

[David Blaikie]

On Wed, Aug 28, 2013 at 11:48 AM, David Majnemer
<david.majnemer at gmail.com> wrote:
> On Wed, Aug 28, 2013 at 10:32 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> On Wed, Aug 28, 2013 at 2:01 AM, David Majnemer
>> <david.majnemer at gmail.com> wrote:
>> > On Tue, Aug 27, 2013 at 5:42 PM, Manman Ren <manman.ren at gmail.com>
>> > wrote:
>> >>
>> >>
>> >>
>> >>
>> >> On Tue, Aug 27, 2013 at 2:34 PM, David Blaikie <dblaikie at gmail.com>
>> >> wrote:
>> >>>
>> >>> 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.
>> >>
>> >>
>> >> Hi David M,
>> >>
>> >> Any comment on whether we should move it out to a separate case?
>> >
>> >
>> > We have yet to define a vendor mangling for it so I wouldn't worry about
>> > testing it when targeting the itanium ABI.
>>
>> Are there places this could lead to a crash in the existing code?
>
>
> It shouldn't lead to a crash, the Itanium mangler will just issue an error
> diagnostic saying it couldn't mangle it.
>
>>
>> Should we disable the language feature on Itanium targets to ensure
>> this doesn't happen? (& modify the test case so the GUID stuff is only
>> tested on win32 ABI targets)
>
>
> I don't know what the original intention of the adding __uuidof() to clang.
> The feature still works as long as you don't need to use it in a way that
> demands a mangling. There is some precedent for this as the GNU extension ?:
> with the middle operand omitted cannot be mangled on Itanium.

Manman,

OK, in that case the test case as written is appropriate - we don't
want to lose coverage over that scenario (we never would want to lose
that coverage - it should either have an error we check, or other
behavior we check for) - and we should have the debug info fall back
(this seems like a bit of a hack, really... ) to not providing a
unique identifier when using itanium mangling of uuid objects. Or we
define a mangling for it - I'm almost inclined to define a mangling,
since I assume that's fairly simple - rather than having such a weird
hack.

(or I suppose we could error out (which is I guess what Manman hit) -
I think we have one or two other fatal errors in debug info, so that
might suffice - but rather than have the test case avoid the failure
it should check for it (conditionally, for the appropriate targets,
etc))

- David