[clang-tools-extra] r344785 - [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.

[Marek Kurdej via cfe-commits]

Author: mkurdej
Date: Fri Oct 19 08:26:17 2018
New Revision: 344785

URL: http://llvm.org/viewvc/llvm-project?rev=344785&view=rev
Log:
[clang-tidy] Resolve readability-else-after-return false positive for constexpr if.

Summary:
It fixes the false positive when using constexpr if and where else cannot be removed:

Example:
```
  if constexpr (sizeof(int) > 4)
    // ...
    return /* ... */;
  else // This else cannot be removed.
    // ...
    return /* ... */;
```

Reviewers: alexfh, aaron.ballman

Reviewed By: aaron.ballman

Subscribers: lebedev.ri, xazax.hun, cfe-commits

Differential Revision: https://reviews.llvm.org/D53372

Added:
    clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return-if-constexpr.cpp
Modified:
    clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp

Modified: clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp?rev=344785&r1=344784&r2=344785&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/ElseAfterReturnCheck.cpp Fri Oct 19 08:26:17 2018
@@ -1,57 +1,58 @@
-//===--- ElseAfterReturnCheck.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 "ElseAfterReturnCheck.h"
-#include "clang/AST/ASTContext.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/Tooling/FixIt.h"
-
-using namespace clang::ast_matchers;
-
-namespace clang {
-namespace tidy {
-namespace readability {
-
-void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
-  const auto ControlFlowInterruptorMatcher =
-      stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"),
-                 breakStmt().bind("break"),
-                 expr(ignoringImplicit(cxxThrowExpr().bind("throw")))));
-  Finder->addMatcher(
-      compoundStmt(forEach(
-          ifStmt(hasThen(stmt(
-                     anyOf(ControlFlowInterruptorMatcher,
-                           compoundStmt(has(ControlFlowInterruptorMatcher))))),
-                 hasElse(stmt().bind("else")))
-              .bind("if"))),
-      this);
-}
-
-void ElseAfterReturnCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *If = Result.Nodes.getNodeAs<IfStmt>("if");
-  SourceLocation ElseLoc = If->getElseLoc();
-  std::string ControlFlowInterruptor;
-  for (const auto *BindingName : {"return", "continue", "break", "throw"})
-    if (Result.Nodes.getNodeAs<Stmt>(BindingName))
-      ControlFlowInterruptor = BindingName;
-
-  DiagnosticBuilder Diag = diag(ElseLoc, "do not use 'else' after '%0'")
-                           << ControlFlowInterruptor;
-  Diag << tooling::fixit::createRemoval(ElseLoc);
-
-  // 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());
-}
-
-} // namespace readability
-} // namespace tidy
-} // namespace clang
+//===--- ElseAfterReturnCheck.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 "ElseAfterReturnCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Tooling/FixIt.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
+  const auto ControlFlowInterruptorMatcher =
+      stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"),
+                 breakStmt().bind("break"),
+                 expr(ignoringImplicit(cxxThrowExpr().bind("throw")))));
+  Finder->addMatcher(
+      compoundStmt(forEach(
+          ifStmt(unless(isConstexpr()),
+                 hasThen(stmt(
+                     anyOf(ControlFlowInterruptorMatcher,
+                           compoundStmt(has(ControlFlowInterruptorMatcher))))),
+                 hasElse(stmt().bind("else")))
+              .bind("if"))),
+      this);
+}
+
+void ElseAfterReturnCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *If = Result.Nodes.getNodeAs<IfStmt>("if");
+  SourceLocation ElseLoc = If->getElseLoc();
+  std::string ControlFlowInterruptor;
+  for (const auto *BindingName : {"return", "continue", "break", "throw"})
+    if (Result.Nodes.getNodeAs<Stmt>(BindingName))
+      ControlFlowInterruptor = BindingName;
+
+  DiagnosticBuilder Diag = diag(ElseLoc, "do not use 'else' after '%0'")
+                           << ControlFlowInterruptor;
+  Diag << tooling::fixit::createRemoval(ElseLoc);
+
+  // 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());
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang

Added: clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return-if-constexpr.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return-if-constexpr.cpp?rev=344785&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return-if-constexpr.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/readability-else-after-return-if-constexpr.cpp Fri Oct 19 08:26:17 2018
@@ -0,0 +1,22 @@
+// RUN: %check_clang_tidy %s readability-else-after-return %t -- -- -std=c++17
+
+// Constexpr if is an exception to the rule, we cannot remove the else.
+void f() {
+  if (sizeof(int) > 4)
+    return;
+  else
+    return;
+  // CHECK-MESSAGES: [[@LINE-2]]:3: warning: do not use 'else' after 'return'
+
+  if constexpr (sizeof(int) > 4)
+    return;
+  else
+    return;
+
+  if constexpr (sizeof(int) > 4)
+    return;
+  else if constexpr (sizeof(long) > 4)
+    return;
+  else
+    return;
+}