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

Transition to standard smart pointers, part 4 #2929

Merged
merged 3 commits into from Mar 25, 2019

Conversation

taketwo
Copy link
Member

@taketwo taketwo commented Mar 16, 2019

Partially addresses #2792.

@taketwo taketwo added the c++14 label Mar 16, 2019
@taketwo taketwo added this to In Progress in C++14 Migration via automation Mar 16, 2019
@taketwo taketwo changed the title Transition to standard smart pointers, part 4 [WIP] Transition to standard smart pointers, part 4 Mar 18, 2019
Modules:
 * kepoints
 * people
 * stereo
 * surface
 * tracking
 * tutorials
@taketwo taketwo changed the title [WIP] Transition to standard smart pointers, part 4 Transition to standard smart pointers, part 4 Mar 22, 2019
@taketwo
Copy link
Member Author

taketwo commented Mar 22, 2019

This one is ready for review.

@@ -141,12 +141,12 @@ template<typename ModelT, typename SceneT>
typename boost::graph_traits<Graph>::adjacency_iterator ai;
typename boost::graph_traits<Graph>::adjacency_iterator ai_end;

boost::shared_ptr < RecognitionModel > current = static_cast<boost::shared_ptr<RecognitionModel> > (graph_id_model_map_[int (v)]);
auto current = boost::dynamic_pointer_cast<RecognitionModel> (graph_id_model_map_[int (v)]);
Copy link
Member

Choose a reason for hiding this comment

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

Is it worth to incur into the dynamic cast runtime penalty here? The subsequent logic is not handling null returns.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, I will replace this with boost::static_pointer_cast.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will squash this fix in corresponding commit right away.


bool a_better_one = false;
for (boost::tie (ai, ai_end) = boost::adjacent_vertices (v, conflict_graph_); (ai != ai_end) && !a_better_one; ++ai)
{
boost::shared_ptr < RecognitionModel > neighbour = static_cast<boost::shared_ptr<RecognitionModel> > (graph_id_model_map_[int (*ai)]);
auto neighbour = boost::dynamic_pointer_cast<RecognitionModel> (graph_id_model_map_[int (*ai)]);
Copy link
Member

Choose a reason for hiding this comment

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

Same comment here.

@@ -169,7 +169,7 @@ template<typename ModelT, typename SceneT>
for (size_t i = 0; i < (recognition_models_.size ()); i++)
{
const typename Graph::vertex_descriptor v = boost::add_vertex (recognition_models_[i], conflict_graph_);
graph_id_model_map_[int (v)] = static_cast<boost::shared_ptr<RecognitionModel> > (recognition_models_[i]);
graph_id_model_map_[int (v)] = boost::dynamic_pointer_cast<RecognitionModel> (recognition_models_[i]);
Copy link
Member

Choose a reason for hiding this comment

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

Same comment here.

For classes:
 * DecisionTreeTrainerDataProvider
 * DistanceCoherence
 * FaceDetectorDataProvider
 * ISMVoteList
 * IntegralImage2D
 * KLDAdaptiveParticleFilterOMPTracker
 * KLDAdaptiveParticleFilterTracker
 * OctreePointCloudChangeDetector
 * ParticleFilterTracker
 * RecognitionModel
@SergioRAgostinho SergioRAgostinho merged commit c20130b into PointCloudLibrary:master Mar 25, 2019
C++14 Migration automation moved this from In Progress to Done Mar 25, 2019
@taketwo taketwo deleted the smart-pointers-4 branch March 26, 2019 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants