[PATCH] D153006: [clang][dataflow] Perform deep copies in copy and move operations.

Mon Jun 19 02:08:12 PDT 2023

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