[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

2021-08-16 Thread Nathan Chancellor via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG9ed4a94d6451: [clang] Expose unreachable fallthrough annotation warning (authored by nathanchance). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

2021-08-16 Thread Nathan Chancellor via Phabricator via cfe-commits
nathanchance updated this revision to Diff 366733. nathanchance marked an inline comment as not done. nathanchance edited the summary of this revision. nathanchance added a comment. - Update commit message (forgot to use `arc diff --edit --verbatim`) [David] Repository: rG LLVM Github

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

2021-08-16 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Patch description probably needs an update - looks like it's out of date ("Add -Wimplicit-fallthrough-unreachable, which is default enabled with -Wimplicit-fallthrough" should read, I guess "Add -Wunreachable-code-fallthrough, which is default enabled with

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

2021-08-16 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, I think this is incremental progress. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107933/new/

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

2021-08-13 Thread Nathan Chancellor via Phabricator via cfe-commits
nathanchance marked an inline comment as done. nathanchance added inline comments. Comment at: clang/test/SemaCXX/switch-implicit-fallthrough.cpp:211-214 +case 224: + n += 400; +case 225: // expected-warning{{unannotated fall-through between switch labels}}

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

2021-08-13 Thread Nathan Chancellor via Phabricator via cfe-commits
nathanchance updated this revision to Diff 366363. nathanchance added a comment. - -Wimplicit-fallthrough-unreachable -> -Wunreachable-code-fallthrough, enabled under -Wunreachable-code (Dávid, Aaron). - Drop "case 225" from new test, as it does not add to test coverage (Nick). - Add

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

2021-08-13 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. In D107933#2944204 , @aaron.ballman wrote: > In D107933#2944135 , @nathanchance > wrote: > >> In D107933#2942430 , @xbolva00 >> wrote: >> >>>

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

2021-08-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D107933#2944135 , @nathanchance wrote: > In D107933#2942430 , @xbolva00 > wrote: > >> Yes, something like that, plus I think you want put >> UnreachableCodeFallthrough into

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

2021-08-13 Thread Nathan Chancellor via Phabricator via cfe-commits
nathanchance added a comment. In D107933#2942430 , @xbolva00 wrote: > Yes, something like that, plus I think you want put > UnreachableCodeFallthrough into group UnreachableCode as well. So you would recommend adding it to `UnreachableCode` rather than

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

2021-08-12 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Probably still worth fixing the bug too? (though maybe not a priority once it's moved out into -Wunreachable-code) - though the general fix, as discussed on the bug (comment 18 and 19 about why this doesn't already produce an unreachable-code warning), may require a

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

2021-08-12 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Yes, something like that, plus I think you want put UnreachableCodeFallthrough into group UnreachableCode as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107933/new/ https://reviews.llvm.org/D107933

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

2021-08-12 Thread Nathan Chancellor via Phabricator via cfe-commits
nathanchance added a comment. So something more along the lines of diff diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 1555a9ee2465..0aa9a82e3d11 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

2021-08-12 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. I think we should just move it, not delete it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107933/new/ https://reviews.llvm.org/D107933 ___ cfe-commits mailing list

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

2021-08-12 Thread Nathan Chancellor via Phabricator via cfe-commits
nathanchance added a comment. > In D107933#2942023 , @xbolva00 > wrote: > >> > > GCC does not warn (with common -Wall) for this case, right? I think Clang > should not as well. Correct, GCC does not warn at all about unreachable fallthrough

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

2021-08-12 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. In D107933#2942023 , @xbolva00 wrote: > GCC does not warn (with common -Wall) for this case, right? I think Clang > should not as well. > > ImplicitFallthroughUnreachable could be enabled with -Wunreachable-code, if > you

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

2021-08-12 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. GCC does not warn (with common -Wall) for this case, right? I think Clang should not as well. ImplicitFallthroughUnreachable could be enabled with -Wunreachable-code, if you think we should have it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

2021-08-12 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments. Comment at: clang/test/SemaCXX/switch-implicit-fallthrough.cpp:211-214 +case 224: + n += 400; +case 225: // expected-warning{{unannotated fall-through between switch labels}} expected-note{{insert '[[clang::fallthrough]];' to

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

2021-08-11 Thread Nathan Chancellor via Phabricator via cfe-commits
nathanchance created this revision. nathanchance added reviewers: dblaikie, rsmith, nickdesaulniers. nathanchance requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The Linux kernel has a macro called IS_ENABLED(), which evaluates to a