Skip to content

Commit 9a10d31

Browse files
yuxuanchen1997facebook-github-bot
authored andcommitted
add static_assert that unique_ptr<T> is constructible from not_null<unique_ptr<T>>
Summary: Adding this as a test for the new compiler. With this upstream PR: llvm/llvm-project#136203 This test starts to fail with the following error: ``` fbcode/folly/memory/test/not_null_test.cpp:465:24: error: call to constructor of 'std::unique_ptr<int>' is ambiguous 465 | std::unique_ptr<int> i{std::move(nnu)}; | ^~~~~~~~~~~~~~~~~ fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/unique_ptr.h:327:7: note: candidate constructor 327 | unique_ptr(unique_ptr&&) = default; | ^ fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/unique_ptr.h:468:7: note: candidate constructor has been explicitly deleted 468 | unique_ptr(const unique_ptr&) = delete; | ^ fbcode/folly/memory/test/not_null_test.cpp:467:7: error: static assertion failed due to requirement 'std::is_constructible_v<std::unique_ptr<int, std::default_delete<int>>, folly::not_null<std::unique_ptr<int, std::default_delete<int>>>>' 467 | std::is_constructible_v< | ^~~~~~~~~~~~~~~~~~~~~~~~ 468 | std::unique_ptr<int>, | ~~~~~~~~~~~~~~~~~~~~~ 469 | not_null<std::unique_ptr<int>>>); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 2 errors generated. ``` Note that when you construct a unique_ptr, its deleted copy constructor still participates in overload resolution. So it's possible for a operator const PtrT&() const& noexcept to be selected. Meanwhile, its operator PtrT&&() && noexcept; is selected to convert not_null to the unique_ptr&& for the move constructor. Hence the ambiguity. Why is this only a problem now? See D74599633. The existence of the conversion operator template was a better match to create a unique_ptr from a not_null previously. However, the SFINAE conditions are circular and caused weird compiler behaviours. This upstream PR now stops looking at templates when there's a "perfect match", which are the nontemplated `operator PtrT&&` and `operator const PtrT&` for not_null. It's possible that the PR has hidden some bugs, but the behaviour is unspecified by the standard anyway. The PR implements the change to make it align with GCC behaviour. Reviewed By: snarkmaster, Gownta Differential Revision: D74602687 fbshipit-source-id: ad6c49e6fa57c6340d6646734e9b6120e309353c
1 parent 6d7a7ef commit 9a10d31

File tree

1 file changed

+7
-0
lines changed

1 file changed

+7
-0
lines changed

third-party/folly/src/folly/memory/test/not_null_test.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,13 @@ TEST_F(NotNullHelperTest, output) {
461461
}
462462

463463
TEST_F(NotNullHelperTest, casting) {
464+
auto nnu = make_not_null_unique<int>(42);
465+
std::unique_ptr<int> i{std::move(nnu)};
466+
static_assert(
467+
std::is_constructible_v<
468+
std::unique_ptr<int>,
469+
not_null<std::unique_ptr<int>>>);
470+
464471
not_null_shared_ptr<Derived> nnd(new Derived());
465472

466473
auto s = static_pointer_cast<Base>(nnd);

0 commit comments

Comments
 (0)