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