[PATCH] D85545: [Diagnostics] Diagnose missing comma in string array initialization

[Dávid Bolvanský via cfe-commits]

For your cases, we currently do not warn. If possible, fetch the
latest Clang trunk and re-evaluate on Firefox.

po 10. 8. 2020 o 23:46 Arthur O'Dwyer <arthur.j.odwyer at gmail.com> napísal(a):
>
> It looks to me as if all of the false-positives so far have been not arrays but structs.
>
> struct X { int a; const char *b; int c; };
> X x = { 41, "forty" "two", 43 };  // false-positive here
>
> The distinguishing feature here is that if you did insert a comma as suggested by the compiler, then the result would no longer type-check.
> X x = { 41, "forty", "two", 43 };  // this is ill-formed because "two" is not a valid initializer for `int c`
>
> Dávid, can you use this in some way?
> IMHO it would be appropriate to just turn the warning off if the entity being initialized is a struct — leave the warning enabled only for initializers of arrays.
>
> my $.02,
> –Arthur
>
>
> On Mon, Aug 10, 2020 at 5:38 PM Dávid Bolvanský <david.bolvansky at gmail.com> wrote:
>>
>> I moved it to -Wextra due to false positives.
>>
>> > Should there be some exception for line length
>>
>> Yeah, but sure how to define the threshold or so. :/
>>
>> po 10. 8. 2020 o 23:21 dmajor via Phabricator
>> <reviews at reviews.llvm.org> napísal(a):
>> >
>> > dmajor added a comment.
>> >
>> > In the Firefox repo this warning is firing on a number of strings that were broken up by clang-format (or humans) for line length, for example https://searchfox.org/mozilla-central/rev/ab81b8552f4aa9696a2524f97fdfeb59d4dc31c1/security/certverifier/ExtendedValidation.cpp#176-178 or https://searchfox.org/mozilla-central/rev/ab81b8552f4aa9696a2524f97fdfeb59d4dc31c1/xpcom/tests/gtest/TestEscape.cpp#103-104 or https://searchfox.org/mozilla-central/rev/ab81b8552f4aa9696a2524f97fdfeb59d4dc31c1/js/src/jsapi-tests/testXDR.cpp#115.
>> >
>> > Do you consider these to be false positives in your view? Should there be some exception for line length, perhaps?
>> >
>> >
>> > Repository:
>> >   rG LLVM Github Monorepo
>> >
>> > CHANGES SINCE LAST ACTION
>> >   https://reviews.llvm.org/D85545/new/
>> >
>> > https://reviews.llvm.org/D85545
>> >