Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Invalid constructor SFINAE for std::shared_ptr's array ctors #60258

Closed
ldionne opened this issue Jan 24, 2023 · 4 comments
Closed

Invalid constructor SFINAE for std::shared_ptr's array ctors #60258

ldionne opened this issue Jan 24, 2023 · 4 comments
Assignees
@huixie90
Labels
accepts-invalid libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@ldionne
Copy link
Member

ldionne commented Jan 24, 2023 •
edited by VoltrexKeyva

This was reported to me by Marshall Clow (thanks!)

The SFINAE criteria for shared_ptr's constructors don't seem right. Consider the following code:

#include <tuple>
#include <cassert>
#include <memory>

int main () {
	std::shared_ptr<int> i;
	std::shared_ptr<int[5]> i5a = i;
	std::shared_ptr<int[]> iUa = i;
	std::shared_ptr<int[]> iUb = i5a;
	std::shared_ptr<int[5]> i5b = iUa;
	std::shared_ptr<int[7]> i7 = i5a;
}

Clang + libc++ accepts it (C++17).
GCC + libstdc++ rejects it (C++17)
MSVC rejects it (C++17)

<source>(7): error C2440: 'initializing': cannot convert from 'std::shared_ptr<int>' to 'std::shared_ptr<int [5]>'
<source>(8): error C2440: 'initializing': cannot convert from 'std::shared_ptr<int>' to 'std::shared_ptr<int []>'
<source>(10): error C2440: 'initializing': cannot convert from 'std::shared_ptr<int []>' to 'std::shared_ptr<int [5]>'
<source>(11): error C2440: 'initializing': cannot convert from 'std::shared_ptr<int [5]>' to 'std::shared_ptr<int [7]>'
@ldionne ldionne added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jan 24, 2023
@ldionne
Copy link
Member Author

ldionne commented Jan 24, 2023

@huixie90 Could you take a look? I think you just tinkered with std::shared_ptr's make_shared functions, so perhaps this is fresh in your memory?

@huixie90
Copy link
Contributor

@huixie90 Could you take a look? I think you just tinkered with std::shared_ptr's make_shared functions, so perhaps this is fresh in your memory?

sure

@huixie90
Copy link
Contributor

huixie90 commented Jan 24, 2023 •
edited

ATM, we unconditionally remove_extent_t whiling checking the constraints

template<class _Tp, class _Up>
struct __compatible_with
#if _LIBCPP_STD_VER > 14
    : is_convertible<remove_extent_t<_Tp>*, remove_extent_t<_Up>*> {};
#else
    : is_convertible<_Tp*, _Up*> {};
#endif // _LIBCPP_STD_VER > 14

https://github.com/llvm/llvm-project/blob/main/libcxx/include/__memory/shared_ptr.h#L373

Instead, we need to check on the original type

@huixie90
Copy link
Contributor

This should be fixed by
a38a465

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@huixie90 huixie90

Labels
accepts-invalid libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@ldionne @philnik777 @huixie90