[Clang] Add -Wimplicit-fallthrough to -Wextra for GCC compatibility

[lb90]

See also #58375

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-clang

Author: None (lb90)

Changes

See also #58375

---

Full diff: https://github.com/llvm/llvm-project/pull/149474.diff

2 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+2)
• (modified) clang/include/clang/Basic/DiagnosticGroups.td (+1)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ea16029268dba..f9b7240919785 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -716,6 +716,8 @@ Improvements to Clang's diagnostics
   Added a new warning in this group for the case where the attribute is missing/implicit on
   an override of a virtual method.
 
+- ``-Wimplicit-fallthrough`` was added to ``-Wextra`` for GCC compatibility.
+
 Improvements to Clang's time-trace
 ----------------------------------
 
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index ccb18aa37447e..6283c1ead791a 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1231,6 +1231,7 @@ def Extra : DiagGroup<"extra", [
     CastFunctionTypeMismatch,
     InitStringTooLongMissingNonString,
     WarnUnnecessaryVirtualSpecifier,
+    ImplicitFallthrough,
   ]>;
 
 def Most : DiagGroup<"most", [

[lb90]

Mmh I now see #58693 is closed as WONTFIX. Not sure if it's worth merging this, then...

[erichkeane]

I did a dive through the history here. It seems that the 'hold up' historically is the 'flex' project using the comment-based form. I agree with historical discussions that recognizing comments here is silly, and we should not do it.

That said, I'm generally in favor of this patch. Having this in -Wextra is a good idea. I personally could argue to add it to -Wall but I'm a bit of a curmudgeon about the 'we don't wanna change our code' attitude.

IMO, I have little sympathy for Flex's argument, and would be ok with merging this anyway (and folks who have to care about it, have to deal with it). That said, if a patch went to Flex to convert them to some form of the add a FALLTHROUGH macro defined to the right thing, I think it makes objections a nothingburger.

[AaronBallman]

One concern I have is about compile time performance impacts; the implicit fallthrough diagnostics require building a CFG and that can be expensive. It may be worth putting up a branch on https://llvm-compile-time-tracker.com/ to see how it impacts compile times

[lb90]

That said, if a patch went to Flex to convert them to some form of the add a FALLTHROUGH macro defined to the right thing, I think it makes objections a nothingburger.

Nice idea, opened westes/flex#709. Let's see how it goes