vmiklos updated this revision to Diff 174021. vmiklos marked 2 inline comments as done.
https://reviews.llvm.org/D54349 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ReadabilityTidyModule.cpp clang-tidy/readability/RedundantPreprocessorCheck.cpp clang-tidy/readability/RedundantPreprocessorCheck.h docs/ReleaseNotes.rst docs/clang-tidy/checks/list.rst docs/clang-tidy/checks/readability-redundant-preprocessor.rst test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp test/clang-tidy/readability-redundant-preprocessor.cpp test/clang-tidy/readability-redundant-preprocessor.h
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 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: 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: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -248,6 +248,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/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -167,6 +167,11 @@ Detects usage of magic numbers, numbers that are used as literals instead of introduced via constants or symbols. +- New :doc:`readability-redundant-preprocessor + <clang-tidy/checks/readability-redundant-preprocessor>` check. + + Finds potentially redundant preprocessor directives. + - New :doc:`readability-uppercase-literal-suffix <clang-tidy/checks/readability-uppercase-literal-suffix>` check. 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,107 @@ +//===--- 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 { +/// 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 { + StringRef Condition = + Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange), + PP.getSourceManager(), PP.getLangOpts()); + 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<PreprocessorEntry, 4> &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<PreprocessorEntry, 4> IfStack; + SmallVector<PreprocessorEntry, 4> IfdefStack; + SmallVector<PreprocessorEntry, 4> IfndefStack; +}; +} // namespace + +void RedundantPreprocessorCheck::registerPPCallbacks( + CompilerInstance &Compiler) { + Compiler.getPreprocessor().addPPCallbacks( + ::llvm::make_unique<RedundantPreprocessorCallbacks>( + *this, Compiler.getPreprocessor())); +} + +} // namespace readability +} // namespace tidy +} // namespace clang 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" @@ -83,6 +84,8 @@ "readability-redundant-function-ptr-dereference"); CheckFactories.registerCheck<RedundantMemberInitCheck>( "readability-redundant-member-init"); + CheckFactories.registerCheck<RedundantPreprocessorCheck>( + "readability-redundant-preprocessor"); CheckFactories.registerCheck<SimplifySubscriptExprCheck>( "readability-simplify-subscript-expr"); CheckFactories.registerCheck<StaticAccessedThroughInstanceCheck>( 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
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits