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 <[email protected]> wrote: > Ping? > > On Mon, May 6, 2013 at 4:02 PM, Hans Wennborg <[email protected]> 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 <[email protected]> > wrote: > >> Thanks! Committed r181004. > >> > >> On Thu, May 2, 2013 at 6:55 PM, Aaron Ballman <[email protected]> > wrote: > >>> Patch LGTM! > >>> > >>> ~Aaron > >>> > >>> On Thu, May 2, 2013 at 1:38 PM, Hans Wennborg <[email protected]> > wrote: > >>>> On Tue, Apr 23, 2013 at 4:46 PM, Hans Wennborg <[email protected]> > wrote: > >>>>> On Tue, Apr 23, 2013 at 2:48 PM, Richard Smith < > [email protected]> 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 >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
