Skip to content

Revert "[clang][dataflow] Refactor PropagateResultObject() with a switch statement." #89176

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 18, 2024

Conversation

martinboehme
Copy link
Contributor

Reverts #88865

There were failing tests in the CI that I didn't notice. Sorry.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:dataflow Clang Dataflow Analysis framework - https://clang.llvm.org/docs/DataFlowAnalysisIntro.html clang:analysis labels Apr 18, 2024
@martinboehme martinboehme merged commit b5f2cec into main Apr 18, 2024
@martinboehme martinboehme deleted the revert-88865-propagate_result_object_switch branch April 18, 2024 07:23
@llvmbot
Copy link
Member

llvmbot commented Apr 18, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-analysis

Author: None (martinboehme)

Changes

Reverts llvm/llvm-project#88865

There were failing tests in the CI that I didn't notice. Sorry.


Full diff: https://github.com/llvm/llvm-project/pull/89176.diff

1 Files Affected:

  • (modified) clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp (+36-50)
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index e5aaa8fea4803f..3f1600d9ac5d87 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -414,52 +414,25 @@ class ResultObjectVisitor : public RecursiveASTVisitor<ResultObjectVisitor> {
 
     ResultObjectMap[E] = Loc;
 
-    switch (E->getStmtClass()) {
     // The following AST node kinds are "original initializers": They are the
     // lowest-level AST node that initializes a given object, and nothing
     // below them can initialize the same object (or part of it).
-    case Stmt::CXXConstructExprClass:
-    case Stmt::CallExprClass:
-    case Stmt::LambdaExprClass:
-    case Stmt::CXXDefaultArgExprClass:
-    case Stmt::CXXDefaultInitExprClass:
-    case Stmt::CXXStdInitializerListExprClass:
-    // We treat `BuiltinBitCastExpr` as an "original initializer" too as it may
-    // not even be casting from a record type -- and even if it is, the two
-    // objects are in general of unrelated type.
-    case Stmt::BuiltinBitCastExprClass:
-      return;
-    case Stmt::BinaryOperatorClass: {
-      auto *Op = cast<BinaryOperator>(E);
-      if (Op->getOpcode() == BO_Cmp) {
-        // Builtin `<=>` returns a `std::strong_ordering` object. We
-        // consider this to be an "original" initializer too (see above).
-        return;
-      }
-      if (Op->isCommaOp()) {
-        PropagateResultObject(Op->getRHS(), Loc);
-        return;
-      }
-      // We don't expect any other binary operators to produce a record
-      // prvalue, so if we get here, we've hit some case we don't know
-      // about.
-      assert(false);
+    if (isa<CXXConstructExpr>(E) || isa<CallExpr>(E) || isa<LambdaExpr>(E) ||
+        isa<CXXDefaultArgExpr>(E) || isa<CXXDefaultInitExpr>(E) ||
+        isa<CXXStdInitializerListExpr>(E) ||
+        // We treat `BuiltinBitCastExpr` as an "original initializer" too as
+        // it may not even be casting from a record type -- and even if it is,
+        // the two objects are in general of unrelated type.
+        isa<BuiltinBitCastExpr>(E)) {
       return;
     }
-    case Stmt::BinaryConditionalOperatorClass:
-    case Stmt::ConditionalOperatorClass: {
-      auto *Cond = cast<AbstractConditionalOperator>(E);
-      PropagateResultObject(Cond->getTrueExpr(), Loc);
-      PropagateResultObject(Cond->getFalseExpr(), Loc);
-      return;
-    }
-    case Stmt::StmtExprClass: {
-      auto *SE = cast<StmtExpr>(E);
-      PropagateResultObject(cast<Expr>(SE->getSubStmt()->body_back()), Loc);
+    if (auto *Op = dyn_cast<BinaryOperator>(E);
+        Op && Op->getOpcode() == BO_Cmp) {
+      // Builtin `<=>` returns a `std::strong_ordering` object.
       return;
     }
-    case Stmt::InitListExprClass: {
-      auto *InitList = cast<InitListExpr>(E);
+
+    if (auto *InitList = dyn_cast<InitListExpr>(E)) {
       if (!InitList->isSemanticForm())
         return;
       if (InitList->isTransparent()) {
@@ -489,20 +462,33 @@ class ResultObjectVisitor : public RecursiveASTVisitor<ResultObjectVisitor> {
       }
       return;
     }
-    default: {
-      // All other expression nodes that propagate a record prvalue should
-      // have exactly one child.
-      SmallVector<Stmt *, 1> Children(E->child_begin(), E->child_end());
-      LLVM_DEBUG({
-        if (Children.size() != 1)
-          E->dump();
-      });
-      assert(Children.size() == 1);
-      for (Stmt *S : Children)
-        PropagateResultObject(cast<Expr>(S), Loc);
+
+    if (auto *Op = dyn_cast<BinaryOperator>(E); Op && Op->isCommaOp()) {
+      PropagateResultObject(Op->getRHS(), Loc);
       return;
     }
+
+    if (auto *Cond = dyn_cast<AbstractConditionalOperator>(E)) {
+      PropagateResultObject(Cond->getTrueExpr(), Loc);
+      PropagateResultObject(Cond->getFalseExpr(), Loc);
+      return;
     }
+
+    if (auto *SE = dyn_cast<StmtExpr>(E)) {
+      PropagateResultObject(cast<Expr>(SE->getSubStmt()->body_back()), Loc);
+      return;
+    }
+
+    // All other expression nodes that propagate a record prvalue should have
+    // exactly one child.
+    SmallVector<Stmt *, 1> Children(E->child_begin(), E->child_end());
+    LLVM_DEBUG({
+      if (Children.size() != 1)
+        E->dump();
+    });
+    assert(Children.size() == 1);
+    for (Stmt *S : Children)
+      PropagateResultObject(cast<Expr>(S), Loc);
   }
 
 private:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:analysis clang:dataflow Clang Dataflow Analysis framework - https://clang.llvm.org/docs/DataFlowAnalysisIntro.html clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants