[clang-tools-extra] 52296f5 - [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Sun Mar 26 06:21:56 PDT 2023
Author: Piotr Zegar
Date: 2023-03-26T13:20:14Z
New Revision: 52296f5ed88b20af29cac517e349b221ed84cc2a
URL: https://github.com/llvm/llvm-project/commit/52296f5ed88b20af29cac517e349b221ed84cc2a
DIFF: https://github.com/llvm/llvm-project/commit/52296f5ed88b20af29cac517e349b221ed84cc2a.diff
LOG: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check
Check flags always enabled or disabled code blocks in preprocessor '#if'
conditions, such as '#if 0' and '#if 1' etc.
Reviewed By: carlosgalvezp
Differential Revision: https://reviews.llvm.org/D145617
Added:
clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp
clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.h
clang-tools-extra/docs/clang-tidy/checks/readability/avoid-unconditional-preprocessor-if.rst
clang-tools-extra/test/clang-tidy/checkers/readability/avoid-unconditional-preprocessor-if.cpp
Modified:
clang-tools-extra/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp b/clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp
new file mode 100644
index 0000000000000..d92d0e8f2dbf7
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp
@@ -0,0 +1,104 @@
+//===--- AvoidUnconditionalPreprocessorIfCheck.cpp - clang-tidy -----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "AvoidUnconditionalPreprocessorIfCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+namespace {
+struct AvoidUnconditionalPreprocessorIfPPCallbacks : public PPCallbacks {
+
+ explicit AvoidUnconditionalPreprocessorIfPPCallbacks(ClangTidyCheck &Check,
+ Preprocessor &PP)
+ : Check(Check), PP(PP) {}
+
+ void If(SourceLocation Loc, SourceRange ConditionRange,
+ ConditionValueKind ConditionValue) override {
+ if (ConditionValue == CVK_NotEvaluated)
+ return;
+ SourceManager &SM = PP.getSourceManager();
+ if (!isImmutable(SM, PP.getLangOpts(), ConditionRange))
+ return;
+
+ if (ConditionValue == CVK_True)
+ Check.diag(Loc, "preprocessor condition is always 'true', consider "
+ "removing condition but leaving its contents");
+ else
+ Check.diag(Loc, "preprocessor condition is always 'false', consider "
+ "removing both the condition and its contents");
+ }
+
+ bool isImmutable(SourceManager &SM, const LangOptions &LangOpts,
+ SourceRange ConditionRange) {
+ SourceLocation Loc = ConditionRange.getBegin();
+ if (Loc.isMacroID())
+ return false;
+
+ Token Tok;
+ if (Lexer::getRawToken(Loc, Tok, SM, LangOpts, true)) {
+ std::optional<Token> TokOpt = Lexer::findNextToken(Loc, SM, LangOpts);
+ if (!TokOpt || TokOpt->getLocation().isMacroID())
+ return false;
+ Tok = *TokOpt;
+ }
+
+ while (Tok.getLocation() <= ConditionRange.getEnd()) {
+ if (!isImmutableToken(Tok))
+ return false;
+
+ std::optional<Token> TokOpt =
+ Lexer::findNextToken(Tok.getLocation(), SM, LangOpts);
+ if (!TokOpt || TokOpt->getLocation().isMacroID())
+ return false;
+ Tok = *TokOpt;
+ }
+
+ return true;
+ }
+
+ bool isImmutableToken(const Token &Tok) {
+ switch (Tok.getKind()) {
+ case tok::eod:
+ case tok::eof:
+ case tok::numeric_constant:
+ case tok::char_constant:
+ case tok::wide_char_constant:
+ case tok::utf8_char_constant:
+ case tok::utf16_char_constant:
+ case tok::utf32_char_constant:
+ case tok::string_literal:
+ case tok::wide_string_literal:
+ case tok::comment:
+ return true;
+ case tok::raw_identifier:
+ return (Tok.getRawIdentifier() == "true" ||
+ Tok.getRawIdentifier() == "false");
+ default:
+ return Tok.getKind() >= tok::l_square && Tok.getKind() <= tok::caretcaret;
+ }
+ }
+
+ ClangTidyCheck &Check;
+ Preprocessor &PP;
+};
+
+} // namespace
+
+void AvoidUnconditionalPreprocessorIfCheck::registerPPCallbacks(
+ const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+ PP->addPPCallbacks(
+ std::make_unique<AvoidUnconditionalPreprocessorIfPPCallbacks>(*this,
+ *PP));
+}
+
+} // namespace clang::tidy::readability
diff --git a/clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.h b/clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.h
new file mode 100644
index 0000000000000..50292fce9d8dc
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.h
@@ -0,0 +1,32 @@
+//===--- AvoidUnconditionalPreprocessorIfCheck.h - clang-tidy ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOIDUNCONDITIONALPREPROCESSORIFCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOIDUNCONDITIONALPREPROCESSORIFCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::readability {
+
+/// Finds code blocks that are constantly enabled or disabled in preprocessor
+/// directives by analyzing `#if` conditions, such as `#if 0` and `#if 1`, etc.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability/avoid-unconditional-preprocessor-if.html
+class AvoidUnconditionalPreprocessorIfCheck : public ClangTidyCheck {
+public:
+ AvoidUnconditionalPreprocessorIfCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+ Preprocessor *ModuleExpanderPP) override;
+};
+
+} // namespace clang::tidy::readability
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOIDUNCONDITIONALPREPROCESSORIFCHECK_H
diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index ea09b2193eb7c..2306641ca9215 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS
add_clang_library(clangTidyReadabilityModule
AvoidConstParamsInDecls.cpp
+ AvoidUnconditionalPreprocessorIfCheck.cpp
BracesAroundStatementsCheck.cpp
ConstReturnTypeCheck.cpp
ContainerContainsCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
index 7aa090424257d..7fd9191844897 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -10,6 +10,7 @@
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
#include "AvoidConstParamsInDecls.h"
+#include "AvoidUnconditionalPreprocessorIfCheck.h"
#include "BracesAroundStatementsCheck.h"
#include "ConstReturnTypeCheck.h"
#include "ContainerContainsCheck.h"
@@ -60,6 +61,8 @@ class ReadabilityModule : public ClangTidyModule {
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.registerCheck<AvoidConstParamsInDecls>(
"readability-avoid-const-params-in-decls");
+ CheckFactories.registerCheck<AvoidUnconditionalPreprocessorIfCheck>(
+ "readability-avoid-unconditional-preprocessor-if");
CheckFactories.registerCheck<BracesAroundStatementsCheck>(
"readability-braces-around-statements");
CheckFactories.registerCheck<ConstReturnTypeCheck>(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index a5f090045615c..38a64616410ac 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -133,6 +133,13 @@ New checks
Checks that all implicit and explicit inline functions in header files are
tagged with the ``LIBC_INLINE`` macro.
+- New :doc:`readability-avoid-unconditional-preprocessor-if
+ <clang-tidy/checks/readability/avoid-unconditional-preprocessor-if>` check.
+
+ Finds code blocks that are constantly enabled or disabled in preprocessor
+ directives by analyzing ``#if`` conditions, such as ``#if 0`` and
+ ``#if 1``, etc.
+
New check aliases
^^^^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index f1945c1b5bf05..095f32f38da77 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -331,6 +331,7 @@ Clang-Tidy Checks
`portability-simd-intrinsics <portability/simd-intrinsics.html>`_,
`portability-std-allocator-const <portability/std-allocator-const.html>`_,
`readability-avoid-const-params-in-decls <readability/avoid-const-params-in-decls.html>`_, "Yes"
+ `readability-avoid-unconditional-preprocessor-if <readability/avoid-unconditional-preprocessor-if.html>`_,
`readability-braces-around-statements <readability/braces-around-statements.html>`_, "Yes"
`readability-const-return-type <readability/const-return-type.html>`_, "Yes"
`readability-container-contains <readability/container-contains.html>`_, "Yes"
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/avoid-unconditional-preprocessor-if.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/avoid-unconditional-preprocessor-if.rst
new file mode 100644
index 0000000000000..ce3bfaffac380
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/avoid-unconditional-preprocessor-if.rst
@@ -0,0 +1,32 @@
+.. title:: clang-tidy - readability-avoid-unconditional-preprocessor-if
+
+readability-avoid-unconditional-preprocessor-if
+===============================================
+
+Finds code blocks that are constantly enabled or disabled in preprocessor
+directives by analyzing ``#if`` conditions, such as ``#if 0`` and ``#if 1``,
+etc.
+
+.. code-block:: c++
+
+ #if 0
+ // some disabled code
+ #endif
+
+ #if 1
+ // some enabled code that can be disabled manually
+ #endif
+
+Unconditional preprocessor directives, such as ``#if 0`` for disabled code
+and ``#if 1`` for enabled code, can lead to dead code and always enabled code,
+respectively. Dead code can make understanding the codebase more
diff icult,
+hinder readability, and may be a sign of unfinished functionality or abandoned
+features. This can cause maintenance issues, confusion for future developers,
+and potential compilation problems.
+
+As a solution for both cases, consider using preprocessor macros or defines,
+like ``#ifdef DEBUGGING_ENABLED``, to control code enabling or disabling.
+This approach provides better coordination and flexibility when working with
+
diff erent parts of the codebase. Alternatively, you can comment out the entire
+code using ``/* */`` block comments and add a hint, such as ``@todo``,
+to indicate future actions.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-unconditional-preprocessor-if.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-unconditional-preprocessor-if.cpp
new file mode 100644
index 0000000000000..5e88bb3257196
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-unconditional-preprocessor-if.cpp
@@ -0,0 +1,94 @@
+// RUN: %check_clang_tidy %s readability-avoid-unconditional-preprocessor-if %t
+
+// CHECK-MESSAGES: :[[@LINE+1]]:2: warning: preprocessor condition is always 'false', consider removing both the condition and its contents [readability-avoid-unconditional-preprocessor-if]
+#if 0
+// some code
+#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 1
+// some code
+#endif
+
+#if test
+
+#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
+
+#endif
+
+// CHECK-MESSAGES: :[[@LINE+1]]:2: warning: preprocessor condition is always 'false', consider removing both the condition and 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
+// some code
+#endif
+
+// CHECK-MESSAGES: :[[@LINE+1]]:2: warning: preprocessor condition is always 'false', consider removing both the condition and its contents [readability-avoid-unconditional-preprocessor-if]
+#if 10 < 5
+// some code
+#endif
+
+// CHECK-MESSAGES: :[[@LINE+1]]:2: warning: preprocessor condition is always 'false', consider removing both the condition and its contents [readability-avoid-unconditional-preprocessor-if]
+#if !(10 > \
+ 5)
+// some code
+#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)
+// some code
+#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 true
+// some code
+#endif
+
+// CHECK-MESSAGES: :[[@LINE+1]]:2: warning: preprocessor condition is always 'false', consider removing both the condition and its contents [readability-avoid-unconditional-preprocessor-if]
+#if false
+// some code
+#endif
+
+#define MACRO
+#ifdef MACRO
+// some code
+#endif
+
+#if !SOMETHING
+#endif
+
+#if !( \
+ defined MACRO)
+// some code
+#endif
+
+
+#if __has_include(<string_view>)
+// some code
+#endif
+
+#if __has_include(<string_view_not_exist>)
+// some code
+#endif
+
+#define DDD 17
+#define EEE 18
+
+#if 10 > DDD
+// some code
+#endif
+
+#if 10 < DDD
+// some code
+#endif
+
+#if EEE > DDD
+// some code
+#endif
More information about the cfe-commits
mailing list