Index: clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tidy/readability/CMakeLists.txt +++ clang-tidy/readability/CMakeLists.txt @@ -24,6 +24,7 @@ RedundantDeclarationCheck.cpp RedundantFunctionPtrDereferenceCheck.cpp RedundantMemberInitCheck.cpp + RedundantPreprocessorCheck.cpp RedundantSmartptrGetCheck.cpp RedundantStringCStrCheck.cpp RedundantStringInitCheck.cpp Index: clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tidy/readability/ReadabilityTidyModule.cpp +++ clang-tidy/readability/ReadabilityTidyModule.cpp @@ -31,6 +31,7 @@ #include "RedundantDeclarationCheck.h" #include "RedundantFunctionPtrDereferenceCheck.h" #include "RedundantMemberInitCheck.h" +#include "RedundantPreprocessorCheck.h" #include "RedundantSmartptrGetCheck.h" #include "RedundantStringCStrCheck.h" #include "RedundantStringInitCheck.h" @@ -79,6 +80,8 @@ "readability-misleading-indentation"); CheckFactories.registerCheck( "readability-misplaced-array-index"); + CheckFactories.registerCheck( + "readability-redundant-preprocessor"); CheckFactories.registerCheck( "readability-redundant-function-ptr-dereference"); CheckFactories.registerCheck( Index: clang-tidy/readability/RedundantPreprocessorCheck.h =================================================================== --- /dev/null +++ clang-tidy/readability/RedundantPreprocessorCheck.h @@ -0,0 +1,35 @@ +//===--- RedundantPreprocessorCheck.h - clang-tidy --------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTPREPROCESSORCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTPREPROCESSORCHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace readability { + +/// This check flags redundant preprocessor directives: nested directives with +/// the same condition. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-preprocessor.html +class RedundantPreprocessorCheck : public ClangTidyCheck { +public: + RedundantPreprocessorCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerPPCallbacks(CompilerInstance &Compiler) override; +}; + +} // namespace readability +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTPREPROCESSORCHECK_H Index: clang-tidy/readability/RedundantPreprocessorCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/readability/RedundantPreprocessorCheck.cpp @@ -0,0 +1,135 @@ +//===--- RedundantPreprocessorCheck.cpp - clang-tidy ----------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "RedundantPreprocessorCheck.h" +#include "clang/Frontend/CompilerInstance.h" + +namespace clang { +namespace tidy { +namespace readability { + +namespace { +/// Gets the condition text of an #if directive from the the condition range's +/// source text. +std::string getCondition(StringRef SourceText) { + // Take the first logical line of SourceText, i.e. ignore line ends where + // the last character is a '\'. + std::string Ret = SourceText; + size_t Pos = 0; + while (true) { + Pos = Ret.find_first_of("\n", Pos); + if (Pos == std::string::npos) + break; + else { + if (Pos > 0 && Ret[Pos - 1] == '\\') + Pos += 1; + else { + Ret = Ret.substr(0, Pos); + break; + } + } + } + return Ret; +} + +/// Information about an opening preprocessor directive. +struct PreprocessorEntry { + SourceLocation Loc; + /// Condition used after the preprocessor directive. + std::string Condition; +}; + +class RedundantPreprocessorCallbacks : public PPCallbacks { +public: + explicit RedundantPreprocessorCallbacks(ClangTidyCheck &Check, + Preprocessor &PP) + : Check(Check), PP(PP) {} + + void If(SourceLocation Loc, SourceRange ConditionRange, + ConditionValueKind ConditionValue) override { + // ConditionRange refers to the full range from the start of the #if + // condition to the end of #endif. Extract just the condition of #if itself + // from that, so we can compare that to other #if directives, similar to + // #ifdef/#ifndef. + StringRef SourceText = + Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange), + PP.getSourceManager(), PP.getLangOpts()); + std::string Condition = getCondition(SourceText); + CheckMacroRedundancy(Loc, Condition, IfStack, + "nested redundant if; consider removing it", + "previous if was here", true); + } + + void Ifdef(SourceLocation Loc, const Token &MacroNameTok, + const MacroDefinition &MacroDefinition) override { + std::string MacroName = PP.getSpelling(MacroNameTok); + CheckMacroRedundancy(Loc, MacroName, IfdefStack, + "nested redundant ifdef; consider removing it", + "previous ifdef was here", true); + CheckMacroRedundancy(Loc, MacroName, IfndefStack, + "nested redundant ifdef; consider removing it", + "previous ifndef was here", false); + } + + void Ifndef(SourceLocation Loc, const Token &MacroNameTok, + const MacroDefinition &MacroDefinition) override { + std::string MacroName = PP.getSpelling(MacroNameTok); + CheckMacroRedundancy(Loc, MacroName, IfndefStack, + "nested redundant ifndef; consider removing it", + "previous ifndef was here", true); + CheckMacroRedundancy(Loc, MacroName, IfdefStack, + "nested redundant ifndef; consider removing it", + "previous ifdef was here", false); + } + + void Endif(SourceLocation Loc, SourceLocation IfLoc) override { + if (!IfStack.empty() && IfLoc == IfStack.back().Loc) + IfStack.pop_back(); + if (!IfdefStack.empty() && IfLoc == IfdefStack.back().Loc) + IfdefStack.pop_back(); + if (!IfndefStack.empty() && IfLoc == IfndefStack.back().Loc) + IfndefStack.pop_back(); + } + +private: + void CheckMacroRedundancy(SourceLocation Loc, StringRef MacroName, + SmallVector &Stack, + StringRef Warning, StringRef Note, bool Store) { + if (PP.getSourceManager().isInMainFile(Loc)) { + for (const auto &Entry : Stack) { + if (Entry.Condition == MacroName) { + Check.diag(Loc, Warning); + Check.diag(Entry.Loc, Note, DiagnosticIDs::Note); + } + } + } + + if (Store) + // This is an actual directive to be remembered. + Stack.push_back({Loc, MacroName}); + } + + ClangTidyCheck &Check; + Preprocessor &PP; + SmallVector IfStack; + SmallVector IfdefStack; + SmallVector IfndefStack; +}; +} // namespace + +void RedundantPreprocessorCheck::registerPPCallbacks( + CompilerInstance &Compiler) { + Compiler.getPreprocessor().addPPCallbacks( + ::llvm::make_unique( + *this, Compiler.getPreprocessor())); +} + +} // namespace readability +} // namespace tidy +} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -154,6 +154,11 @@ Detects usage of magic numbers, numbers that are used as literals instead of introduced via constants or symbols. +- New :doc:`readability-redundant-preprocessor + ` check. + + Finds potentially redundant preprocessor directives. + - New :doc:`readability-uppercase-literal-suffix ` check. Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -244,6 +244,7 @@ readability-redundant-declaration readability-redundant-function-ptr-dereference readability-redundant-member-init + readability-redundant-preprocessor readability-redundant-smartptr-get readability-redundant-string-cstr readability-redundant-string-init Index: docs/clang-tidy/checks/readability-redundant-preprocessor.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/readability-redundant-preprocessor.rst @@ -0,0 +1,61 @@ +.. title:: clang-tidy - readability-redundant-preprocessor + +readability-redundant-preprocessor +================================== + +Finds potentially redundant preprocessor directives. At the moment the +following cases are detected: + +* `#ifdef` .. `#endif` pairs which are nested inside an outer pair with the + same condition. For example: + +.. code-block:: c++ + + #ifdef FOO + #ifdef FOO // inner ifdef is considered redundant + void f(); + #endif + #endif + +* Same for `#ifndef` .. `#endif` pairs. For example: + +.. code-block:: c++ + + #ifndef FOO + #ifndef FOO // inner ifndef is considered redundant + void f(); + #endif + #endif + +* `#ifndef` inside an `#ifdef` with the same condition: + +.. code-block:: c++ + + #ifdef FOO + #ifndef FOO // inner ifndef is considered redundant + void f(); + #endif + #endif + +* `#ifdef` inside an `#ifndef` with the same condition: + +.. code-block:: c++ + + #ifndef FOO + #ifdef FOO // inner ifdef is considered redundant + void f(); + #endif + #endif + +* `#if` .. `#endif` pairs which are nested inside an outer pair with the same + condition. For example: + +.. code-block:: c++ + + #define FOO 4 + #if FOO == 4 + #if FOO == 4 // inner if is considered redundant + void f(); + #endif + #endif + Index: test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp =================================================================== --- /dev/null +++ test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp @@ -0,0 +1,36 @@ +// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -DFOO + +// Positive testing. +#ifdef FOO +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor] +#ifdef FOO +// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here +void f(); +#endif +#endif + +// Positive testing of inverted condition. +#ifdef FOO +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor] +#ifndef FOO +// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here +void f2(); +#endif +#endif + +// Negative testing. +#ifdef BAR +void g(); +#endif + +#ifdef FOO +#ifdef BAR +void h(); +#endif +#endif + +#ifdef FOO +#ifndef BAR +void i(); +#endif +#endif Index: test/clang-tidy/readability-redundant-preprocessor.h =================================================================== --- /dev/null +++ test/clang-tidy/readability-redundant-preprocessor.h @@ -0,0 +1,5 @@ +#ifndef FOO +#ifndef FOO // this would warn, but not in a header +void f(); +#endif +#endif Index: test/clang-tidy/readability-redundant-preprocessor.cpp =================================================================== --- /dev/null +++ test/clang-tidy/readability-redundant-preprocessor.cpp @@ -0,0 +1,76 @@ +// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -I %S + +// Positive testing. +#ifndef FOO +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor] +#ifndef FOO +// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here +void f(); +#endif +#endif + +// Positive testing of inverted condition. +#ifndef FOO +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor] +#ifdef FOO +// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here +void f2(); +#endif +#endif + +// Negative testing. +#include "readability-redundant-preprocessor.h" + +#ifndef BAR +void g(); +#endif + +#ifndef FOO +#ifndef BAR +void h(); +#endif +#endif + +#ifndef FOO +#ifdef BAR +void i(); +#endif +#endif + +// Positive #if testing. +#define FOO 4 + +#if FOO == 4 +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant if; consider removing it [readability-redundant-preprocessor] +#if FOO == 4 +// CHECK-NOTES: [[@LINE-3]]:2: note: previous if was here +void j(); +#endif +#endif + +#if FOO == \ + 4 +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant if; consider removing it [readability-redundant-preprocessor] +#if FOO == \ + 4 +// CHECK-NOTES: [[@LINE-5]]:2: note: previous if was here +void j(); +#endif +#endif + +// Negative #if testing. +#define BAR 4 + +#if FOO == 4 +#if BAR == 4 +void k(); +#endif +#endif + +#if FOO == \ + 4 +#if BAR == \ + 5 +void k(); +#endif +#endif