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

[Dávid Bolvanský via cfe-commits]

Reworked ;) Now it should ignore structs properly.

ut 11. 8. 2020 o 22:03 Arthur O'Dwyer <arthur.j.odwyer at gmail.com> napísal(a):
>
> Dávid: Please just disable it for initializers of structs. That seems to be the common denominator in all the false positives I've observed on this thread.
>
>
> On Tue, Aug 11, 2020 at 3:07 PM Dávid Bolvanský <david.bolvansky at gmail.com> wrote:
>>
>> Ok, I will bump that limit + 1.
>>
>> ut 11. 8. 2020 o 20:52 Arthur Eubanks via Phabricator
>> <reviews at reviews.llvm.org> napísal(a):
>> >
>> > aeubanks added a comment.
>> >
>> > In D85545#2211070 <https://reviews.llvm.org/D85545#2211070>, @xbolva00 wrote:
>> >
>> > > I check if all elements of init list are strings, so this is not
>> > > exactly true "struct checker" (I have no such info in that part of
>> > > code..) but I consider it good enough. Your test case is based on real
>> > > code? Does not seem so - well sure, we could construct many examples
>> > > where warning fires uselessly but my focus is on real world false
>> > > positive cases :)
>> > >
>> > > ut 11. 8. 2020 o 19:55 Arthur Eubanks via Phabricator
>> > > <reviews at reviews.llvm.org> napísal(a):
>> > >
>> > >> aeubanks added a comment.
>> > >>
>> > >> Actually sorry, it does still seem like there are false positives on structs. Reduced:
>> > >>
>> > >>   $ cat /tmp/a.cpp
>> > >>
>> > >>   struct A {
>> > >>     const char* a;
>> > >>     const char* b;
>> > >>     const char* c;
>> > >>   };
>> > >>
>> > >>   static constexpr A foo2 = A{"",
>> > >>                               ""
>> > >>                               "",
>> > >>                               ""};
>> > >>
>> > >>   $ ~/repos/llvm-project/build_cmake/bin/clang /tmp/a.cpp -o /dev/null -c -Wstring-concatenation
>> > >>   /tmp/a.cpp:10:29: warning: suspicious concatenation of string literals in an array initialization; did you mean to separate the elements with a comma? [-Wstring-concatenation]
>> > >>                               "",
>> > >>                               ^
>> > >>   /tmp/a.cpp:9:29: note: place parentheses around the string literal to silence warning
>> > >>                               ""
>> > >>                               ^
>> > >>   1 warning generated.
>> > >>
>> > >> Repository:
>> > >>
>> > >>   rG LLVM Github Monorepo
>> > >>
>> > >> CHANGES SINCE LAST ACTION
>> > >>
>> > >>   https://reviews.llvm.org/D85545/new/
>> > >>
>> > >> https://reviews.llvm.org/D85545
>> >
>> > Yup it's reduced from real world code, same link as before: https://source.chromium.org/chromium/chromium/src/+/master:third_party/dawn/src/dawn_native/Toggles.cpp;drc=80f927d763211ea8e6a6377f86282809c86dc107;l=32.
>> > Clearly if there are 3 strings in the struct and the initializer has three strings, then the warning shouldn't apply.
>> >
>> >
>> > Repository:
>> >   rG LLVM Github Monorepo
>> >
>> > CHANGES SINCE LAST ACTION
>> >   https://reviews.llvm.org/D85545/new/
>> >
>> > https://reviews.llvm.org/D85545
>> >