Skip to content

Conversation

@thomasjpfan
Copy link

neigh_distances and neigh_indices can be a unique pointer.

The shared_ptr constructor allows casting from unique to shared (item 13).

CC @jjerphan

@jjerphan
Copy link
Owner

Thank you for the two suggestions, @thomasjpfan.

@jjerphan jjerphan merged commit a2efdd1 into jjerphan:pairwise-distances-radius-neighborhood Feb 27, 2022
@jjerphan
Copy link
Owner

In fact item 13 is a move constructor which must take a r-value reference (see the && for item 13) in order to be used.

We get a compile error because we aren't using r-value here, but self.neigh_{distance,indices} which are l-value.
I guess it makes sense not the be able to cast unique_ptr to shared_ptr because this would just break the uniqueness of the pointer. This was my prior understanding before this PR, and it seems that the problem we are meeting here likely illustrates it.

Is this right or am I missing something?

I will just revert for now to have the CI pass on the original PR, still I am interested in pursuing the discussions here, @thomasjpfan.

@thomasjpfan
Copy link
Author

thomasjpfan commented Feb 27, 2022

Yes you are correct.

My mental model was incorrect about the interaction between shared + unique pointers. Everything needs to be shared here. For the casting to work, we would need to move it.

@jjerphan
Copy link
Owner

jjerphan commented Feb 27, 2022

That was an opportunity for me to learn as well. :)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants