PiotrZSL updated this revision to Diff 503529.
PiotrZSL added a comment.
Micro fixes in documentation
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145617/new/
https://reviews.llvm.org/D145617
Files:
clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp
clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.h
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
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
Index: clang-tools-extra/test/clang-tidy/checkers/readability/avoid-unconditional-preprocessor-if.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/avoid-unconditional-preprocessor-if.cpp
@@ -0,0 +1,69 @@
+// 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 '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 '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
+
+#if 10 > DDD
+// some code
+#endif
Index: clang-tools-extra/docs/clang-tidy/checks/readability/avoid-unconditional-preprocessor-if.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability/avoid-unconditional-preprocessor-if.rst
@@ -0,0 +1,75 @@
+.. title:: clang-tidy - readability-avoid-unconditional-preprocessor-if
+
+readability-avoid-unconditional-preprocessor-if
+===============================================
+
+Check flags always enabled or disabled code blocks in preprocessor ``#if``
+conditions, such as ``#if 0`` and ``#if 1``.
+
+.. code-block:: c++
+
+ #if 0
+ // some disabled code
+ #endif
+
+ #if 1
+ // some enabled code that can be disabled manually
+ #endif
+
+Preprocessor directives are a powerful feature in C and C++ that allow code to
+be modified before it is compiled. These directives can be used to include or
+exclude code based on conditions, or to define macros that can be used
+throughout the code. However, the misuse of preprocessor directives can create
+several issues that can impact the readability and maintainability of the code.
+
+One issue that can arise from the use of preprocessor directives is dead code.
+Dead code refers to code that is never executed or used during runtime. This can
+occur when code is wrapped in a preprocessor directive that is always disabled,
+such as ``#if 0``. Dead code can make the code harder to read and understand,
+and can create clutter that makes it harder to identify and modify the active
+parts of the code.
+
+Dead code can also create maintenance issues. Code that is never executed or
+used can indicate unfinished or abandoned features, or can create confusion for
+future developers who may try to modify or remove it. This can lead to a
+decrease in the quality and reliability of the codebase, and can make it harder
+to maintain and update over time.
+
+Another issue that can arise from the use of preprocessor directives is the
+creation of additional code branches and paths that need to be tested and
+debugged. Unconditional preprocessor directives that enable code, such as
+``#if 1``, can create additional paths through the code that need to be tested
+and debugged, potentially leading to additional errors or issues.
+
+In the case of unconditional preprocessor directives that enable code, such as
+``#if 1``, there can be a risk of creating always enabled code that may cause
+issues when different parts of the codebase are meant to work together but can
+be enabled separately. This can result in broken functionality if different
+parts of the code are enabled without proper coordination. In such cases, it is
+recommended to use preprocessor macros or defines to control the enabling of the
+code, such as ``#ifdef DEBUGGING_ENABLED``.
+
+Using preprocessor macros or defines can help to prevent issues that may arise
+from the unconditional enabling of code, making it easier to coordinate
+different parts of the codebase and to prevent broken functionality.
+
+By using conditional preprocessor directives with meaningful expressions or
+preprocessor macros instead of unconditional directives that enable code,
+developers can make the codebase more readable, understandable, and maintainable.
+This approach can also help identify unfinished or abandoned features, reduce
+dead code, and minimize the number of additional code branches and paths that
+require testing and debugging. Ultimately, this can lead to a more reliable and
+efficient codebase.
+
+Overall, the use of preprocessor directives should be approached with care to
+ensure that the codebase remains readable, maintainable, and reliable. Avoiding
+the use of unconditional preprocessor directives that enable code and instead
+utilizing conditional preprocessor directives with meaningful expressions or
+preprocessor macros can help to make the codebase easier to read, understand,
+and maintain, ultimately leading to a more reliable and efficient development
+process.
+
+In summary, this check helps detect the use of unconditional preprocessor
+directives that can create technical debt, such as dead code or always enabled
+code. By identifying these issues, this check helps improve the maintainability
+and readability of the codebase.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -329,6 +329,7 @@
`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"
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -120,6 +120,12 @@
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.
+
+ Check flags always enabled or disabled code blocks in preprocessor ``#if``
+ conditions, such as ``#if 0`` and ``#if 1``.
+
New check aliases
^^^^^^^^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ 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 @@
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>(
Index: clang-tools-extra/clang-tidy/readability/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -5,6 +5,7 @@
add_clang_library(clangTidyReadabilityModule
AvoidConstParamsInDecls.cpp
+ AvoidUnconditionalPreprocessorIfCheck.cpp
BracesAroundStatementsCheck.cpp
ConstReturnTypeCheck.cpp
ContainerContainsCheck.cpp
Index: clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.h
===================================================================
--- /dev/null
+++ 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 {
+
+/// Check flags always enabled or disabled code blocks in preprocessor `#if`
+/// conditions, such as `#if 0` and `#if 1`.
+///
+/// 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
Index: clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp
@@ -0,0 +1,105 @@
+//===--- 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;
+ auto &SM = PP.getSourceManager();
+ if (!isStatic(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 isStatic(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 (!isStaticToken(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 isStaticToken(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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
