[ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

[njames93]

Summary

Adds a new ASTMatcher condition called 'hasInitStatement()' that matches if, switch and range-for statements with an initializer. Reworked clang-tidy readability-else-after-return to handle variables in the if condition or init statements in c++17 ifs. Also checks if removing the else would affect object lifetimes in the else branch

[malcolm.parsons]

Does Clang support C++20's range-based for statements with initializer?

[njames93]

It does support the new range for loop, I'll add in a case for that in the AST matcher

[aaron.ballman]

Thank you for this!

There's some missing test coverage for the changes to the clang-tidy check and for the new matcher. Also, the new matcher should be registered in clang\lib\ASTMatchers\Dynamic\Registry.cpp (in alphabetical order). Oh, and you should also run clang\docs\tools\dump_ast_matchers.py to regenerate the AST matcher documentation.

clang/include/clang/ASTMatchers/ASTMatchers.h

4300

Missing a full stop at the end of the sentence.

[njames93]

Thanks for the pointers, I'll amend those changes and update the patch, thanks for pointing it out for me. Probably be after boxing day though

[njames93]

I added test cases for the matchers and the check, wasn't able to add the CXXRangeFor to the matcher as that class uses a different dialect for the initializer statement handling. I also couldn't generate the new documentation as the script always fails on my machine, even without the changes i have made, resulting in the ASTMatcher html being empty

[njames93]

Renamed 'hasInitStorage' to 'hasInitStatement'.
Added support for range for loops in 'hasInitStatement'.
'hasInitStatement' now has a match predicate on the init statement.
ReadabilityElseAfterReturn check now checks the init statement is a decl statement before disregarding it.
Added test cases for check and the matcher.
Regenerated the docs for the AST Matchers.

[njames93]

fixed a spelling mistake in unit tests

[aaron.ballman]

LGTM aside from a minor style nit.

clang/include/clang/ASTMatchers/ASTMatchers.h

4325–4326

Can drop the top-level const qualifier on the pointer (we don't typically do that). And can remove the extra parens around the return expression. e.g.,

const Stmt *Init = Node.getInit();
return Init && InnerMatcher.matches(*Init, Finder, Builder);

[njames93]

So I wasn't happy with the vagueness of the else after return check implementation. Almost finished rewriting the check to properly handle the if statements with condition variables based on scope restrictions and where decls are used etc.

int *varInitAndCondition() {
  if (int *X = g(); X != nullptr) {
    return X;
  } else {
    h(&X);
    return X;
  }
}
int *varInitAndConditionUnusedInElseWithDecl() {
  int *Y = g();
  if (int *X = g(); X != nullptr) {
    return X;
  } else {
    int *Y = g();
    h(&Y);
  }
  return Y;
}

transforms to

int *varInitAndCondition() {
  int *X = g();
  if (X != nullptr) {
    return X;
  }
  h(&X);
  return X;
}
int *varInitAndConditionUnusedInElseWithDecl() {
  int *Y = g();
  if (int *X = g(); X != nullptr) {
    return X;
  } else {
    int *Y = g();
    h(&Y);
  }
  return Y;
}

There's a few more test cases but that's the general idea. I'm having a little trouble writing the test cases as I can't run them on my windows machine to verify they report correctly

[njames93]

Think all the test cases are working, but they look like they could do with a clean up from someone who may know the clang-tidy-checks system better than myself

[njames93]

Should also point out that clang-format now has a better time reformatting as i expanded the replacements range to include the entire else block rather than the start and end braces

[aaron.ballman]

In D71846#1796883, @njames93 wrote:

So I wasn't happy with the vagueness of the else after return check implementation. Almost finished rewriting the check to properly handle the if statements with condition variables based on scope restrictions and where decls are used etc.

int *varInitAndCondition() {
  if (int *X = g(); X != nullptr) {
    return X;
  } else {
    h(&X);
    return X;
  }
}
int *varInitAndConditionUnusedInElseWithDecl() {
  int *Y = g();
  if (int *X = g(); X != nullptr) {
    return X;
  } else {
    int *Y = g();
    h(&Y);
  }
  return Y;
}

transforms to

int *varInitAndCondition() {
  int *X = g();
  if (X != nullptr) {
    return X;
  }
  h(&X);
  return X;
}
int *varInitAndConditionUnusedInElseWithDecl() {
  int *Y = g();
  if (int *X = g(); X != nullptr) {
    return X;
  } else {
    int *Y = g();
    h(&Y);
  }
  return Y;
}

There's a few more test cases but that's the general idea. I'm having a little trouble writing the test cases as I can't run them on my windows machine to verify they report correctly

Hmm, I'm not convinced the new behavior is better in all cases, and I suspect this may boil down to user preference (suggesting we may want an option to control the behavior). There are competing stylistic decisions to be made for varInitAndCondition() -- one is the else after a return, but the other is the scope of the variable X. The behavior now forces the user to prefer removing the else after the return over hiding the variable X in a logical scope. For the contrived example provided, this isn't a huge deal because the function is small. However, in real-world code with larger function bodies, I can see wanting to prefer the name hiding. WDYT?

[njames93]

Sorry I didn't make it obvious with the test cases. The fix will never extend the lifetime of variables either in the condition or declared in the else block. It will only apply if the if is the last statement in its scope. Though I should check back through the scope for any declarations that have the same identifier as those in the if statement which would cause an error if they were brought out of the if scope

[njames93]

Adding checks to see if there are any declarations already in the ifs contained scope is really starting to bloat the code while trying to cover every edge case.

[njames93]

Fixed a few more edge cases with test cases

[njames93]

Fixed a few of the test cases failing

[aaron.ballman]

In D71846#1798115, @njames93 wrote:

Sorry I didn't make it obvious with the test cases. The fix will never extend the lifetime of variables either in the condition or declared in the else block. It will only apply if the if is the last statement in its scope. Though I should check back through the scope for any declarations that have the same identifier as those in the if statement which would cause an error if they were brought out of the if scope

Oh, thank you for the clarification, I hadn't picked up on that.

clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp

21

Hmm, I just realized we missed co_return for this check. That's a separate patch, though, so don't feel the need to change anything about this here.

160

The use of dyn_cast_or_null is suspicious in these methods -- if Node is null, then we'll get into the else clause and promptly dereference a null pointer. My guess is that these should be dyn_cast calls instead, but if Node can truly be null, that case should be handled.

165

const auto *, no? (Same below.)

166

const auto * for sure. (Same below.)

168

warn -> Warn
and should have a full stop at the end of the comment. The same applies to the other instances of this comment.

192

const auto * (this comment applies throughout the patch -- we don't want to deduce qualifiers or pointer/references, so they should be spelled out explicitly.)

201

const auto *, no?

238

Please don't use auto here as the type is not spelled out in the initialization. Also, drop the top-level const qualifier. Same elsewhere.

[njames93]

hopefully adhered to all conventions :)

[njames93]

I'm in two minds about issuing a warning when scope restrictions prevent a fix. Do you think creating an option to enable or disable emitting warnings for cases where the scope prevents a fix would be a good idea?

[aaron.ballman]

In D71846#1800344, @njames93 wrote:

I'm in two minds about issuing a warning when scope restrictions prevent a fix. Do you think creating an option to enable or disable emitting warnings for cases where the scope prevents a fix would be a good idea?

It's not uncommon for fixits to only be generated under specific circumstances, so I'm wondering what your concern is with warning when we can't provide a fixit? The cases I am thinking about all seem reasonable to diagnose (are true positives) without fixing, but maybe you have different circumstances in mind.

[njames93]

In D71846#1800381, @aaron.ballman wrote:

In D71846#1800344, @njames93 wrote:

I'm in two minds about issuing a warning when scope restrictions prevent a fix. Do you think creating an option to enable or disable emitting warnings for cases where the scope prevents a fix would be a good idea?

It's not uncommon for fixits to only be generated under specific circumstances, so I'm wondering what your concern is with warning when we can't provide a fixit? The cases I am thinking about all seem reasonable to diagnose (are true positives) without fixing, but maybe you have different circumstances in mind.

Right now an issue is raised for every else after return flag, but not all else after return flags can be fixed due to declaration statements and scope issues. My suggestion is that you can choose to warn about those cases or not. For example a developer may want else after return for when they need to limit the scope and getting a warning for it may be undesirable.

[aaron.ballman]

In D71846#1800401, @njames93 wrote:

In D71846#1800381, @aaron.ballman wrote:

In D71846#1800344, @njames93 wrote:

I'm in two minds about issuing a warning when scope restrictions prevent a fix. Do you think creating an option to enable or disable emitting warnings for cases where the scope prevents a fix would be a good idea?

It's not uncommon for fixits to only be generated under specific circumstances, so I'm wondering what your concern is with warning when we can't provide a fixit? The cases I am thinking about all seem reasonable to diagnose (are true positives) without fixing, but maybe you have different circumstances in mind.

Right now an issue is raised for every else after return flag, but not all else after return flags can be fixed due to declaration statements and scope issues. My suggestion is that you can choose to warn about those cases or not. For example a developer may want else after return for when they need to limit the scope and getting a warning for it may be undesirable.

Okay, I can see the value in having an option for that -- especially given that silencing the diagnostic would add // NOLINT noise to the code. Do you think the option should default to diagnosing all cases, even ones without a fixit?

[njames93]

In D71846#1800411, @aaron.ballman wrote:

In D71846#1800401, @njames93 wrote:

In D71846#1800381, @aaron.ballman wrote:

In D71846#1800344, @njames93 wrote:

I'm in two minds about issuing a warning when scope restrictions prevent a fix. Do you think creating an option to enable or disable emitting warnings for cases where the scope prevents a fix would be a good idea?

It's not uncommon for fixits to only be generated under specific circumstances, so I'm wondering what your concern is with warning when we can't provide a fixit? The cases I am thinking about all seem reasonable to diagnose (are true positives) without fixing, but maybe you have different circumstances in mind.

Right now an issue is raised for every else after return flag, but not all else after return flags can be fixed due to declaration statements and scope issues. My suggestion is that you can choose to warn about those cases or not. For example a developer may want else after return for when they need to limit the scope and getting a warning for it may be undesirable.

Okay, I can see the value in having an option for that -- especially given that silencing the diagnostic would add // NOLINT noise to the code. Do you think the option should default to diagnosing all cases, even ones without a fixit?

Definitely default to diagnosing everything, that's how my current implementation looks right now. Also is there a way to do options as bool because I don't particularly like putting 1 for true in the config file

[aaron.ballman]

In D71846#1800480, @njames93 wrote:

In D71846#1800411, @aaron.ballman wrote:

In D71846#1800401, @njames93 wrote:

In D71846#1800381, @aaron.ballman wrote:

In D71846#1800344, @njames93 wrote:

I'm in two minds about issuing a warning when scope restrictions prevent a fix. Do you think creating an option to enable or disable emitting warnings for cases where the scope prevents a fix would be a good idea?

It's not uncommon for fixits to only be generated under specific circumstances, so I'm wondering what your concern is with warning when we can't provide a fixit? The cases I am thinking about all seem reasonable to diagnose (are true positives) without fixing, but maybe you have different circumstances in mind.

Right now an issue is raised for every else after return flag, but not all else after return flags can be fixed due to declaration statements and scope issues. My suggestion is that you can choose to warn about those cases or not. For example a developer may want else after return for when they need to limit the scope and getting a warning for it may be undesirable.

Okay, I can see the value in having an option for that -- especially given that silencing the diagnostic would add // NOLINT noise to the code. Do you think the option should default to diagnosing all cases, even ones without a fixit?

Definitely default to diagnosing everything, that's how my current implementation looks right now.

SGTM

Also is there a way to do options as bool because I don't particularly like putting 1 for true in the config file

Not to my knowledge.

[njames93]

In D71846#1800481, @aaron.ballman wrote:

In D71846#1800480, @njames93 wrote:

Definitely default to diagnosing everything, that's how my current implementation looks right now.

SGTM

Also is there a way to do options as bool because I don't particularly like putting 1 for true in the config file

Not to my knowledge.

I just did a quick grep and found that all checks that take bools for config options use a 1 for true, 0 for false, so I guess best stick to convention rather than hack at OptionsView to get and set bools

[njames93]

Added option to disable warning when an automatic fix can't be applied due to scope restrictions of variables, default option is to show all warnings

[aaron.ballman]

The new option LGTM but one of the tests can be updated to have a less complex RUN line.

clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp

2–3

You don't need to explicitly set it to 1 here.

[njames93]

removed the unnecessary explicit setting of WarnOnUnfixable value for test case

[njames93]

In D71846#1800503, @aaron.ballman wrote:

The new option LGTM but one of the tests can be updated to have a less complex RUN line.

that was just put in when i was initially testing the behavoiur, removed it now

[njames93]

small reformat

[aaron.ballman]

Still LG -- do you need someone to land this on your behalf?

[njames93]

Yes please, I'm still very new to the llvm contributing system so I would have no idea what to do next.

[aaron.ballman]

Thank you for the patch, I've committed on your behalf in ec3d8e61b527c6312f77a4dab095ffc34e954927

clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.h

 namespace tidy {
 namespace readability {
 
 /// Flags the usages of `else` after `return`.
 ///
 /// http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return
 class ElseAfterReturnCheck : public ClangTidyCheck {
 public:
-  ElseAfterReturnCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  ElseAfterReturnCheck(StringRef Name, ClangTidyContext *Context);
+
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  const bool WarnOnUnfixable;
 };
 
 } // namespace readability
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_ELSEAFTERRETURNCHECK_H

clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp

 //===--- ElseAfterReturnCheck.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 "ElseAfterReturnCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Tooling/FixIt.h"
+#include "llvm/ADT/SmallVector.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace readability {
 
+namespace {
+static const char ReturnStr[] = "return";
+static const char ContinueStr[] = "continue";
+static const char BreakStr[] = "break";
+static const char ThrowStr[] = "throw";
+static const char WarningMessage[] = "do not use 'else' after '%0'";
+static const char WarnOnUnfixableStr[] = "WarnOnUnfixable";
+
+const DeclRefExpr *findUsage(const Stmt *Node, int64_t DeclIdentifier) {
+  if (const auto *DeclRef = dyn_cast<DeclRefExpr>(Node)) {
+    if (DeclRef->getDecl()->getID() == DeclIdentifier) {
+      return DeclRef;
+    }
+  } else {
+    for (const Stmt *ChildNode : Node->children()) {
+      if (const DeclRefExpr *Result = findUsage(ChildNode, DeclIdentifier)) {
+        return Result;
+      }
+    }
+  }
+  return nullptr;
+}
+
+const DeclRefExpr *
+findUsageRange(const Stmt *Node,
+               const llvm::iterator_range<int64_t *> &DeclIdentifiers) {
+  if (const auto *DeclRef = dyn_cast<DeclRefExpr>(Node)) {
+    if (llvm::is_contained(DeclIdentifiers, DeclRef->getDecl()->getID())) {
+      return DeclRef;
+    }
+  } else {
+    for (const Stmt *ChildNode : Node->children()) {
+      if (const DeclRefExpr *Result =
+              findUsageRange(ChildNode, DeclIdentifiers)) {
+        return Result;
+      }
+    }
+  }
+  return nullptr;
+}
+
+const DeclRefExpr *checkInitDeclUsageInElse(const IfStmt *If) {
+  const auto *InitDeclStmt = dyn_cast_or_null<DeclStmt>(If->getInit());
+  if (!InitDeclStmt)
+    return nullptr;
+  if (InitDeclStmt->isSingleDecl()) {
+    const Decl *InitDecl = InitDeclStmt->getSingleDecl();
+    assert(isa<VarDecl>(InitDecl) && "SingleDecl must be a VarDecl");
+    return findUsage(If->getElse(), InitDecl->getID());
+  }
+  llvm::SmallVector<int64_t, 4> DeclIdentifiers;
+  for (const Decl *ChildDecl : InitDeclStmt->decls()) {
+    assert(isa<VarDecl>(ChildDecl) && "Init Decls must be a VarDecl");
+    DeclIdentifiers.push_back(ChildDecl->getID());
+  }
+  return findUsageRange(If->getElse(), DeclIdentifiers);
+}
+
+const DeclRefExpr *checkConditionVarUsageInElse(const IfStmt *If) {
+  const VarDecl *CondVar = If->getConditionVariable();
+  return CondVar != nullptr ? findUsage(If->getElse(), CondVar->getID())
+                            : nullptr;
+}
+
+bool containsDeclInScope(const Stmt *Node) {
+  if (isa<DeclStmt>(Node)) {
+    return true;
+  }
+  if (const auto *Compound = dyn_cast<CompoundStmt>(Node)) {
+    return llvm::any_of(Compound->body(), [](const Stmt *SubNode) {
+      return isa<DeclStmt>(SubNode);
+    });
+  }
+  return false;
+}
+
+void removeElseAndBrackets(DiagnosticBuilder &Diag, ASTContext &Context,
+                           const Stmt *Else, SourceLocation ElseLoc) {
+  auto Remap = [&](SourceLocation Loc) {
+    return Context.getSourceManager().getExpansionLoc(Loc);
+  };
+  auto TokLen = [&](SourceLocation Loc) {
+    return Lexer::MeasureTokenLength(Loc, Context.getSourceManager(),
+                                     Context.getLangOpts());
+  };
+
+  if (const auto *CS = dyn_cast<CompoundStmt>(Else)) {
+    Diag << tooling::fixit::createRemoval(ElseLoc);
+    SourceLocation LBrace = CS->getLBracLoc();
+    SourceLocation RBrace = CS->getRBracLoc();
+    SourceLocation RangeStart =
+        Remap(LBrace).getLocWithOffset(TokLen(LBrace) + 1);
+    SourceLocation RangeEnd = Remap(RBrace).getLocWithOffset(-1);
+
+    llvm::StringRef Repl = Lexer::getSourceText(
+        CharSourceRange::getTokenRange(RangeStart, RangeEnd),
+        Context.getSourceManager(), Context.getLangOpts());
+    Diag << tooling::fixit::createReplacement(CS->getSourceRange(), Repl);
+  } else {
+    SourceLocation ElseExpandedLoc = Remap(ElseLoc);
+    SourceLocation EndLoc = Remap(Else->getEndLoc());
+
+    llvm::StringRef Repl = Lexer::getSourceText(
+        CharSourceRange::getTokenRange(
+            ElseExpandedLoc.getLocWithOffset(TokLen(ElseLoc) + 1), EndLoc),
+        Context.getSourceManager(), Context.getLangOpts());
+    Diag << tooling::fixit::createReplacement(
+        SourceRange(ElseExpandedLoc, EndLoc), Repl);
+  }
+}
+} // namespace
+
+ElseAfterReturnCheck::ElseAfterReturnCheck(StringRef Name,
+                                           ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      WarnOnUnfixable(Options.get(WarnOnUnfixableStr, true)) {}
+
+void ElseAfterReturnCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, WarnOnUnfixableStr, WarnOnUnfixable);
+}
+
 void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
   const auto InterruptsControlFlow =
-      stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"),
-                 breakStmt().bind("break"),
-                 expr(ignoringImplicit(cxxThrowExpr().bind("throw")))));
+      stmt(anyOf(returnStmt().bind(ReturnStr), continueStmt().bind(ContinueStr),
+                 breakStmt().bind(BreakStr),
+                 expr(ignoringImplicit(cxxThrowExpr().bind(ThrowStr)))));
   Finder->addMatcher(
-      compoundStmt(forEach(
-          ifStmt(unless(isConstexpr()),
-                 // FIXME: Explore alternatives for the
-                 // `if (T x = ...) {... return; } else { <use x> }`
-                 // pattern:
-                 //   * warn, but don't fix;
-                 //   * fix by pulling out the variable declaration out of
-                 //     the condition.
-                 unless(hasConditionVariableStatement(anything())),
-                 hasThen(stmt(anyOf(InterruptsControlFlow,
-                                    compoundStmt(has(InterruptsControlFlow))))),
+      compoundStmt(
+          forEach(ifStmt(unless(isConstexpr()),
+                         hasThen(stmt(
+                             anyOf(InterruptsControlFlow,
                          hasElse(stmt().bind("else")))
-              .bind("if"))),
+                      .bind("if")))
+          .bind("cs"),
       this);
 }
 
 void ElseAfterReturnCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *If = Result.Nodes.getNodeAs<IfStmt>("if");
+  const auto *Else = Result.Nodes.getNodeAs<Stmt>("else");
+  const auto *OuterScope = Result.Nodes.getNodeAs<CompoundStmt>("cs");
+
+  bool IsLastInScope = OuterScope->body_back() == If;
   SourceLocation ElseLoc = If->getElseLoc();
-  std::string ControlFlowInterruptor;
-  for (const auto *BindingName : {"return", "continue", "break", "throw"})
+
+  auto ControlFlowInterruptor = [&]() -> llvm::StringRef {
+    for (llvm::StringRef BindingName :
+         {ReturnStr, ContinueStr, BreakStr, ThrowStr})
       if (Result.Nodes.getNodeAs<Stmt>(BindingName))
-      ControlFlowInterruptor = BindingName;
+        return BindingName;
+    return {};
+  }();
+
+  if (!IsLastInScope && containsDeclInScope(Else)) {
+    if (WarnOnUnfixable) {
+      // Warn, but don't attempt an autofix.
+      diag(ElseLoc, WarningMessage) << ControlFlowInterruptor;
+    }
+    return;
+  }
 
-  DiagnosticBuilder Diag = diag(ElseLoc, "do not use 'else' after '%0'")
-                           << ControlFlowInterruptor;
-  Diag << tooling::fixit::createRemoval(ElseLoc);
+  if (checkConditionVarUsageInElse(If) != nullptr) {
+    if (IsLastInScope) {
+      // If the if statement is the last statement its enclosing statements
+      // scope, we can pull the decl out of the if statement.
+      DiagnosticBuilder Diag =
+          diag(ElseLoc, WarningMessage, clang::DiagnosticIDs::Level::Remark)
+      if (checkInitDeclUsageInElse(If) != nullptr) {
+        Diag << tooling::fixit::createReplacement(
+                    SourceRange(If->getIfLoc()),
+                    (tooling::fixit::getText(*If->getInit(), *Result.Context) +
+                     llvm::StringRef("\n"))
+                        .str())
+             << tooling::fixit::createRemoval(If->getInit()->getSourceRange());
+      }
+      const DeclStmt *VDeclStmt = If->getConditionVariableDeclStmt();
+      const VarDecl *VDecl = If->getConditionVariable();
+      std::string Repl =
+          (tooling::fixit::getText(*VDeclStmt, *Result.Context) +
+           llvm::StringRef(";\n") +
+           tooling::fixit::getText(If->getIfLoc(), *Result.Context))
+              .str();
+      Diag << tooling::fixit::createReplacement(SourceRange(If->getIfLoc()),
+                                                Repl)
+           << tooling::fixit::createReplacement(VDeclStmt->getSourceRange(),
+                                                VDecl->getName());
+      removeElseAndBrackets(Diag, *Result.Context, Else, ElseLoc);
+    } else if (WarnOnUnfixable) {
+      // Warn, but don't attempt an autofix.
+      diag(ElseLoc, WarningMessage) << ControlFlowInterruptor;
+    }
+    return;
+  }
 
-  // FIXME: Removing the braces isn't always safe. Do a more careful analysis.
-  // FIXME: Change clang-format to correctly un-indent the code.
-  if (const auto *CS = Result.Nodes.getNodeAs<CompoundStmt>("else"))
-    Diag << tooling::fixit::createRemoval(CS->getLBracLoc())
-         << tooling::fixit::createRemoval(CS->getRBracLoc());
+  if (checkInitDeclUsageInElse(If) != nullptr) {
+    if (IsLastInScope) {
+      // If the if statement is the last statement its enclosing statements
+      // scope, we can pull the decl out of the if statement.
+      DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
+                               << ControlFlowInterruptor;
+      Diag << tooling::fixit::createReplacement(
+                  SourceRange(If->getIfLoc()),
+                  (tooling::fixit::getText(*If->getInit(), *Result.Context) +
+                   "\n" +
+                   tooling::fixit::getText(If->getIfLoc(), *Result.Context))
+                      .str())
+           << tooling::fixit::createRemoval(If->getInit()->getSourceRange());
+      removeElseAndBrackets(Diag, *Result.Context, Else, ElseLoc);
+    } else if (WarnOnUnfixable) {
+      // Warn, but don't attempt an autofix.
+      diag(ElseLoc, WarningMessage) << ControlFlowInterruptor;
+    }
+    return;
+  }
+
+  DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
+                           << ControlFlowInterruptor;
+  removeElseAndBrackets(Diag, *Result.Context, Else, ElseLoc);
 }
 
 } // namespace readability
 } // namespace tidy
 } // namespace clang

clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return-no-warn.cpp

+// RUN: %check_clang_tidy %s readability-else-after-return %t -- \
+// RUN:     -config='{CheckOptions: [ \
+// RUN:         {key: readability-else-after-return.WarnOnUnfixable, value: 0}, \
+// RUN:     ]}'
+
+int h(int);
+
+int lifeTimeExtensionTests(int a) {
+  if (a > 0) {
+    return a;
+  } else {
+    // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+    int b = 0;
+    h(b);
+  }
+  if (int b = a) {
+    return a;
+  } else {
+    // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+    b++;
+  }
+  if (int b = a) { // comment-0
+    // CHECK-FIXES:      {{^}}  int b = a;
+    // CHECK-FIXES-NEXT: {{^}}if (b) { // comment-0
+    return a;
+  } else { // comment-0
+           // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+           // CHECK-FIXES: {{^}}  } // comment-0
+    return b;
+  }
+}

clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp

-// RUN: %check_clang_tidy %s readability-else-after-return %t -- -- -fexceptions
+// RUN: %check_clang_tidy %s readability-else-after-return %t -- -- -fexceptions -std=c++17
 
 namespace std {
 struct string {
   string(const char *);
   ~string();
 };
 } // namespace std
 
 struct my_exception {
   my_exception(const std::string &s);
for (unsigned x = 0; x < 42; ++x) {
     } else { // comment-10
     // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'throw'
     // CHECK-FIXES: {{^}}    } // comment-10
       x++;
     }
   }
 }
 
-extern int *g();
-extern void h(int **x);
+int g();
+int h(int);
 
-int *decl_in_condition() {
-  if (int *x = g()) {
-    return x;
+int declInConditionUsedInElse() {
+  if (int X = g()) { // comment-11
+    // CHECK-FIXES: {{^}}  int X = g();
+    // CHECK-FIXES-NEXT: {{^}}if (X) { // comment-11
+    return X;
+  } else { // comment-11
+           // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+           // CHECK-FIXES: {{^}}  } // comment-11
+    return h(X);
+  }
+}
+int declInConditionUnusedInElse() {
+  if (int X = g()) {
+    return h(X);
+  } else { // comment-12
+           // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+           // CHECK-FIXES: {{^}}  } // comment-12
+    return 0;
+  }
+}
+
+int varInitAndCondition() {
+  if (int X = g(); X != 0) { // comment-13
+    // CHECK-FIXES: {{^}}  int X = g();
+    // CHECK-FIXES-NEXT: {{^}}if ( X != 0) { // comment-13
+    return X;
+  } else { // comment-13
+           // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+           // CHECK-FIXES: {{^}}  } // comment-13
+    return h(X);
+  }
+}
+
+int varInitAndConditionUnusedInElse() {
+  if (int X = g(); X != 0) {
+    return X;
+  } else { // comment-14
+           // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+           // CHECK-FIXES: {{^}}  } // comment-14
+    return 0;
+  }
+}
+
+int initAndCondition() {
+  int X;
+  if (X = g(); X != 0) {
+    return X;
+  } else { // comment-15
+           // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+           // CHECK-FIXES: {{^}}  } // comment-15
+    return h(X);
+  }
+}
+
+int varInitAndConditionUnusedInElseWithDecl() {
+  int Y = g();
+  if (int X = g(); X != 0) {
+    return X;
+  } else { // comment-16
+           // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+           // CHECK-FIXES-NOT: {{^}}  } //comment-16
+    int Y = g();
+    h(Y);
+  }
+  return Y;
+}
+
+int varInitAndCondVarUsedInElse() {
+  if (int X = g(); int Y = g()) { // comment-17
+    // CHECK-FIXES:      {{^}}  int X = g();
+    // CHECK-FIXES-NEXT: {{^}}int Y = g();
+    // CHECK-FIXES-NEXT: {{^}}if ( Y) { // comment-17
+    return X ? X : Y;
+  } else { // comment-17
+           // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+           // CHECK-FIXES: {{^}}  } // comment-17
+    return X ? X : h(Y);
+  }
+}
+
+int lifeTimeExtensionTests(int a) {
+  if (a > 0) {
+    return a;
+  } else {
+      // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+    int b = 0;
+    h(b);
+  }
+  if (int b = a; (b & 1) == 0) {
+    return a;
   } else {
-    h(&x);
-    return x;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+    b++;
+  }
+  if (int b = a; b > 1) { // comment-18
+    // CHECK-FIXES:      {{^}}  int b = a;
+    // CHECK-FIXES-NEXT: {{^}}if ( b > 1) { // comment-18
+    return a;
+  } else { // comment-18
+           // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+           // CHECK-FIXES: {{^}}  } // comment-18
+    return b;
   }
 }

clang/docs/LibASTMatchersReference.html

 
 For example, the array bound of "Chars" in the following example is
 value-dependent.
   template<int Size> int f() { return Size; }
 expr(isValueDependent()) matches return Size
 </pre></td></tr>
 
 
+<tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1Expr.html">Expr</a>&gt;</td><td class="name" onclick="toggle('nullPointerConstant0')"><a name="nullPointerConstant0Anchor">nullPointerConstant</a></td><td></td></tr>
+<tr><td colspan="4" class="doc" id="nullPointerConstant0"><pre>Matches expressions that resolve to a null pointer constant, such as
+GNU's __null, C++11's nullptr, or C's NULL macro.
+
+Given:
+  void *v1 = NULL;
+  void *v2 = nullptr;
+  void *v3 = __null; // GNU extension
+  char *cp = (char *)0;
+  int *ip = 0;
+  int i = 0;
+expr(nullPointerConstant())
+  matches the initializer for v1, v2, v3, cp, and ip. Does not match the
+  initializer for i.
+</pre></td></tr>
+
+
 <tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1FieldDecl.html">FieldDecl</a>&gt;</td><td class="name" onclick="toggle('hasBitWidth0')"><a name="hasBitWidth0Anchor">hasBitWidth</a></td><td>unsigned Width</td></tr>
 <tr><td colspan="4" class="doc" id="hasBitWidth0"><pre>Matches non-static data members that are bit-fields of the specified
 bit width.
 
 Given
   class C {
     int a : 2;
     int b : 4;

 
 
 <tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1ParmVarDecl.html">ParmVarDecl</a>&gt;</td><td class="name" onclick="toggle('hasDefaultArgument0')"><a name="hasDefaultArgument0Anchor">hasDefaultArgument</a></td><td></td></tr>
 <tr><td colspan="4" class="doc" id="hasDefaultArgument0"><pre>Matches a declaration that has default arguments.
 
 Example matches y (matcher = parmVarDecl(hasDefaultArgument()))
 void x(int val) {}
 void y(int val = 0) {}
+
+Deprecated. Use hasInitializer() instead to be able to
+match on the contents of the default argument.  For example:
+
+void x(int val = 7) {}
+void y(int val = 42) {}
+parmVarDecl(hasInitializer(integerLiteral(equals(42))))
+  matches the parameter of y
+
+A matcher such as
+  parmVarDecl(hasInitializer(anything()))
+is equivalent to parmVarDecl(hasDefaultArgument()).
 </pre></td></tr>
 
 
 <tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1QualType.html">QualType</a>&gt;</td><td class="name" onclick="toggle('asString0')"><a name="asString0Anchor">asString</a></td><td>std::string Name</td></tr>
 <tr><td colspan="4" class="doc" id="asString0"><pre>Matches if the matched type is represented by the given string.
 
 Given
   class Y { public: void x(); };
Given
   template&lt;typename T&gt; void A(T t) { T i; }
   A(0);
   A(0U);
 functionDecl(isInstantiated())
   matches 'A(int) {...};' and 'A(unsigned) {...}'.
 </pre></td></tr>
 
 
-<tr><td>Matcher&lt;internal::Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1Expr.html">Expr</a>&gt;&gt;</td><td class="name" onclick="toggle('nullPointerConstant0')"><a name="nullPointerConstant0Anchor">nullPointerConstant</a></td><td></td></tr>
-<tr><td colspan="4" class="doc" id="nullPointerConstant0"><pre>Matches expressions that resolve to a null pointer constant, such as
-GNU's __null, C++11's nullptr, or C's NULL macro.
-
-Given:
-  void *v1 = NULL;
-  void *v2 = nullptr;
-  void *v3 = __null; // GNU extension
-  char *cp = (char *)0;
-  int *ip = 0;
-  int i = 0;
-expr(nullPointerConstant())
-  matches the initializer for v1, v2, v3, cp, and ip. Does not match the
-  initializer for i.
-</pre></td></tr>
-
-
 <tr><td>Matcher&lt;internal::Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1NamedDecl.html">NamedDecl</a>&gt;&gt;</td><td class="name" onclick="toggle('hasAnyName0')"><a name="hasAnyName0Anchor">hasAnyName</a></td><td>StringRef, ..., StringRef</td></tr>
 <tr><td colspan="4" class="doc" id="hasAnyName0"><pre>Matches NamedDecl nodes that have any of the specified names.
 
 This matcher is only provided as a performance optimization of hasName.
     hasAnyName(a, b, c)
  is equivalent to, but faster than
     anyOf(hasName(a), hasName(b), hasName(c))
 </pre></td></tr>
Given
   for (;;) {}
 hasBody(compoundStmt())
   matches 'for (;;) {}'
 with compoundStmt()
   matching '{}'
 </pre></td></tr>
 
 
+<tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1CXXForRangeStmt.html">CXXForRangeStmt</a>&gt;</td><td class="name" onclick="toggle('hasInitStatement2')"><a name="hasInitStatement2Anchor">hasInitStatement</a></td><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1Stmt.html">Stmt</a>&gt; InnerMatcher</td></tr>
+<tr><td colspan="4" class="doc" id="hasInitStatement2"><pre>Matches selection statements with initializer.
+
+Given:
+ void foo() { 
+   if (int i = foobar(); i &gt; 0) {}
+   switch (int i = foobar(); i) {}
+   for (auto&amp; a = get_range(); auto&amp; x : a) {} 
+ }
+ void bar() { 
+   if (foobar() &gt; 0) {}
+   switch (foobar()) {}
+   for (auto&amp; x : get_range()) {} 
+ }
+ifStmt(hasInitStatement(anything()))
+  matches the if statement in foo but not in bar.
+switchStmt(hasInitStatement(anything()))
+  matches the switch statement in foo but not in bar.
+cxxForRangeStmt(hasInitStatement(anything()))
+  matches the range for statement in foo but not in bar.
+</pre></td></tr>
+
+
 <tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1CXXForRangeStmt.html">CXXForRangeStmt</a>&gt;</td><td class="name" onclick="toggle('hasLoopVariable0')"><a name="hasLoopVariable0Anchor">hasLoopVariable</a></td><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1VarDecl.html">VarDecl</a>&gt; InnerMatcher</td></tr>
 <tr><td colspan="4" class="doc" id="hasLoopVariable0"><pre>Matches the initialization statement of a for loop.
 
 Example:
     forStmt(hasLoopVariable(anything()))
 matches 'int x' in
     for (int x : a) { }
 </pre></td></tr>

 <tr><td colspan="4" class="doc" id="hasElse0"><pre>Matches the else-statement of an if statement.
 
 Examples matches the if statement
   (matcher = ifStmt(hasElse(cxxBoolLiteral(equals(true)))))
   if (false) false; else true;
 </pre></td></tr>
 
 
+<tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1IfStmt.html">IfStmt</a>&gt;</td><td class="name" onclick="toggle('hasInitStatement0')"><a name="hasInitStatement0Anchor">hasInitStatement</a></td><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1Stmt.html">Stmt</a>&gt; InnerMatcher</td></tr>
+<tr><td colspan="4" class="doc" id="hasInitStatement0"><pre>Matches selection statements with initializer.
+
+Given:
+ void foo() { 
+   if (int i = foobar(); i &gt; 0) {}
+   switch (int i = foobar(); i) {}
+   for (auto&amp; a = get_range(); auto&amp; x : a) {} 
+ }
+ void bar() { 
+   if (foobar() &gt; 0) {}
+   switch (foobar()) {}
+   for (auto&amp; x : get_range()) {} 
+ }
+ifStmt(hasInitStatement(anything()))
+  matches the if statement in foo but not in bar.
+switchStmt(hasInitStatement(anything()))
+  matches the switch statement in foo but not in bar.
+cxxForRangeStmt(hasInitStatement(anything()))
+  matches the range for statement in foo but not in bar.
+</pre></td></tr>
+
+
 <tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1IfStmt.html">IfStmt</a>&gt;</td><td class="name" onclick="toggle('hasThen0')"><a name="hasThen0Anchor">hasThen</a></td><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1Stmt.html">Stmt</a>&gt; InnerMatcher</td></tr>
 <tr><td colspan="4" class="doc" id="hasThen0"><pre>Matches the then-statement of an if statement.
 
 Examples matches the if statement
   (matcher = ifStmt(hasThen(cxxBoolLiteral(equals(true)))))
   if (false) true; else false;
 </pre></td></tr>
 

 <tr><td colspan="4" class="doc" id="hasCondition4"><pre>Matches the condition expression of an if statement, for loop,
 switch statement or conditional operator.
 
 Example matches true (matcher = hasCondition(cxxBoolLiteral(equals(true))))
   if (true) {}
 </pre></td></tr>
 
 
+<tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1SwitchStmt.html">SwitchStmt</a>&gt;</td><td class="name" onclick="toggle('hasInitStatement1')"><a name="hasInitStatement1Anchor">hasInitStatement</a></td><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1Stmt.html">Stmt</a>&gt; InnerMatcher</td></tr>
+<tr><td colspan="4" class="doc" id="hasInitStatement1"><pre>Matches selection statements with initializer.
+
+Given:
+ void foo() { 
+   if (int i = foobar(); i &gt; 0) {}
+   switch (int i = foobar(); i) {}
+   for (auto&amp; a = get_range(); auto&amp; x : a) {} 
+ }
+ void bar() { 
+   if (foobar() &gt; 0) {}
+   switch (foobar()) {}
+   for (auto&amp; x : get_range()) {} 
+ }
+ifStmt(hasInitStatement(anything()))
+  matches the if statement in foo but not in bar.
+switchStmt(hasInitStatement(anything()))
+  matches the switch statement in foo but not in bar.
+cxxForRangeStmt(hasInitStatement(anything()))
+  matches the range for statement in foo but not in bar.
+</pre></td></tr>
+
+
 <tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1TagType.html">TagType</a>&gt;</td><td class="name" onclick="toggle('hasDeclaration4')"><a name="hasDeclaration4Anchor">hasDeclaration</a></td><td>const Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1Decl.html">Decl</a>&gt;  InnerMatcher</td></tr>
 <tr><td colspan="4" class="doc" id="hasDeclaration4"><pre>Matches a node if the declaration associated with that node
 matches the given matcher.
 
 The associated declaration is:
 - for type nodes, the declaration of the underlying type
 - for CallExpr, the declaration of the callee
 - for MemberExpr, the declaration of the referenced member
The matcher:
   cxxRecordDecl(hasName("::A"),
                 findAll(cxxRecordDecl(isDefinition()).bind("m")))
 will generate results for A, B and C.
 
 Usable as: Any Matcher
 </pre></td></tr>
 
 
+<tr><td>Matcher&lt;T&gt;</td><td class="name" onclick="toggle('traverse0')"><a name="traverse0Anchor">traverse</a></td><td>ast_type_traits::TraversalKind TK, const Matcher&lt;T&gt;  InnerMatcher</td></tr>
+<tr><td colspan="4" class="doc" id="traverse0"><pre>Causes all nested matchers to be matched with the specified traversal kind.
+
+Given
+  void foo()
+  {
+      int i = 3.0;
+  }
+The matcher
+  traverse(ast_type_traits::TK_IgnoreImplicitCastsAndParentheses,
+    varDecl(hasInitializer(floatLiteral().bind("init")))
+  )
+matches the variable declaration with "init" bound to the "3.0".
+</pre></td></tr>
+
+
 <tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1TypedefNameDecl.html">TypedefNameDecl</a>&gt;</td><td class="name" onclick="toggle('hasType2')"><a name="hasType2Anchor">hasType</a></td><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1QualType.html">QualType</a>&gt; InnerMatcher</td></tr>
 <tr><td colspan="4" class="doc" id="hasType2"><pre>Matches if the expression's or declaration's type matches a type
 matcher.
 
 Example matches x (matcher = expr(hasType(cxxRecordDecl(hasName("X")))))
             and z (matcher = varDecl(hasType(cxxRecordDecl(hasName("X")))))
             and U (matcher = typedefDecl(hasType(asString("int")))
             and friend class X (matcher = friendDecl(hasType("X"))

clang/include/clang/ASTMatchers/ASTMatchers.h

 ///   matches the if statement in baz.
 AST_POLYMORPHIC_MATCHER(isConstexpr,
                         AST_POLYMORPHIC_SUPPORTED_TYPES(VarDecl,
                                                         FunctionDecl,
                                                         IfStmt)) {
   return Node.isConstexpr();
 }
 
+/// Matches selection statements with initializer.
+///
+/// Given:
+/// \code
+///  void foo() {
+///    if (int i = foobar(); i > 0) {}
+///    switch (int i = foobar(); i) {}
+///    for (auto& a = get_range(); auto& x : a) {}
+///  }
+///  void bar() {
+///    if (foobar() > 0) {}
+///    switch (foobar()) {}
+///    for (auto& x : get_range()) {}
+///  }
+/// \endcode
+/// ifStmt(hasInitStatement(anything()))
+///   matches the if statement in foo but not in bar.
+/// switchStmt(hasInitStatement(anything()))
+///   matches the switch statement in foo but not in bar.
+/// cxxForRangeStmt(hasInitStatement(anything()))
+///   matches the range for statement in foo but not in bar.
+AST_POLYMORPHIC_MATCHER_P(hasInitStatement,
+                          AST_POLYMORPHIC_SUPPORTED_TYPES(IfStmt, SwitchStmt,
+                                                          CXXForRangeStmt),
+                          internal::Matcher<Stmt>, InnerMatcher) {
+  const Stmt *Init = Node.getInit();
+  return Init != nullptr && InnerMatcher.matches(*Init, Finder, Builder);
+}
+
 /// Matches the condition expression of an if statement, for loop,
 /// switch statement or conditional operator.
 ///
 /// Example matches true (matcher = hasCondition(cxxBoolLiteral(equals(true))))
 /// \code
 ///   if (true) {}
 /// \endcode
 AST_POLYMORPHIC_MATCHER_P(

clang/lib/ASTMatchers/Dynamic/Registry.cpp

RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(hasFalseExpression);
   REGISTER_MATCHER(hasGlobalStorage);
   REGISTER_MATCHER(hasImplicitDestinationType);
   REGISTER_MATCHER(hasInClassInitializer);
   REGISTER_MATCHER(hasIncrement);
   REGISTER_MATCHER(hasIndex);
   REGISTER_MATCHER(hasInit);
   REGISTER_MATCHER(hasInitializer);
+  REGISTER_MATCHER(hasInitStatement);
   REGISTER_MATCHER(hasKeywordSelector);
   REGISTER_MATCHER(hasLHS);
   REGISTER_MATCHER(hasLocalQualifiers);
   REGISTER_MATCHER(hasLocalStorage);
   REGISTER_MATCHER(hasLoopInit);
   REGISTER_MATCHER(hasLoopVariable);
   REGISTER_MATCHER(hasMethod);
   REGISTER_MATCHER(hasName);

clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

TEST(isConstexpr, MatchesConstexprDeclarations) {
   EXPECT_TRUE(matches("constexpr int bar();",
                       functionDecl(hasName("bar"), isConstexpr())));
   EXPECT_TRUE(matches("void baz() { if constexpr(1 > 0) {} }",
                       ifStmt(isConstexpr()), LanguageMode::Cxx17OrLater));
   EXPECT_TRUE(notMatches("void baz() { if (1 > 0) {} }", ifStmt(isConstexpr()),
                          LanguageMode::Cxx17OrLater));
 }
 
+TEST(hasInitStatement, MatchesSelectionInitializers) {
+  EXPECT_TRUE(matches("void baz() { if (int i = 1; i > 0) {} }",
+                      ifStmt(hasInitStatement(anything())),
+                      LanguageMode::Cxx17OrLater));
+  EXPECT_TRUE(notMatches("void baz() { if (int i = 1) {} }",
+                         ifStmt(hasInitStatement(anything()))));
+  EXPECT_TRUE(notMatches("void baz() { if (1 > 0) {} }",
+                         ifStmt(hasInitStatement(anything()))));
+  EXPECT_TRUE(matches(
+      "void baz(int i) { switch (int j = i; j) { default: break; } }",
+      switchStmt(hasInitStatement(anything())), LanguageMode::Cxx17OrLater));
+  EXPECT_TRUE(notMatches("void baz(int i) { switch (i) { default: break; } }",
+                         switchStmt(hasInitStatement(anything()))));
+}
+
+TEST(hasInitStatement, MatchesRangeForInitializers) {
+  EXPECT_TRUE(matches("void baz() {"
+                      "int items[] = {};"
+                      "for (auto &arr = items; auto &item : arr) {}"
+                      "}",
+                      cxxForRangeStmt(hasInitStatement(anything())),
+                      LanguageMode::Cxx2aOrLater));
+  EXPECT_TRUE(notMatches("void baz() {"
+                         "int items[] = {};"
+                         "for (auto &item : items) {}"
+                         "}",
+                         cxxForRangeStmt(hasInitStatement(anything()))));
+}
+
 TEST(TemplateArgumentCountIs, Matches) {
   EXPECT_TRUE(
     matches("template<typename T> struct C {}; C<int> c;",
             classTemplateSpecializationDecl(templateArgumentCountIs(1))));
   EXPECT_TRUE(
     notMatches("template<typename T> struct C {}; C<int> c;",
                classTemplateSpecializationDecl(templateArgumentCountIs(2))));