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

[Dávid Bolvanský via cfe-commits]

Pushed fix. It should not warn for structs or long texts.

ut 11. 8. 2020 o 0:34 Arthur Eubanks via Phabricator
<reviews at reviews.llvm.org> napísal(a):
>
> aeubanks added a comment.
>
> In D85545#2208266 <https://reviews.llvm.org/D85545#2208266>, @arthur.j.odwyer wrote:
>
> > To decrease the number of false-positives, you could emit the warning only
> > if *exactly one* comma was missing.
> >
> >   const char *likely_a_bug[] = { "a", "b", "c" "d", "e", "f", "g", "h",
> >
> > "i" };
> >
> >   const char *likely_not_a_bug[] = { "a", "b" "c", "d" "e", "f" "g" };
> >   const char *oops_still_a_bug[] = { "a", "b", "c" "d", "e", "f" "g",
> >
> > "h", "i" };
> >
> > However, as `oops_still_a_bug` shows, that tactic would also decrease the
> > number of true positives, and it would confuse the end-user, for whom
> > predictability is key.
> >
> > I still think it would be appropriate to *stop issuing the warning for
> > structs*, though.
> > Here's my struct example from below in Godbolt: https://godbolt.org/z/6jjv6a
> > Speaking of predictability, I don't understand why `struct Y` avoids the
> > warning whereas `struct X` hits it.
> > After removing the warning for structs, neither `X` nor `Y` should hit it,
> > and that should fix pretty much all the Firefox hits as I understand them.
> >
> > –Arthur
>
> +1 to ignoring structs. See https://crbug.com/1114873 and https://source.chromium.org/chromium/chromium/src/+/master:third_party/dawn/src/dawn_native/Toggles.cpp;drc=80f927d763211ea8e6a6377f86282809c86dc107;l=32.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D85545/new/
>
> https://reviews.llvm.org/D85545
>