mboehme marked 2 inline comments as done. mboehme added a comment. In D153006#4424980 <https://reviews.llvm.org/D153006#4424980>, @xazax.hun wrote:
> I am not opposed to this solution, but I do anticipate some performance > problems in the future. I wonder if copy-on-write, or some persistent data > structures where copying is cheap would be a better strategy long term. But > getting it right first and fast after sounds like a good strategy :) Agreed - I think "right first and fast after" is the way to go here. Also, we should wait and see whether this really does turn out to be a performance issue. We'll only be doing this for types that are copyable, and by their nature, these tend to have relatively few fields, as programmers try to avoid making types copyable that are expensive to copy at runtime. ================ Comment at: clang/lib/Analysis/FlowSensitive/RecordOps.cpp:56-57 + + llvm::iterator_range< + llvm::DenseMap<const ValueDecl *, Value *>::const_iterator> + DstChildren = DstVal->children(); ---------------- xazax.hun wrote: > I know we usually like to mention types at least once, but wonder whether > ranges/iterators are actually exceptions. I've looked at the style guide, and it seems to support this usage of `auto`, so done! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153006/new/ https://reviews.llvm.org/D153006 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
