[Patch] Add the __wchar_t type in MS-compatibility mode (PR15815)

[Richard Smith]

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.

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.

On Wed, May 8, 2013 at 9:20 AM, Hans Wennborg <hans at chromium.org> wrote:

> Ping?
>
> On Mon, May 6, 2013 at 4:02 PM, Hans Wennborg <hans at chromium.org> wrote:
> > Hello again,
> >
> > As it turned out, that commit broke test/Sema/wchar.c on Windows, and
> > was reverted.
> >
> > The line that failed was this:
> >
> >   unsigned short s[] = L"something";
> >
> > The problem is that my patch was causing the type of the string
> > literal to change to __wchar_t on Windows, and since that is a
> > different type than unsigned short, the initialization doesn't work.
> >
> > I experimented some more with MSVC and learned that the code above
> > does compile in C mode, but not in C++ mode. The following does not
> > compile in C mode:
> >
> >   __wchar_t s[] = L"something";
> >
> > Which I found surprising.
> >
> > I think the semantics are like this:
> >
> > In C++, we have the wchar_t built-in type, and that's the type used
> > for wide string literals. __wchar_t is the same as wchar_t.
> >
> > In C, __wchar_t is the same as the built-in wchar_t would have been if
> > it were available. The type of wide string literals is array of
> > unsigned short.
> >
> > I'm attaching a new patch that implements this behavior. Please take a
> > look, and sorry again for the breakage.
> >
> > Thanks,
> > Hans
> >
> > On Fri, May 3, 2013 at 10:16 AM, Hans Wennborg <hans at chromium.org>
> wrote:
> >> Thanks! Committed r181004.
> >>
> >> On Thu, May 2, 2013 at 6:55 PM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
> >>> Patch LGTM!
> >>>
> >>> ~Aaron
> >>>
> >>> On Thu, May 2, 2013 at 1:38 PM, Hans Wennborg <hans at chromium.org>
> wrote:
> >>>> On Tue, Apr 23, 2013 at 4:46 PM, Hans Wennborg <hans at chromium.org>
> wrote:
> >>>>> On Tue, Apr 23, 2013 at 2:48 PM, Richard Smith <
> richard at metafoo.co.uk> wrote:
> >>>>>> Does the setup code in ASTContext::InitBuiltinTypes do the right
> thing here?
> >>>>>
> >>>>> Hmm, turns out it didn't.
> >>>>>
> >>>>> I guess it's not obvious what the right thing is here. From
> >>>>> experimenting a bit, it seems that __wchar_t is always available, and
> >>>>> is always a distinct builtin type in visual studio, even in C.
> >>>>>
> >>>>> New patch attached.
> >>>>
> >>>> Richard pointed out on IRC that we shouldn't change semantics in
> >>>> -fms-extensions.
> >>>>
> >>>> I'm attaching a new patch. In -fms-extensions, __wchar_t is the same
> >>>> as built-in wchar_t if available, otherwise it is the same as the
> >>>> appropriate integer type.
> >>>>
> >>>> In -fms-compatibility we try to mimic MSVC exactly: there is always a
> >>>> __wchar_t type, and it is always separate from the regular integer
> >>>> types.
> >>>>
> >>>> There are a number of parameters here: C vs. C++, -fms-extensions vs.
> >>>> -fms-compatibility, and -fno-wchar. The patch covers all of them and I
> >>>> think the tests make it reasonably clear. If we think this is too
> >>>> complicated, we could just use only the -fms-extensions part of the
> >>>> patch.
> >>>>
> >>>> New patch attached, please take a look.
> >>>>
> >>>> Thanks,
> >>>> Hans
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130508/bd6281b1/attachment.html>