[clang-tidy] Sequence init statements, declarations, and conditions correctly in if, switch, and while

[mboehme]

Summary

Fixes https://bugs.llvm.org/show_bug.cgi?id=36516.

[aaron.ballman]

clang-tidy/utils/ExprSequence.cpp

150

Should we also handle switch statements at the same time?

[alexfh]

LG modulo outstanding comments.

[mboehme]

Add support for switch statement.

While I'm here, also add support for the init statement in an if statement.

[mboehme]

clang-tidy/utils/ExprSequence.cpp

150

Good point.

While I was doing this, I noticed that we were also not sequencing init statements in if statements correctly, so I've added code for that as well.

The description of the change is now too narrow, so I'll change that in a moment.

[mboehme]

Any further comments? This is marked ready to land, but I've made some non-trivial changes since then, so I didn't just want to land this.

[alexfh]

Still LG

test/clang-tidy/bugprone-use-after-move.cpp

1141

nit: clang-format this, please.

[mboehme]

Apply clang-format.

clang-tidy/utils/ExprSequence.cpp

if (const auto *BO = dyn_cast<BinaryOperator>(Parent)) {
       }
     } else if (const auto *ForRange = dyn_cast<CXXForRangeStmt>(Parent)) {
       // Range-based for: Loop variable declaration is sequenced before the
       // body. (We need this rule because these get placed in the same
       // CFGBlock.)
       if (S == ForRange->getLoopVarStmt())
         return ForRange->getBody();
     } else if (const auto *TheIfStmt = dyn_cast<IfStmt>(Parent)) {
-      // If statement: If a variable is declared inside the condition, the
-      // expression used to initialize the variable is sequenced before the
-      // evaluation of the condition.
+      // If statement:
+      // - Sequence init statement before variable declaration.
+      // - Sequence variable declaration (along with the expression used to
+      //   initialize it) before the evaluation of the condition.
+      if (S == TheIfStmt->getInit())
+        return TheIfStmt->getConditionVariableDeclStmt();
       if (S == TheIfStmt->getConditionVariableDeclStmt())
         return TheIfStmt->getCond();
+    } else if (const auto *TheSwitchStmt = dyn_cast<SwitchStmt>(Parent)) {
+      // Ditto for switch statements.
+      if (S == TheSwitchStmt->getInit())
+        return TheSwitchStmt->getConditionVariableDeclStmt();
+      if (S == TheSwitchStmt->getConditionVariableDeclStmt())
+        return TheSwitchStmt->getCond();
+    } else if (const auto *TheWhileStmt = dyn_cast<WhileStmt>(Parent)) {
+      // While statement: Sequence variable declaration (along with the
+      // expression used to initialize it) before the evaluation of the
+      // condition.
+      if (S == TheWhileStmt->getConditionVariableDeclStmt())
+        return TheWhileStmt->getCond();
     }
   }
 
   return nullptr;
 }
 
 const Stmt *ExprSequence::resolveSyntheticStmt(const Stmt *S) const {
   if (SyntheticStmtSourceMap.count(S))

test/clang-tidy/bugprone-use-after-move.cpp

-// RUN: %check_clang_tidy %s bugprone-use-after-move %t -- -- -std=c++11 -fno-delayed-template-parsing
+// RUN: %check_clang_tidy %s bugprone-use-after-move %t -- -- -std=c++17 -fno-delayed-template-parsing
 
 typedef decltype(nullptr) nullptr_t;
 
 namespace std {
 typedef unsigned size_t;
 
 template <typename T>
 struct unique_ptr {

 // A range-based for sequences the loop variable declaration before the body.
 void forRangeSequences() {
   A v[2] = {A(), A()};
   for (A &a : v) {
     std::move(a);
   }
 }
 
-// If a variable is declared in an if statement, the declaration of the variable
-// (which is treated like a reinitialization by the check) is sequenced before
-// the evaluation of the condition (which constitutes a use).
-void ifStmtSequencesDeclAndCondition() {
+// If a variable is declared in an if, while or switch statement, the init
+// statement (for if and switch) is sequenced before the variable declaration,
+// which in turn is sequenced before the evaluation of the condition.
+void ifWhileAndSwitchSequenceInitDeclAndCondition() {
   for (int i = 0; i < 10; ++i) {
-    if (A a = A()) {
-      std::move(a);
-    }
+    A a1;
+    if (A a2 = std::move(a1)) {
+      std::move(a2);
+    }
+  }
+  for (int i = 0; i < 10; ++i) {
+    A a1;
+    if (A a2 = std::move(a1); A a3 = std::move(a2)) {
+      std::move(a3);
+    }
+  }
+  while (A a = A()) {
+  for (int i = 0; i < 10; ++i) {
+    A a1;
+    switch (A a2 = a1; A a3 = std::move(a2)) {
+      case true:
+        std::move(a3);
+    }
   }
 }
 
 namespace PR33020 {
 class D {
   ~D();
 };
 struct A {