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

[Arthur Eubanks via Phabricator via cfe-commits]

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