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

Provide dynamic and static pointer casts in pcl namespace to allow easy migration from boost to std smart pointers #3770

Merged
merged 8 commits into from
Apr 16, 2020

Conversation

aPonza
Copy link
Contributor

@aPonza aPonza commented Mar 21, 2020

Stems from #3750.

@kunaltyagi
Copy link
Member

Based on the CI, I'd say that we need pcl::*_cast.

error: 'dynamic_pointer_cast' was not declared in this scope

@Morwenn
Copy link
Contributor

Morwenn commented Mar 23, 2020

If I'm not mistaken ADL does not work here because smart pointer casts are function templates taking an explicit template parameter - this was fixed in C++20 but you have to workaround it in earlier standard versions.

You can work around it by first making one of the names visible to the compiler in scope, then ADL should work properly:

boost::shared<U> ptr;
using std::dynamic_pointer_cast; // The compiler sees a function template and considers that unqualified `dynamic_pointer_cast` has to be resolved as a function name/overload set
auto ptr2 = dynamic_pointer_cast<T>(ptr); // ADL is performed, boost::dynamic_pointer_cast is found despite std:: being used to make the name visible

Sprinkling using std::*_cast; should be enough without having to introduce PCL-specific functions. Now introducing functions also avoids having to rely on this specific form of ADL and lets you write one line instead of two.

apps/src/feature_matching.cpp Outdated Show resolved Hide resolved
common/include/pcl/memory.h Show resolved Hide resolved
Copy link
Member

@kunaltyagi kunaltyagi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kunaltyagi kunaltyagi added the changelog: new feature Meta-information for changelog generation label Mar 23, 2020
@kunaltyagi kunaltyagi changed the title Unqualify dynamic and static pointer casts for ease of switch to std::shared_ptr Provide dynamic and static pointer casts in namespace pcl to allow easy migration in future Mar 23, 2020
@kunaltyagi kunaltyagi added module: common needs: code review Specify why not closed/merged yet labels Mar 23, 2020
@kunaltyagi
Copy link
Member

Just waiting for another set of eyes before merge.

@SergioRAgostinho SergioRAgostinho merged commit 0f62b5d into PointCloudLibrary:master Apr 16, 2020
@aPonza aPonza deleted the pointer_casts branch April 16, 2020 09:04
@kunaltyagi kunaltyagi changed the title Provide dynamic and static pointer casts in namespace pcl to allow easy migration in future Provide dynamic and static pointer casts in namespace pcl to allow easy migration May 9, 2020
@taketwo taketwo changed the title Provide dynamic and static pointer casts in namespace pcl to allow easy migration Provide dynamic and static pointer casts in pcl namespace to allow easy migration May 10, 2020
@taketwo taketwo changed the title Provide dynamic and static pointer casts in pcl namespace to allow easy migration Provide dynamic and static pointer casts in pcl namespace to allow easy migration from boost to std smart pointers May 10, 2020
truhoang pushed a commit to truhoang/pcl that referenced this pull request Jun 30, 2020
…sy migration in future (PointCloudLibrary#3770)

* unqualify boost::dynamic_pointer_cast for switch to std::
* unqualify boost::static_pointer_cast for switch to std::
* have consistent using declarations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: new feature Meta-information for changelog generation module: common needs: code review Specify why not closed/merged yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants