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 On Mon, Aug 10, 2020 at 5:51 PM Dávid Bolvanský <david.bolvan...@gmail.com> wrote: > Something like this: > const char *Sources[] = { > "// \\tparam aaa Bbb\n", > "// \\tparam\n" > "// aaa Bbb\n", > "// \\tparam \n" > "// aaa Bbb\n", > "// \\tparam aaa\n" > "// Bbb\n" > }; > > annoys me :/ Any idea/heuristic how to avoid warning here? > > po 10. 8. 2020 o 23:48 Dávid Bolvanský <david.bolvan...@gmail.com> > napísal(a): > > > > 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.odw...@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.bolvan...@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 > > >> <revi...@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 > > >> > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits