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

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added a reviewer: clang-tools-extra.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Adds a new ASTMatcher condition called 'hasInitStorage()' that matches if and switch statements with initializer. Then tweaks the clang-tidy 'readability-else-after-return' check to ignore if statements with an initializer as removing the else branch would result in the condition variable going out of scope


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71846

Files:
  clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
  clang/include/clang/ASTMatchers/ASTMatchers.h


Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===================================================================
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -4297,6 +4297,22 @@
   return Node.isConstexpr();
 }
 
+/// Matches selection statements with initializer
+///
+/// Given:
+/// \code
+///  if (int foo = bar(); bar > 0) {}
+///  switch (int baz = bar(); baz - 1) {}
+/// \endcode
+/// IfStmt(hasInitStorage())
+///   matches the declaration of foo.
+/// SwitchStmt(hasInitStorage())
+///   matches the declaration of baz.
+AST_POLYMORPHIC_MATCHER(hasInitStorage,
+                        AST_POLYMORPHIC_SUPPORTED_TYPES(IfStmt, SwitchStmt)) {
+  return Node.hasInitStorage();
+}
+
 /// Matches the condition expression of an if statement, for loop,
 /// switch statement or conditional operator.
 ///
Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -26,12 +26,14 @@
       compoundStmt(forEach(
           ifStmt(unless(isConstexpr()),
                  // FIXME: Explore alternatives for the
-                 // `if (T x = ...) {... return; } else { <use x> }`
-                 // pattern:
+                 // `if (T x = ...) {... return; } else { <use x> }` and
+                 // 'if (T x = ...; cond) {... return; } else { use <x> }'
+                 // patterns:
                  //   * warn, but don't fix;
                  //   * fix by pulling out the variable declaration out of
                  //     the condition.
                  unless(hasConditionVariableStatement(anything())),
+                 unless(hasInitStorage()),
                  hasThen(stmt(anyOf(InterruptsControlFlow,
                                     compoundStmt(has(InterruptsControlFlow))))),
                  hasElse(stmt().bind("else")))


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D71846.235172.patch
Type: text/x-patch
Size: 2049 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191223/1e800a2d/attachment.bin>