[clang] [clang][dataflow] Refactor `PropagateResultObject()` with a switch statement. (PR #88865)

via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 16 02:54:46 PDT 2024


https://github.com/martinboehme created https://github.com/llvm/llvm-project/pull/88865

See also discussion in #88726.


>From 3da6980d1957c19bdb821c6059c032b1e1c55863 Mon Sep 17 00:00:00 2001
From: Martin Braenne <mboehme at google.com>
Date: Tue, 16 Apr 2024 09:52:35 +0000
Subject: [PATCH] [clang][dataflow] Refactor `PropagateResultObject()` with a
 switch statement.

See also discussion in #88726.
---
 .../FlowSensitive/DataflowEnvironment.cpp     | 128 ++++++++++--------
 1 file changed, 71 insertions(+), 57 deletions(-)

diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index ee2581143e1141..86b42b12c93e94 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -500,72 +500,86 @@ class ResultObjectVisitor : public RecursiveASTVisitor<ResultObjectVisitor> {
 
     ResultObjectMap[E] = Loc;
 
-    // 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).
-    if (isa<CXXConstructExpr>(E) || isa<CallExpr>(E) || isa<LambdaExpr>(E) ||
-        isa<CXXDefaultArgExpr>(E) || isa<CXXDefaultInitExpr>(E) ||
-        isa<CXXStdInitializerListExpr>(E)) {
-      return;
-    }
-    if (auto *Op = dyn_cast<BinaryOperator>(E);
-        Op && Op->getOpcode() == BO_Cmp) {
-      // Builtin `<=>` returns a `std::strong_ordering` object.
-      return;
-    }
-
-    if (auto *InitList = dyn_cast<InitListExpr>(E)) {
-      if (!InitList->isSemanticForm())
+    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:
+        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);
         return;
-      if (InitList->isTransparent()) {
-        PropagateResultObject(InitList->getInit(0), Loc);
+      }
+      case Stmt::BinaryConditionalOperatorClass:
+      case Stmt::ConditionalOperatorClass: {
+        auto *Cond = cast<AbstractConditionalOperator>(E);
+        PropagateResultObject(Cond->getTrueExpr(), Loc);
+        PropagateResultObject(Cond->getFalseExpr(), Loc);
         return;
       }
+      case Stmt::InitListExprClass: {
+        auto *InitList = cast<InitListExpr>(E);
+        if (!InitList->isSemanticForm())
+          return;
+        if (InitList->isTransparent()) {
+          PropagateResultObject(InitList->getInit(0), Loc);
+          return;
+        }
 
-      RecordInitListHelper InitListHelper(InitList);
+        RecordInitListHelper InitListHelper(InitList);
 
-      for (auto [Base, Init] : InitListHelper.base_inits()) {
-        assert(Base->getType().getCanonicalType() ==
-               Init->getType().getCanonicalType());
+        for (auto [Base, Init] : InitListHelper.base_inits()) {
+          assert(Base->getType().getCanonicalType() ==
+                 Init->getType().getCanonicalType());
 
-        // Storage location for the base class is the same as that of the
-        // derived class because we "flatten" the object hierarchy and put all
-        // fields in `RecordStorageLocation` of the derived class.
-        PropagateResultObject(Init, Loc);
-      }
+          // Storage location for the base class is the same as that of the
+          // derived class because we "flatten" the object hierarchy and put all
+          // fields in `RecordStorageLocation` of the derived class.
+          PropagateResultObject(Init, Loc);
+        }
 
-      for (auto [Field, Init] : InitListHelper.field_inits()) {
-        // Fields of non-record type are handled in
-        // `TransferVisitor::VisitInitListExpr()`.
-        if (!Field->getType()->isRecordType())
-          continue;
-        PropagateResultObject(
-            Init, cast<RecordStorageLocation>(Loc->getChild(*Field)));
+        for (auto [Field, Init] : InitListHelper.field_inits()) {
+          // Fields of non-record type are handled in
+          // `TransferVisitor::VisitInitListExpr()`.
+          if (!Field->getType()->isRecordType())
+            continue;
+          PropagateResultObject(
+              Init, cast<RecordStorageLocation>(Loc->getChild(*Field)));
+        }
+        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);
+        return;
       }
-      return;
-    }
-
-    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;
     }
-
-    // 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:



More information about the cfe-commits mailing list