Implement P1132R7 out_ptr() and inout_ptr()#1998
Conversation
|
Awesome! I was wondering whether I should give it a try but you beat me to it. Will Review tomorrow |
|
|
||
| operator void**() const noexcept requires(!is_same_v<_Pointer, void*>) { | ||
| static_assert(is_pointer_v<_Pointer>, "out_ptr_t's operator void** requires Pointer to be a pointer"); | ||
| return reinterpret_cast<void**>(_STD addressof(const_cast<_Pointer&>(_Ptr))); |
There was a problem hiding this comment.
We have the precondition that we should not have called operator _Pointer* Do we care enough to actually validate this somehow? It would most likely cost some memory so I am totally fine with nasal demons emerging
There was a problem hiding this comment.
I don't think it's worth validating. It will be used as foo(out_ptr(my_ptr)) 99.9% of the time.
There was a problem hiding this comment.
I agree with Adam. It's not so much the memory as it is having the ABI of out_ptr differ between debug and release modes. We already do that with many types, but doing it here will probably cause more harm than good, given Adam's point about common usage
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
barcharcraz
left a comment
barcharcraz
left a comment
There was a problem hiding this comment.
You need to add the feature test macros, additionally I found the error messages produced by the static_asserts to be possibly confusing, but I don't care all that strongly about that.
I observe that we may be able to apply certain optimizations under the "as if" rule to this type. In particular for raw and unique ptrs. Raw pointers maybe aren't worth it, but unique_ptrs might be.
I think the optimization involves calling release in the constructor and then not storing the reference to the smart pointer itself. I'm OK with not applying such an optimization but we should keep it in mind.
|
|
||
| operator void**() const noexcept requires(!is_same_v<_Pointer, void*>) { | ||
| static_assert(is_pointer_v<_Pointer>, "out_ptr_t's operator void** requires Pointer to be a pointer"); | ||
| return reinterpret_cast<void**>(_STD addressof(const_cast<_Pointer&>(_Ptr))); |
There was a problem hiding this comment.
I agree with Adam. It's not so much the memory as it is having the ABI of out_ptr differ between debug and release modes. We already do that with many types, but doing it here will probably cause more harm than good, given Adam's point about common usage
StephanTLavavej
left a comment
StephanTLavavej
left a comment
There was a problem hiding this comment.
Thanks! My major question is whether it's worth compressing away empty tuple<>s; everything else is nitpick-level.
|
@AdamBucior @barcharcraz I've pushed a merge with |
|
Thanks for implementing this C++23 feature! 😻 🚀 🎉 |
7 participants
Resolves #1971.