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;
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
