Implement c++20 shared_ptr rvalue overloads for aliasing constructor and pointer casts. See https://cplusplus.github.io/LWG/issue2996
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Thanks for working on this. In general looks quite good, but some code uses an older style we don't use for new code.
libcxx/docs/Status/Cxx20Issues.csv | ||
---|---|---|
105 | ||
libcxx/include/__memory/shared_ptr.h | ||
632 | Please update the synopsis too. | |
632 | We (and other vendors) typically backport LWG-issue, can you do that for this issue too. | |
634 | ||
722 | Please undo this change. In the past we used _VSTD instead of std. We don't change all existing code to avoid breaking too much work in progress, but we use it when changing code. | |
1521 | _LIBCPP_HIDE_FROM_ABI is the new name for _LIBCPP_INLINE_VISIBILITY. | |
1522 | ||
1540 | See above. | |
1541 | In new code we prefer using over typedef. | |
1577 | The extra space isn't fixed by clang-format due to C++03 compatibility. | |
libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.cast/const_pointer_cast.pass.cpp | ||
14 | Please test whether this is really noexcept. There is a ASSERT_NOEXCEPT helper macro. |
libcxx/include/__memory/shared_ptr.h | ||
---|---|---|
632 | I think back-porting to c++17 will break code that does a std::move() but relies on the fact that no move happens. Arguable this a just a bug in user code, but probably still something we don't want to change? |
libcxx/include/__memory/shared_ptr.h | ||
---|---|---|
632 | I don't feel that we should guard against users doing bad things; updating their code to C++20 will break that scenario. I see MSVC STL applied this change unconditionally, but libstdc++ limits it to C++20 and newer. @jwakely is there a reason for not backporting LWG-2996 to older versions? | |
632 | Or remove it entirely. | |
libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.cast/const_pointer_cast.pass.cpp | ||
69 | Or removed, the same for other tests. |
libcxx/include/__memory/shared_ptr.h | ||
---|---|---|
632 | I don't remember why I only added it for C++20, but probably because it's questionable whether there was a defect. It seems more like an evolutionary change (which is why it went through LEWG) than a defect fix. |
libcxx/include/__memory/shared_ptr.h | ||
---|---|---|
632 |
Thanks for the information. Then it makes sense to me to do the same in libc++. @pateldeev please leave a comment why this LWG issue isn't backported to earlier language versions. |
Please mention the LWG issue you are implementing in the title. Other than that and inline comments this LGTM. Leaving final approval to @Mordante, since he had comments.
libcxx/include/__memory/shared_ptr.h | ||
---|---|---|
634 | The LWG issue is against C++20. Same for the other changes. | |
libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.cast/const_pointer_cast.pass.cpp | ||
69 | ||
libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/shared_ptr_pointer.pass.cpp | ||
106–115 | I'd just remove this test. I don't really see the point of testing this, since it's questionable to rely on this behaviour and it's been changed with an LWG issue which some implementations back-port. |
Thanks for the change, LGTM modulo comments and green CI (please rebase first). Thanks!
libcxx/docs/Status/Cxx20Issues.csv | ||
---|---|---|
105 |
Rebased diff and addressed all comments. Should be ready to merge. But it seems I don't have the ability to do this.
@ldionne: Can you help merge this?