PiotrZSL updated this revision to Diff 508321. PiotrZSL added a comment. Fix doc
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,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 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,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. 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 @@ -331,6 +331,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 @@ -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 + <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 ^^^^^^^^^^^^^^^^^ 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 { + +/// 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 Index: clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp =================================================================== --- /dev/null +++ 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
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits