diff --git a/clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.h b/clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.h new file mode 100644 --- /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 { + +/// 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 diff --git a/clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp b/clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp new file mode 100644 --- /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 (!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 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 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(*this, + *PP)); +} + +} // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/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 diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp --- 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 @@ void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck( "readability-avoid-const-params-in-decls"); + CheckFactories.registerCheck( + "readability-avoid-unconditional-preprocessor-if"); CheckFactories.registerCheck( "readability-braces-around-statements"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -133,6 +133,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 + ` check. + + Check flags always enabled or disabled code blocks in preprocessor ``#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 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -331,6 +331,7 @@ `portability-simd-intrinsics `_, `portability-std-allocator-const `_, `readability-avoid-const-params-in-decls `_, "Yes" + `readability-avoid-unconditional-preprocessor-if `_, `readability-braces-around-statements `_, "Yes" `readability-const-return-type `_, "Yes" `readability-container-contains `_, "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 --- /dev/null +++ b/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`` etc. + +.. 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. 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 --- /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() +// some code +#endif + +#if __has_include() +// 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