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

[Arthur O'Dwyer via cfe-commits]

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
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200811/42877b39/attachment-0001.html>