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 { + +/// 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/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 (!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 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 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(*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,13 @@ 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. + + 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 --- 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,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 difficult, +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 +different 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 --- /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