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.

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

What is the purpose of unique_impl_ptr(unique_impl_ptr< U, D > &&u)? #13

Open
meator opened this issue Jan 23, 2025 · 1 comment
Open

Comments

@meator
Copy link
Contributor

meator commented Jan 23, 2025

I don't understand the purpose of unique_impl_ptr(unique_impl_ptr< U, D > &&u). spimpl seems to care about construction/assignment from not only types identical to T, but also types convertible to it.

What is the use case for this feature? The PIMPLs I know don't use inheritance often.

Also, I have noticed this function in particular because it contains a bug. It tries to access the ptr_ protected member of the u argument. If it's called like this:

spimpl::unique_impl_ptr<BaseType>(spimpl::unique_impl_ptr<DerivedType>());

spimpl::unique_impl_ptr<BaseType> and spimpl::unique_impl_ptr<DerivedType> are not related classes, so they cannot access private nor protected members of each other.

EDIT: spimpl::impl_ptr::impl_ptr(spimpl::impl_ptr::impl_ptr<U, D, C> &&) suffers from the same problem.

EDIT 2: spimpl::impl_ptr::operator=(const spimpl::impl_ptr::impl_ptr<U, D, C> &) seems to lead to different and more subtle errors.

I would have made a PR for this (like I've done before), but I am not sure what would be the best way to fix this.

@meator
Copy link
Contributor Author

meator commented Jan 24, 2025

I see that std::unique_ptr has a similar move constructor from a convertible instance of the pointer, so this could have some use.

A solution to this problem could be marking all instantiations of spimpl::unique_impl_ptr friends of each other (like described here). The same could be done for spimpl::impl_ptr.

A impl ptr implementation should only inherit functions of std::unique_ptr which are relevant to PIMPLs. Is having such move constructor relevant for PIMPLs? I'm open to discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant