dblaikie added inline comments.
================
Comment at: llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:75-76
RefCountedBase() = default;
+ // Copy and move constructors/assignments are no-ops as the RefCount isn't
+ // dictated by the class directly.
RefCountedBase(const RefCountedBase &) {}
----------------
I can't quite understand this comment - perhaps you could try rephrasing it?
================
Comment at: llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:79-80
+ RefCountedBase(RefCountedBase &&) {}
+ RefCountedBase &operator=(const RefCountedBase &) { return *this; }
+ RefCountedBase &operator=(RefCountedBase &&) { return *this; }
+
----------------
What does it mean to copy a ref counted object with a builtin ref count?
I would /guess/ maybe copies (or moves) of ref counted objects like this should
only be valid when the ref count is 1 or maybe only 0? (since all the other
users who might have an active reference certainly aren't getting a reference
to the newly copied object implicitly)
Do you know which users which relied on any of the copy behavior? Maybe leave
it as = default for now?
I guess it makes sense that the copy would have a zero ref count - same as if
it were newly default constructed. But if no one's been using this
effectively/at all already - maybe avoid creating this new behavior/changing
the semantics here? What if we only allowed copies of objects with a zero ref
count - is that sufficient for existing uses?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92480/new/
https://reviews.llvm.org/D92480
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
