Is it essential to define __wchar_t as a separate keyword and TST rather than just reusing wchar_t? More generally, I think I'd prefer to see ASTContext have a 'WCharType', which is always the same as __wchar_t, and is also the same as wchar_t when it's a keyword, and also a 'WideCharType', which is the type of wide characters and wide strings.<div>
<br></div><div>What's the change in SemaInit.cpp for? It looks like it's a diagnostic improvement for the case where you try to initialize an array of character type from a string literal of an incompatible type. If this is desirable, could it be committed separately? Also, should it be using IsStringInit rather than isa<StringLiteral>? Right now, it seems to be missing a check for ObjCEncodeExpr.<br>
<div><br><div class="gmail_quote">On Wed, May 8, 2013 at 9:20 AM, Hans Wennborg <span dir="ltr"><<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Ping?<br>
<div class="HOEnZb"><div class="h5"><br>
On Mon, May 6, 2013 at 4:02 PM, Hans Wennborg <<a href="mailto:hans@chromium.org">hans@chromium.org</a>> wrote:<br>
> Hello again,<br>
><br>
> As it turned out, that commit broke test/Sema/wchar.c on Windows, and<br>
> was reverted.<br>
><br>
> The line that failed was this:<br>
><br>
>   unsigned short s[] = L"something";<br>
><br>
> The problem is that my patch was causing the type of the string<br>
> literal to change to __wchar_t on Windows, and since that is a<br>
> different type than unsigned short, the initialization doesn't work.<br>
><br>
> I experimented some more with MSVC and learned that the code above<br>
> does compile in C mode, but not in C++ mode. The following does not<br>
> compile in C mode:<br>
><br>
>   __wchar_t s[] = L"something";<br>
><br>
> Which I found surprising.<br>
><br>
> I think the semantics are like this:<br>
><br>
> In C++, we have the wchar_t built-in type, and that's the type used<br>
> for wide string literals. __wchar_t is the same as wchar_t.<br>
><br>
> In C, __wchar_t is the same as the built-in wchar_t would have been if<br>
> it were available. The type of wide string literals is array of<br>
> unsigned short.<br>
><br>
> I'm attaching a new patch that implements this behavior. Please take a<br>
> look, and sorry again for the breakage.<br>
><br>
> Thanks,<br>
> Hans<br>
><br>
> On Fri, May 3, 2013 at 10:16 AM, Hans Wennborg <<a href="mailto:hans@chromium.org">hans@chromium.org</a>> wrote:<br>
>> Thanks! Committed r181004.<br>
>><br>
>> On Thu, May 2, 2013 at 6:55 PM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:<br>
>>> Patch LGTM!<br>
>>><br>
>>> ~Aaron<br>
>>><br>
>>> On Thu, May 2, 2013 at 1:38 PM, Hans Wennborg <<a href="mailto:hans@chromium.org">hans@chromium.org</a>> wrote:<br>
>>>> On Tue, Apr 23, 2013 at 4:46 PM, Hans Wennborg <<a href="mailto:hans@chromium.org">hans@chromium.org</a>> wrote:<br>
>>>>> On Tue, Apr 23, 2013 at 2:48 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:<br>
>>>>>> Does the setup code in ASTContext::InitBuiltinTypes do the right thing here?<br>
>>>>><br>
>>>>> Hmm, turns out it didn't.<br>
>>>>><br>
>>>>> I guess it's not obvious what the right thing is here. From<br>
>>>>> experimenting a bit, it seems that __wchar_t is always available, and<br>
>>>>> is always a distinct builtin type in visual studio, even in C.<br>
>>>>><br>
>>>>> New patch attached.<br>
>>>><br>
>>>> Richard pointed out on IRC that we shouldn't change semantics in<br>
>>>> -fms-extensions.<br>
>>>><br>
>>>> I'm attaching a new patch. In -fms-extensions, __wchar_t is the same<br>
>>>> as built-in wchar_t if available, otherwise it is the same as the<br>
>>>> appropriate integer type.<br>
>>>><br>
>>>> In -fms-compatibility we try to mimic MSVC exactly: there is always a<br>
>>>> __wchar_t type, and it is always separate from the regular integer<br>
>>>> types.<br>
>>>><br>
>>>> There are a number of parameters here: C vs. C++, -fms-extensions vs.<br>
>>>> -fms-compatibility, and -fno-wchar. The patch covers all of them and I<br>
>>>> think the tests make it reasonably clear. If we think this is too<br>
>>>> complicated, we could just use only the -fms-extensions part of the<br>
>>>> patch.<br>
>>>><br>
>>>> New patch attached, please take a look.<br>
>>>><br>
>>>> Thanks,<br>
>>>> Hans<br>
</div></div></blockquote></div><br></div></div>