george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed.
There's quite a lot of code duplication here, I think we could do better with that. Great job modeling semantics though! ================ Comment at: lib/Analysis/CFG.cpp:1320 + auto *MTE = cast<MaterializeTemporaryExpr>(Child); + findConstructionContexts(ConstructionContextLayer::create( + cfg->getBumpVectorContext(), MTE, Layer), ---------------- There are three repeated calls to `findConstructionContexts`, with only the second argument changing. Perhaps a helper lambda closure would help? ================ Comment at: lib/Analysis/CFG.cpp:4959 + const Stmt *S1 = nullptr, *S2 = nullptr, *S3 = nullptr; + const ConstructionContext *CC1 = nullptr; switch (CC->getKind()) { ---------------- begs for a comment differentiating CC from CC1 ================ Comment at: lib/Analysis/CFG.cpp:5014 } if (S1) { OS << ", "; ---------------- three blocks below could really benefit from a helper function. ================ Comment at: lib/Analysis/ConstructionContext.cpp:140 - assert(TopLayer->isLast()); - return create<TemporaryObjectConstructionContext>(C, nullptr, MTE); + // Handle pre-C++17 copy and move elision. + const CXXConstructExpr *ElidedCE = nullptr; ---------------- seems this chunk has a fair bit of code duplication vs. lines 79-99 Repository: rC Clang https://reviews.llvm.org/D47616 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits