rsmith added inline comments.
================ Comment at: clang/lib/Sema/SemaConcept.cpp:744 ArrayRef<const Expr *> E) { - assert(E.size() != 0); - auto First = fromConstraintExpr(S, D, E[0]); ---------------- Might be useful to keep this assert; it will make it clearer to future readers that this function isn't intended to handle this case. ================ Comment at: clang/lib/Sema/SemaConcept.cpp:751 + return None; + Conjunction.emplace(S.Context, std::move(*Conjunction), std::move(*Next), + CCK_Conjunction); ---------------- Doesn't this have a use-after-destruction bug? I'd expect `emplace` to first destroy its contained value and then construct a new one; in this case the second constructor argument looks like it'll be a reference to a destroyed object. I assume that's why the old code did the two-step construct and assign dance. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106907/new/ https://reviews.llvm.org/D106907 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits