[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

Looks good, minor comments!



================
Comment at: clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.h:16
+
+///  Check flags always enabled or disabled code blocks in preprocessor `#if`
+///  conditions, such as `#if 0` and `#if 1`.
----------------
Ultra nit: remove extra space


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/readability/avoid-unconditional-preprocessor-if.cpp:17-31
+// CHECK-MESSAGES: :[[@LINE+1]]:2: warning: preprocessor condition is always 'true', consider removing condition but leaving its contents [readability-avoid-unconditional-preprocessor-if]
+#if 10>5
+
+#endif
+
+// CHECK-MESSAGES: :[[@LINE+1]]:2: warning: preprocessor condition is always 'true', consider removing condition but leaving its contents [readability-avoid-unconditional-preprocessor-if]
+#if 10 > 5
----------------
For completeness, would it make sense to add the same test but with the "is always 'false'" case?


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/readability/avoid-unconditional-preprocessor-if.cpp:67-69
+#if 10 > DDD
+// some code
+#endif
----------------
Add case for if 10 < DDD ?

Also, add test case for comparing 2 macros? If people follow the "no magic numbers" policy they'll likely have defines for both sides of the comparison.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145617