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

Homogenize deprecations with PCL_DEPRECATED #3925

Merged
merged 3 commits into from
May 2, 2020

Conversation

aPonza
Copy link
Contributor

@aPonza aPonza commented Apr 15, 2020

Stems from #3905. I git grep -n deprec and tried my best at recognizing the results needing homogeneity.

The missing instances are tracked in #3924

Fixes #2724

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 (except inline)

outofcore/include/pcl/outofcore/octree_base_node.h Outdated Show resolved Hide resolved
@kunaltyagi kunaltyagi added changelog: deprecation Meta-information for changelog generation module: outofcore needs: more work Specify why not closed/merged yet labels Apr 16, 2020
@aPonza
Copy link
Contributor Author

aPonza commented Apr 16, 2020

Nice! The tutorials build caught a deprecation warning in the examples:

/__w/1/s/doc/tutorials/content/sources/registration_api/example2.cpp: In function 'void rejectBadCorrespondences(const CorrespondencesPtr&, const Ptr&, const Ptr&, pcl::Correspondences&)':
/__w/1/s/doc/tutorials/content/sources/registration_api/example2.cpp:123:45: error: 'void pcl::registration::CorrespondenceRejectorDistance::setInputCloud(const typename pcl::PointCloud<PointT>::ConstPtr&) [with PointT = pcl::PointXYZ; typename pcl::PointCloud<PointT>::ConstPtr = boost::shared_ptr<const pcl::PointCloud<pcl::PointXYZ> >]' is deprecated: pcl::registration::CorrespondenceRejectorDistance::setInputCloud is deprecated. Please use setInputSource instead (It will be removed in PCL 1.12) [-Werror=deprecated-declarations]
   rej.setInputCloud<PointXYZ> (keypoints_src);
                                             ^
In file included from /__w/1/s/doc/tutorials/content/sources/registration_api/example2.cpp:12:0:
/__w/1/install/include/pcl-1.10/pcl/registration/correspondence_rejection_distance.h:109:9: note: declared here
         setInputCloud (const typename pcl::PointCloud<PointT>::ConstPtr &cloud)
         ^
cc1plus: all warnings being treated as errors

No clue about what windows wants from me though:

     Creating library D:/a/build/lib/pcl_surface.lib and object D:/a/build/lib/pcl_surface.exp
  Generating code
  LINK : the 32-bit linker (C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\VC\Tools\MSVC\14.16.27023\bin\HostX86\x64\link.exe) ran out of heap space and is going to restart linking with a 64-bit linker
  LINK : restarting link with 64-bit linker `C:\Program Files\Git\usr\bin\link.exe'
  /c/Program Files (x86)/Microsoft Visual Studio/2017/Enterprise/VC/Tools/MSVC/14.16.27023/bin/HostX86/x64/link: cannot create link ' ■/' to '/ERRORREPORT:QUEUE': No such file or directory
C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(720,5): error MSB6006: "link.exe" exited with code 1. [D:\a\build\surface\pcl_surface.vcxproj]
  Building Custom Rule D:/a/1/s/tracking/CMakeLists.txt
  tracking.cpp
  particle_filter.cpp
  kld_adaptive_particle_filter.cpp
  coherence.cpp

The linker is complaining (link), I'm not touching [surface] directly... Is it some inspectable problem? SO gives answers that are all over the place. this was about link.exe, not cmd.exe
EDIT this might be the link I was after, and it's solved in VS2019. Let's first see if it happens again.

@kunaltyagi
Copy link
Member

ran out of heap space

Windows OOMed. Restarting can sometimes solve the issue.

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.

CI is green. 🚀

@SergioRAgostinho SergioRAgostinho added needs: code review Specify why not closed/merged yet and removed needs: more work Specify why not closed/merged yet labels Apr 17, 2020
Copy link
Member

@SergioRAgostinho SergioRAgostinho left a comment

Choose a reason for hiding this comment

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

LGTM but we need to figure out what to do with the deprecations with empty messages.

outofcore/include/pcl/outofcore/octree_base.h Outdated Show resolved Hide resolved
outofcore/include/pcl/outofcore/octree_base.h Outdated Show resolved Hide resolved
outofcore/include/pcl/outofcore/octree_base_node.h Outdated Show resolved Hide resolved
visualization/src/pcl_visualizer.cpp Outdated Show resolved Hide resolved
outofcore/include/pcl/outofcore/octree_base.h Outdated Show resolved Hide resolved
outofcore/include/pcl/outofcore/octree_base.h Outdated Show resolved Hide resolved
outofcore/include/pcl/outofcore/octree_base.h Outdated Show resolved Hide resolved
outofcore/include/pcl/outofcore/octree_base_node.h Outdated Show resolved Hide resolved
Copy link
Member

@SergioRAgostinho SergioRAgostinho left a comment

Choose a reason for hiding this comment

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

Minor indentation issue in my patch. Leave it for clang-format.

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. The based on the online comment, the labels need to be updated

outofcore/include/pcl/outofcore/octree_base.h Show resolved Hide resolved
@kunaltyagi kunaltyagi added needs: feedback Specify why not closed/merged yet and removed needs: code review Specify why not closed/merged yet labels Apr 19, 2020
@aPonza
Copy link
Contributor Author

aPonza commented Apr 19, 2020

Commit wise I'm thinking:

  • homogenize;
  • fix setInputSource;
  • fix deallocemptynodecache.

kunaltyagi
kunaltyagi previously approved these changes Apr 19, 2020
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.

Rebase + CI = 🚀

@kunaltyagi kunaltyagi added needs: code review Specify why not closed/merged yet and removed needs: feedback Specify why not closed/merged yet labels Apr 19, 2020
@SergioRAgostinho SergioRAgostinho added changelog: ABI break Meta-information for changelog generation and removed changelog: ABI break Meta-information for changelog generation labels Apr 20, 2020
@kunaltyagi kunaltyagi added needs: more work Specify why not closed/merged yet and removed needs: code review Specify why not closed/merged yet labels Apr 30, 2020
@kunaltyagi kunaltyagi dismissed their stale review April 30, 2020 05:10

more work needed

@aPonza aPonza requested a review from kunaltyagi April 30, 2020 11:09
@kunaltyagi
Copy link
Member

Squash DeAllocEmptyNodeCache related commits or squash all during merge?

@kunaltyagi kunaltyagi added needs: author reply Specify why not closed/merged yet needs: code review Specify why not closed/merged yet and removed needs: more work Specify why not closed/merged yet needs: code review Specify why not closed/merged yet labels Apr 30, 2020
@kunaltyagi kunaltyagi merged commit 3d4a096 into PointCloudLibrary:master May 2, 2020
@aPonza aPonza deleted the homogenize branch May 3, 2020 12:30
@taketwo taketwo changed the title Homogenize deprecation with PCL_DEPRECATED Homogenize deprecations with PCL_DEPRECATED May 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: deprecation Meta-information for changelog generation module: outofcore module: registration needs: author reply Specify why not closed/merged yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Removing deprecated methods changed behavior
3 participants