[PATCH] D4784: [clang-tidy] Add check for possibly incomplete switch statements

Shivam Gupta via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 12 23:24:38 PDT 2023


xgupta added inline comments.


================
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:225
+
+  Ensures that incomplete switch statements without default cases are
+  flagged, covering cases beyond enums where the compiler may not issue warnings.
----------------
PiotrZSL wrote:
> actually that are `complete` switch statements, developer covered everything He/She wanted, so other wording should be used
Sure.


================
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:226
+  Ensures that incomplete switch statements without default cases are
+  flagged, covering cases beyond enums where the compiler may not issue warnings.
+
----------------
PiotrZSL wrote:
> this suggest that enums are covered by check, and thats false
Corrected.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone/switch-missing-default-case.rst:46-49
+Options
+-------
+
+This check does not have any configurable options.
----------------
PiotrZSL wrote:
> this isnt needed, no options, not point to mention them
Sure, will remove.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone/switch-missing-default-case.rst:52
+.. note::
+   Enum types are already covered by compiler warnings when a switch statement
+   does not handle all enum values. This check focuses on non-enum types where
----------------
PiotrZSL wrote:
> would be nice to list them
Didn't understand, what should I list, non-enum types?


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone/switch-missing-default-case.rst:58
+   The C++ Core Guidelines provide guidelines on switch statements, including
+   the recommendation to always provide a default case: :cpp:dir:`C.89`.
----------------
PiotrZSL wrote:
> C.89 is C.89: Make a hash noexcept
> There is "ES.79: Use default to handle common cases (only)" but thats only about enums., still could be put here as reference, in such case you should put link
Accurate, thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D4784/new/

https://reviews.llvm.org/D4784



More information about the cfe-commits mailing list