On Wed, May 15, 2013 at 4:08 AM, Hans Wennborg <[email protected]> wrote:
> On Tue, May 14, 2013 at 9:37 PM, Richard Smith <[email protected]> > wrote: > > @@ -79,7 +117,7 @@ static Expr *IsStringInit(Expr *init, QualType > declType, > > ASTContext &Context) { > > const ArrayType *arrayType = Context.getAsArrayType(declType); > > if (!arrayType) return 0; > > > > - return IsStringInit(init, arrayType, Context); > > + return IsStringInit(init, arrayType, Context) == SIF_None ? init : 0; > > > > Looks like the one and only caller of this doesn't actually want the > > expression; maybe just return the SIF value here too? > > I've changed it to just return bool since that's really what the > caller is looking for. The overload set for IsStringInit is now pretty scary: one overload returns zero (as an enum) on success and the other returns true, and the only difference in their signatures is that one takes a 'const ArrayType*' where the other takes a 'QualType'. Please make them both return the same type. > @@ -4307,9 +4363,10 @@ InitializationSequence::InitializationSequence(Sema > > &S, > > TryListInitialization(S, Entity, Kind, > > cast<InitListExpr>(Initializer), > > *this); > > AddParenthesizedArrayInitStep(DestType); > > - } else if (DestAT->getElementType()->isAnyCharacterType()) > > + } else if (DestAT->getElementType()->isAnyCharacterType() && > > + !isa<StringLiteral>(Initializer->IgnoreParens())) { > > SetFailed(FK_ArrayNeedsInitListOrStringLiteral); > > > > I don't think this is right -- the diagnostic should depend on whether > the > > element type could be initialized by a string literal, and not on whether > > the initializer actually was a string literal. That is, in MS C mode, > > > > __wchar_t str[] = xxxx; > > > > should not suggest initializing with a string literal, whether xxxx is a > > string literal or not. > > Ah, you're right, it's really the isAnyCharacterType() part that's > wrong. I've changed this, and now it means we also get better > diagnostics for code like: > > wchar_t s[] = 1; > > in C, where wchar_t is not built-in and thus not part of > "isAnyCharacterType()". > Thanks! > > Otherwise, looks great, thanks. > > Thanks! Committed r181880. > > - Hans > > > > On Tue, May 14, 2013 at 9:48 AM, Hans Wennborg <[email protected]> > wrote: > >> > >> On Tue, May 14, 2013 at 4:12 PM, David Blaikie <[email protected]> > wrote: > >> > On May 13, 2013 9:59 AM, "Hans Wennborg" <[email protected]> wrote: > >> >> /tmp/a.c:3:9: error: initializing wide char array with non-wide > >> >> string > >> >> literal > >> >> wchar_t s[] = "Hi"; > >> > > >> > Worth it/possible/convenient to go one step further and provide a > fixit > >> > to > >> > insert the 'L' and appropriately recover? I suppose that's probably > >> > orthogonal to your patch, though. > >> > >> I can look into it, but as you say, probably orthogonal to this patch. > >> > >> Thanks, > >> Hans >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
