Add TfDelegatedCountPtr to support custom bookkeeping
#2891
Conversation
nvmkuruc
force-pushed
the
tfintrusiveptr
branch
3 times, most recently
from
b227f2c
to
49d6d18
Compare
|
Filed as internal issue #USD-9122 |
nvmkuruc
force-pushed
the
tfintrusiveptr
branch
from
49d6d18
to
730d079
Compare
nvmkuruc
force-pushed
the
tfintrusiveptr
branch
2 times, most recently
from
755d12d
to
8552493
Compare
nvmkuruc
changed the title
Tf_TrackingPtr to support custom bookkeepingTfDelegatedCountPtr to support custom bookkeeping
nvmkuruc
force-pushed
the
tfintrusiveptr
branch
2 times, most recently
from
fbe72b6
to
5715c18
Compare
pxr/base/tf/delegatedCountPtr.h
Outdated
| } | ||
|
|
||
| /// Dereference the underlying pointer | ||
| ReferenceType operator*() const { |
pxr/base/tf/delegatedCountPtr.h
Outdated
| /// construction, assignment, and destruction operations. | ||
| /// | ||
| /// `TfDelegatedCountIncrement` and `TfDelegatedCountDecrement` are expected to | ||
| /// be `noexcept`. The increment and decrement methods can assume pointers are |
There was a problem hiding this comment.
Requiring these to be noexcept seems potentially too restrictive to me... What's the motivation for this requirement? Could we just transparently propagate the noexcept-ness of these so that it's a "pay for what you use" type of thing?
nvmkuruc
force-pushed
the
tfintrusiveptr
branch
4 times, most recently
from
5d8db81
to
147c677
Compare
nvmkuruc
force-pushed
the
tfintrusiveptr
branch
from
147c677
to
1efa3c7
Compare
pxr/base/tf/delegatedCountPtr.h
Outdated
| /// construction, assignment, and destruction operations. | ||
| /// | ||
| /// As it may be called in a destructor, `TfDelegatedCountDecrement` is expected | ||
| /// to be `noexcept`. The increment and decrement methods can assume pointers |
There was a problem hiding this comment.
We can remove this comment now, no?
pxr/base/tf/delegatedCountPtr.h
Outdated
|
|
||
| using IncrementIsNoExcept = | ||
| std::integral_constant< | ||
| bool, noexcept(TfDelegatedCountIncrement(RawPtrType{}))>; |
There was a problem hiding this comment.
could you use std::declval<RawPtrType>() for the value here?
pxr/base/tf/delegatedCountPtr.h
Outdated
| } | ||
|
|
||
| /// Dereference the underlying pointer | ||
| ReferenceType operator*() const { |
There was a problem hiding this comment.
this can be noexcept I think.
| /// | ||
| /// `ptr` will be reset to its default state (ie. `nullptr`). | ||
| TfDelegatedCountPtr& operator=(TfDelegatedCountPtr&& ptr) | ||
| noexcept(DecrementIsNoExcept()) { |
There was a problem hiding this comment.
ok if I had a foo that was a TfDelegatedCountPtr<Foo> and I did:
foo = std::move(foo);
That would blow up, right?
There was a problem hiding this comment.
There's a test case for this specifically. It doesn't blow up. foo is left in a valid state. See TestMovingSelf().
pxr/base/tf/delegatedCountPtr.h
Outdated
|
|
||
| private: | ||
| void _IncrementIfValid() noexcept(IncrementIsNoExcept()) { | ||
| if (get()) { |
There was a problem hiding this comment.
any reason not to just do if (_pointer) here? we use it directly in the body immediately after -- it seems clearer to me that way.
There was a problem hiding this comment.
A bunch of prior usages of _pointer had to be converted to get() to support operations on convertible types without friend-ship. But there's no reason this usage had to be converted.
nvmkuruc
force-pushed
the
tfintrusiveptr
branch
3 times, most recently
from
9bcaf2c
to
e64b2a5
Compare
nvmkuruc
force-pushed
the
tfintrusiveptr
branch
from
e64b2a5
to
56678ce
Compare
Description of Change(s)
OpenUSD currently uses
boost::intrusive_ptrto support customized bookkeeping of types. Specialization is achieved through ADL (intrusive_ptr_add_refandintrusive_ptr_release).This introduces
TfDelegatedCountPtras an alternative to support customized bookkeeping via ADL overloadsTfDelegatedCountIncrementandTfDelegatedCountDecrement.The
TfDelegatedCountPtravoids implicit conversion of raw pointers and requires explicit tag dispatch to specify whether the pointer should be retained or not.This also provides a
TfMakeDelegatedCountPtrcreator function to avoid direct allocation.Fixes Issue(s)