For your godbolt example, you hit the heuristic to not warn, if literals count is <= 2. (motivated by many false positives; and I didnt see a true positive case, so..)
>> you could emit the warning only if exactly one comma was missing. This could work. ut 11. 8. 2020 o 0:04 Arthur O'Dwyer <arthur.j.odw...@gmail.com> napísal(a): > > 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