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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
