[Patch] Better diagnostics for string initialization (and fix for C11)

[Hans Wennborg]

On Tue, May 14, 2013 at 9:37 PM, Richard Smith <richard at metafoo.co.uk> 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.

> @@ -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()".

> Otherwise, looks great, thanks.

Thanks! Committed r181880.

 - Hans


> On Tue, May 14, 2013 at 9:48 AM, Hans Wennborg <hans at chromium.org> wrote:
>>
>> On Tue, May 14, 2013 at 4:12 PM, David Blaikie <dblaikie at gmail.com> wrote:
>> > On May 13, 2013 9:59 AM, "Hans Wennborg" <hans at chromium.org> 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