Added virtual dtor for PointCloud

[erdustiggen]

As requested in #3470, a virtual destructor was readded to the PointCloud class.

[SergioRAgostinho]

Thank you, however pcl::RangeImage is the one that needs a virtual destructor, not pcl::PointCloud.

[taketwo]

To resolve this warning indeed only pcl::RangeImage needs a virtual destructor.

However, as we have now found out, pcl::PointCloud serves as a base class. It's a good practice to mark destructors of every base class as virutal (ref).

[SergioRAgostinho]

Since PointCloud has no virtual methods, I'm struggling to find good practice cases where storing a base pointer to PointCloud objects, while wanting to address child methods from RangeImage, would make sense. That being said, if the criteria for adding a virtual dtor is a class having children, then I messed up my review during the destructor removal. I only focused on checking if the class had virtual methods and if any base class to it already defined a virtual dtor.

[taketwo]

I agree with your first point. Perhaps we should even evaluate a possibility of "un-deriving" RangeImage from PointCloud.

[SergioRAgostinho]

I just had a look at the public interface of RangeImage and it looks messy. I wish we could write it from scratch.

• The base container as a Eigen::Matrix/Eigen::Array
• Most public methods actually feel like they should be free functions or static factory methods.
• Not derived from PointCloud.

[taketwo]

So maybe we pretend that RangeImage is not derived from PointCloud -> nothing is derived from PointCloud -> PointCloud is not supposed to be a base class -> PointCloud does not need a virtual destructor. And RangeImage gets a virtual destructor since we pretend it's at the root of a class hierarchy?

[SergioRAgostinho]

Sounds good to me. I'll just switch my review to make sure no one merges this as is.

[kunaltyagi]

PointCloud is not supposed to be a base class

Maybe add final specifier to PointCloud in that case?