rjmccall accepted this revision. rjmccall added inline comments. This revision is now accepted and ready to land.
================ Comment at: clang/lib/Sema/SemaExpr.cpp:15692 + MaybeODRUseExprSet LocalMaybeODRUseExprs; + std::swap(LocalMaybeODRUseExprs, MaybeODRUseExprs); + ---------------- erik.pilkington wrote: > rjmccall wrote: > > It looks like `SmallPtrSet`'s move constructor does actually guarantee to > > leave the source empty; you could probably just assert that. But I don't > > think there's an algorithmic cost, so this is fine, too. > > > > Please leave an assertion at the bottom of this function that the set is > > empty. > If you have to actually check the constructor to verify the client code is > doing the right thing then it makes more sense to just be explicit, IMO. New > patch adds the assert though. If it were perf-significant I might disagree, but it shouldn't be, so this is fine. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59670/new/ https://reviews.llvm.org/D59670 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
