[clang-tidy] new check 'readability-redundant-preprocessor'

[vmiklos]

Summary

Finds potentially redundant preprocessor directives.

[vmiklos]

I've used this check originally on LibreOffice (core, online) code and it found a handful of not necessary ifdef / ifndef lines. It seemed it's simple and generic enough that it would make sense to upstream this.

[lebedev.ri]

No one will know for sure what "pp" in "readability-redundant-pp" means.
I'd highly recommend to fully spell it out.

Also, i'd like to see some analysis of the false-positives.

[JonasToth]

Hi vmiklos,

thank you for working on this patch!
I added a few comments.

clang-tidy/readability/ReadabilityTidyModule.cpp

83

I think that name is not very descriptive for the user of clang-tidy. pp should be preprocessor or some other constellation that makes it very clear its about the preprocessor.

clang-tidy/readability/RedundantPpCheck.cpp

28 ↗

(On Diff #173432)

you are in namespace clang, you can ellide clang::

37 ↗

(On Diff #173432)

Maybe SmallVector<Entry, 4>? Would be better for performance.

48 ↗

(On Diff #173432)

I think it would be better to have these methods defined inline, as the splitting does not achieve its original goal (declaration in header, definition in impl).

52 ↗

(On Diff #173432)

The two functions are copied, please remove this duplicatoin and refactor it to a general parametrized function.

docs/ReleaseNotes.rst

70

Please order the checks alphabetically in the release notes, and one empty line at the end is enough.

docs/clang-tidy/checks/readability-redundant-pp.rst

8 ↗

(On Diff #173432)

This needs more explanation, please add .. code-block:: c++ sections for the cases that demonstrate the issue.

test/clang-tidy/readability-redundant-pp-ifdef.cpp

6 ↗

(On Diff #173432)

Please add a test where the redundancy comes from including other headers. If the headers are nested this case might still occur, but its not safe to diagnose/remove these checks as other include-places might not have the same constellation.

ifdef and ifndef are used for the include-guards and therefore particularly necessary to test.

[vmiklos]

In D54349#1293622, @lebedev.ri wrote:

No one will know for sure what "pp" in "readability-redundant-pp" means.
I'd highly recommend to fully spell it out.

Will do.

Also, i'd like to see some analysis of the false-positives.

Things I considered:

• header guards would easily generate "nested ifndef" false positives, so I limited the check to the main file only

• if there are nested #ifdef FOO .. #endif blocks, but FOO is not defined, we fail to detect the redundancy.

• I read that in general checks should be careful around templates and macros, but given this deals with the preprocessor, I don't expect issues there.

This implicitly means that I'm not aware of actual false positives of the check in its current form.

[Eugene.Zelenko]

docs/ReleaseNotes.rst

73

preprocessor directives? Same in documentation.

docs/clang-tidy/checks/readability-redundant-pp.rst

8 ↗

(On Diff #173432)

Please use ``. Same below.

[vmiklos]

I think that name is not very descriptive for the user of clang-tidy. pp
should be preprocessor or some other constellation that makes it very clear
its about the preprocessor.

Done, renamed to readability-redundant-preprocessor.

you are in namespace clang, you can ellide clang::

Done.

Maybe SmallVector<Entry, 4>? Would be better for performance.

Done.

I think it would be better to have these methods defined inline, as the
splitting does not achieve its original goal (declaration in header,
definition in impl).

Done.

The two functions are copied, please remove this duplicatoin and refactor it
to a general parametrized function.

Done.

Please order the checks alphabetically in the release notes, and one empty
line at the end is enough.

Done.

This needs more explanation, please add .. code-block:: c++ sections for
the cases that demonstrate the issue.

Done.

Please add a test where the redundancy comes from including other headers. If
the headers are nested this case might still occur, but its not safe to
diagnose/remove these checks as other include-places might not have the same
constellation.

ifdef and ifndef are used for the include-guards and therefore
particularly necessary to test.

Done. I've added a test to make sure we don't warn in headers, the code for
this was already there, just had no coverage. (Exactly for the reason you
mention, the possibility of include-guards generating false positives.)

[vmiklos]

preprocessor directives? Same in documentation.

Done.

[aaron.ballman]

What do you think about code like:

#if FOO == 4
#if FOO == 4
#endif
#endif

#if defined(FOO)
#if defined(FOO)
#endif
#endif

#if !defined(FOO)
#if !defined(FOO)
#endif
#endif

#if defined(FOO)
#if !defined(FOO)
#endif
#endif

#if !defined(FOO)
#if defined(FOO)
#endif
#endif

clang-tidy/readability/CMakeLists.txt

23

Please keep this list sorted alphabetically.

clang-tidy/readability/RedundantPreprocessorCheck.cpp

53

This name is not particularly descriptive. This seems to be more like CheckMacroRedundancy or something like that?

clang-tidy/readability/RedundantPreprocessorCheck.h

2–3

This comment should be re-flowed to fit the column width.

21

What constitutes "redundancy"? A bit more exposition here would be useful.

[JonasToth]

Done.

Please also mark the inline comments as done. Otherwise its hard to follow if there are outstanding issues somewhere, especially if code moves around.

[Szelethus]

clang-tidy/readability/RedundantPreprocessorCheck.cpp

20–23

This is a way too general name in my opinion. Either include comments that describe it, or rename it (preferably both).

[vmiklos]

What do you think about code like:

#if FOO == 4
#if FOO == 4
#endif
#endif

#if defined(FOO)
#if defined(FOO)
#endif
#endif

#if !defined(FOO)
#if !defined(FOO)
#endif
#endif

I looked at supporting these, but it's not clear to me how to get e.g. the 'FOO == 4' string (the condition) from the If() callback. So far I only see how to get the start location of the condition and the whole range till the end of endif. Do you have a code pointer how to do this, please? (Or OK if I leave this for a follow-up patch?)

#if defined(FOO)
#if !defined(FOO)
#endif
#endif

#if !defined(FOO)
#if defined(FOO)
#endif
#endif

I expect handling these correctly is more complex -- I would prefer focusing on matching conditons in this patch, and get back to inverted conditions in a follow-up patch. Is that OK? If we handle inverted conditions, then handling a && b and !a || !b would make sense as well for example.

Please keep this list sorted alphabetically.

Done.

This name is not particularly descriptive. This seems to be more like
CheckMacroRedundancy or something like that?

Makes sense, done.

This comment should be re-flowed to fit the column width.

Done.

What constitutes "redundancy"? A bit more exposition here would be useful.

Hopefully "nested directives with the same condition" makes it easier to understand the intention and current scope.

[vmiklos]

clang-tidy/readability/RedundantPreprocessorCheck.cpp

20–23

Renamed to PreprocessorCondition, hope it helps. :-)

[Szelethus]

clang-tidy/readability/RedundantPreprocessorCheck.cpp

20–23

I actually meant it for Entry, if you hover your mouse over an inline comment, you can see which lines it applies to. Sorry for the confusing communication :D

[aaron.ballman]

In D54349#1294325, @vmiklos wrote:

What do you think about code like:

#if FOO == 4
#if FOO == 4
#endif
#endif

#if defined(FOO)
#if defined(FOO)
#endif
#endif

#if !defined(FOO)
#if !defined(FOO)
#endif
#endif

I looked at supporting these, but it's not clear to me how to get e.g. the 'FOO == 4' string (the condition) from the If() callback. So far I only see how to get the start location of the condition and the whole range till the end of endif. Do you have a code pointer how to do this, please? (Or OK if I leave this for a follow-up patch?)

I think it requires manual work. There's a FIXME in PPCallbacks::If() to pass a list/tree of tokens that would make implementing this reasonable. I'd say it's fine as a follow-up patch.

#if defined(FOO)
#if !defined(FOO)
#endif
#endif

#if !defined(FOO)
#if defined(FOO)
#endif
#endif

I expect handling these correctly is more complex -- I would prefer focusing on matching conditons in this patch, and get back to inverted conditions in a follow-up patch. Is that OK? If we handle inverted conditions, then handling a && b and !a || !b would make sense as well for example.

I agree that it's more complex, but that's why I was asking for it -- I don't think the design you have currently will extend for these sort of cases, and I was hoping to cover more utility with the check to hopefully shake out those forward-looking design considerations. As it stands, I feel like this check is a bit too simplistic to have much value, but if you covered some of these other cases, it would alleviate that worry for me.

[vmiklos]

As it stands, I feel like this check is a bit too simplistic to have much value, but if you covered some of these other cases, it would alleviate that worry for me.

I've now added support for detecting inverted conditions when it comes to #ifdef and #ifndef (see extended documentation and testcases). I'll also check what can I do for #if. I think it would not be too hard to detect just repeated conditions. if I have the full range, then I can look for end of the first line that does not end with \, and that would be a poor man's way to get the #if condition. Let's see if that works in practice.

clang-tidy/readability/RedundantPreprocessorCheck.cpp

20–23

Done now. Nah, it's more about I'm not so fluent with phabricator. :-)

[vmiklos]

In D54349#1294600, @vmiklos wrote:

Let's see if that works in practice.

I've implemented this now, please take a look. (Extended test + docs as well, as usual.) Thanks!

[JonasToth]

docs/clang-tidy/checks/readability-redundant-preprocessor.rst

35

The indendation for these examples (following as well) is not enough. Please use 2 spaces.

[vmiklos]

docs/clang-tidy/checks/readability-redundant-preprocessor.rst

35

Fixed.

[Szelethus]

clang-tidy/readability/RedundantPreprocessorCheck.cpp

57–60

I'm a little confused. To me, it seems like you acquired the condition already -- doesn't ConditionRange actually cover the, well, condition range? This is how I imagined it:

#ifdef CUTE_PANDA_CUBS
       ^~~~~~~~~~~~~~~
       ConditionRange

Why is there a need for getCondition? Is there any? If there is (maybe the acquired text contains other things), can you document it? I haven't played with PPCallbacks much, so I'm fine with being in the wrong.

[vmiklos]

clang-tidy/readability/RedundantPreprocessorCheck.cpp

57–60

ConditionRange covers more than what you expect:

#if FOO == 4
   ^~~~~~~~~
void f();
~~~~~~~~~
#endif
~~~~~~

to find out if the condition of the #if is the same as a previous one, I want to extract just FOO == 4 from that, then deal with that part similar to #ifdef and #ifndef, which are easier as you have a single Token for the condition. But you're right, I should add a comment explaining this.

[Szelethus]

clang-tidy/readability/RedundantPreprocessorCheck.cpp

57–60

Oh my god. There is no tool or a convenient method for this??? I had the displeasure of working with the preprocessor in the last couple months, and I get shocked by things like this almost every day.

Yea, unfortunately you will have to write excessive amount of comments to counterweights the shortcomings of Preprocessor :/

[aaron.ballman]

clang-tidy/readability/ReadabilityTidyModule.cpp

84

Please keep this list sorted alphabetically.

clang-tidy/readability/RedundantPreprocessorCheck.cpp

57–60

This is working around a bug, and I think it would be better to fix that bug instead of jump through these hoops here.

Preprocessor::EvaluateDirectiveExpression() needs to squirrel away the condition range in the DirectiveEvalResult it returns. I'll take a stab at it and report back.

[vmiklos]

clang-tidy/readability/ReadabilityTidyModule.cpp

84

Done.

clang-tidy/readability/RedundantPreprocessorCheck.cpp

57–60

Thanks! I've now rebased this on top of D54450, to be able to drop the ugly getCondition() function.

[aaron.ballman]

clang-tidy/readability/RedundantPreprocessorCheck.cpp

38

I think these diagnostics should be hoisted as private constant members of the class. Something like:
nested redundant %select{#if|#ifdef|#ifndef}0; consider removing it" and previous %select{#if|#ifdef|#ifndef}0 here

test/clang-tidy/readability-redundant-preprocessor.cpp

46

Can you add a test like #if FOO == 3 + 1 as well?

[vmiklos]

clang-tidy/readability/RedundantPreprocessorCheck.cpp

38

Done, I've also added an enum to specify if/ifdef/ifndef to avoid hard-to-read 0/1/2 for these.

[aaron.ballman]

test/clang-tidy/readability-redundant-preprocessor.cpp

53

I didn't describe my test scenario very well -- can you change this line to FOO == 4 but leave the preceding conditional check as FOO == 3 + 1? The goal is to test that this isn't a token-by-token check, but uses the symbolic values instead.

[vmiklos]

test/clang-tidy/readability-redundant-preprocessor.cpp

53

That doesn't work at the moment, since PreprocessorEntry::Condition is just a string, so 3 + 1 won't equal to 4. I think we discussed this above already, and you said:

I think it requires manual work. There's a FIXME in PPCallbacks::If() to pass a list/tree of tokens that would make implementing this reasonable. I'd say it's fine as a follow-up patch.

But later you wrote:

I agree that it's more complex, but that's why I was asking for it -- I don't think the design you have currently will extend for these sort of cases, and I was hoping to cover more utility with the check to hopefully shake out those forward-looking design considerations. As it stands, I feel like this check is a bit too simplistic to have much value, but if you covered some of these other cases, it would alleviate that worry for me.

My hope is that the check with its current "feature set" already has some value, but let me know if I'm too optimistic. :-)

That being said, if I want to support simple cases like "realize that 3 + 1 and 4 is the same" -- do you imagine that would be manually handled in this check or there is already some reusable building block in the codebase to evaluate if two expressions have the same integer value? I guess doing this manually is a lot of work, e.g. the answer for FOO + 4 would be "it depends", so that should not equal to 8, even if FOO happens to be 4, etc.

[aaron.ballman]

I think this check is in good shape for the initial commit. Additional functionality can be added incrementally.

test/clang-tidy/readability-redundant-preprocessor.cpp

53

That's a good point -- sorry about the mistake on my suggestion that this test should pass, I had a temporary lapse. :-)

[vmiklos]

In D54349#1301663, @aaron.ballman wrote:

I think this check is in good shape for the initial commit. Additional functionality can be added incrementally.

OK, thanks. I'll lcommit this once D54450 is committed.

clang-tidy/readability/CMakeLists.txt

add_clang_library(clangTidyReadabilityModule
   IsolateDeclarationCheck.cpp
   MagicNumbersCheck.cpp
   MisleadingIndentationCheck.cpp
   MisplacedArrayIndexCheck.cpp
   NamedParameterCheck.cpp
   NamespaceCommentCheck.cpp
   NonConstParameterCheck.cpp
   ReadabilityTidyModule.cpp
   RedundantControlFlowCheck.cpp
   RedundantDeclarationCheck.cpp
   RedundantFunctionPtrDereferenceCheck.cpp
   RedundantMemberInitCheck.cpp
+  RedundantPreprocessorCheck.cpp
   RedundantSmartptrGetCheck.cpp
   RedundantStringCStrCheck.cpp
   RedundantStringInitCheck.cpp
   SimplifyBooleanExprCheck.cpp
   SimplifySubscriptExprCheck.cpp
   StaticAccessedThroughInstanceCheck.cpp
   StaticDefinitionInAnonymousNamespaceCheck.cpp
   StringCompareCheck.cpp

clang-tidy/readability/ReadabilityTidyModule.cpp

 #include "MisleadingIndentationCheck.h"
 #include "MisplacedArrayIndexCheck.h"
 #include "NamedParameterCheck.h"
 #include "NonConstParameterCheck.h"
 #include "RedundantControlFlowCheck.h"
 #include "RedundantDeclarationCheck.h"
 #include "RedundantFunctionPtrDereferenceCheck.h"
 #include "RedundantMemberInitCheck.h"
+#include "RedundantPreprocessorCheck.h"
 #include "RedundantSmartptrGetCheck.h"
 #include "RedundantStringCStrCheck.h"
 #include "RedundantStringInitCheck.h"
 #include "SimplifyBooleanExprCheck.h"
 #include "SimplifySubscriptExprCheck.h"
 #include "StaticAccessedThroughInstanceCheck.h"
 #include "StaticDefinitionInAnonymousNamespaceCheck.h"
 #include "StringCompareCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<IsolateDeclarationCheck>(
         "readability-isolate-declaration");
     CheckFactories.registerCheck<MagicNumbersCheck>(
         "readability-magic-numbers");
     CheckFactories.registerCheck<MisleadingIndentationCheck>(
         "readability-misleading-indentation");
     CheckFactories.registerCheck<MisplacedArrayIndexCheck>(
         "readability-misplaced-array-index");
     CheckFactories.registerCheck<RedundantFunctionPtrDereferenceCheck>(
         "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>(
         "readability-static-accessed-through-instance");
     CheckFactories.registerCheck<StaticDefinitionInAnonymousNamespaceCheck>(
         "readability-static-definition-in-anonymous-namespace");
     CheckFactories.registerCheck<StringCompareCheck>(
         "readability-string-compare");

clang-tidy/readability/RedundantPreprocessorCheck.h

+//===--- 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

clang-tidy/readability/RedundantPreprocessorCheck.cpp

+//===--- 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 {
+  enum DirectiveKind { DK_If = 0, DK_Ifdef = 1, DK_Ifndef = 2 };
+
+public:
+  explicit RedundantPreprocessorCallbacks(ClangTidyCheck &Check,
+                                          Preprocessor &PP)
+      : Check(Check), PP(PP),
+        WarningDescription("nested redundant %select{#if|#ifdef|#ifndef}0; "
+                           "consider removing it"),
+        NoteDescription("previous %select{#if|#ifdef|#ifndef}0 was here") {}
+
+  void If(SourceLocation Loc, SourceRange ConditionRange,
+          ConditionValueKind ConditionValue) override {
+    StringRef Condition =
+        Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange),
+                             PP.getSourceManager(), PP.getLangOpts());
+    CheckMacroRedundancy(Loc, Condition, IfStack, DK_If, DK_If, true);
+  }
+
+  void Ifdef(SourceLocation Loc, const Token &MacroNameTok,
+             const MacroDefinition &MacroDefinition) override {
+    std::string MacroName = PP.getSpelling(MacroNameTok);
+    CheckMacroRedundancy(Loc, MacroName, IfdefStack, DK_Ifdef, DK_Ifdef, true);
+    CheckMacroRedundancy(Loc, MacroName, IfndefStack, DK_Ifdef, DK_Ifndef,
+                         false);
+  }
+
+  void Ifndef(SourceLocation Loc, const Token &MacroNameTok,
+              const MacroDefinition &MacroDefinition) override {
+    std::string MacroName = PP.getSpelling(MacroNameTok);
+    CheckMacroRedundancy(Loc, MacroName, IfndefStack, DK_Ifndef, DK_Ifndef,
+                         true);
+    CheckMacroRedundancy(Loc, MacroName, IfdefStack, DK_Ifndef, DK_Ifdef,
+                         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,
+                            DirectiveKind WarningKind, DirectiveKind NoteKind,
+                            bool Store) {
+    if (PP.getSourceManager().isInMainFile(Loc)) {
+      for (const auto &Entry : Stack) {
+        if (Entry.Condition == MacroName) {
+          Check.diag(Loc, WarningDescription) << WarningKind;
+          Check.diag(Entry.Loc, NoteDescription, DiagnosticIDs::Note)
+              << NoteKind;
+        }
+      }
+    }
+
+    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;
+  const std::string WarningDescription;
+  const std::string NoteDescription;
+};
+} // namespace
+
+void RedundantPreprocessorCheck::registerPPCallbacks(
+    CompilerInstance &Compiler) {
+  Compiler.getPreprocessor().addPPCallbacks(
+      ::llvm::make_unique<RedundantPreprocessorCallbacks>(
+          *this, Compiler.getPreprocessor()));
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang

docs/ReleaseNotes.rst

 Improvements to clang-rename
 ----------------------------
 
 The improvements are...
 
 Improvements to clang-tidy
 --------------------------
 
 - New :doc:`abseil-duration-division
   <clang-tidy/checks/abseil-duration-division>` check.
 
   Checks for uses of ``absl::Duration`` division that is done in a
   floating-point context, and recommends the use of a function that
   returns a floating-point value.
 
 - New :doc:`abseil-duration-factory-float
   <clang-tidy/checks/abseil-duration-factory-float>` check.
 
   Checks for cases where the floating-point overloads of various
   ``absl::Duration`` factory functions are called when the more-efficient
- New :doc:`readability-const-return-type
   removal of the ``const`` keyword.
 
 - New :doc:`readability-magic-numbers
   <clang-tidy/checks/readability-magic-numbers>` check.
 
   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.
 
   Detects when the integral literal or floating point literal has non-uppercase
   suffix, and suggests to make the suffix uppercase. The list of destination
   suffixes can be optionally provided.
 
 - New alias :doc:`cert-dcl16-c

docs/clang-tidy/checks/list.rst

.. toctree::
    readability-misleading-indentation
    readability-misplaced-array-index
    readability-named-parameter
    readability-non-const-parameter
    readability-redundant-control-flow
    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
    readability-simplify-boolean-expr
    readability-simplify-subscript-expr
    readability-static-accessed-through-instance
    readability-static-definition-in-anonymous-namespace
    readability-string-compare
    readability-uniqueptr-delete-release
    readability-uppercase-literal-suffix
    zircon-temporary-objects

docs/clang-tidy/checks/readability-redundant-preprocessor.rst

+.. 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
+

test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp

+// 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

test/clang-tidy/readability-redundant-preprocessor.h

+#ifndef FOO
+#ifndef FOO // this would warn, but not in a header
+void f();
+#endif
+#endif

test/clang-tidy/readability-redundant-preprocessor.cpp

+// 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 == 3 + 1
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant #if; consider removing it [readability-redundant-preprocessor]
+#if FOO == 3 + 1
+// 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