search

ros / roscpp_core

ros distribution sandbox
88 stars 116 forks source link

fix #86. #87

Closed csukuangfj closed 5 years ago

csukuangfj commented 6 years ago

Fix #86.

csukuangfj commented 6 years ago

@dirk-thomas Could you please have a look at the test failure? http://build.ros.org/job/Kpr__roscpp_core__ubuntu_xenial_amd64/46/testReport/

dirk-thomas commented 5 years ago

I think changing the destructor to be virtual would break ABI compatibility. Therefore this can only be considered for future distributions but can't be released into existing distributions.

cwecht commented 5 years ago

Changing the destructor to be virtual will introduce a vtable and a vptr to the class. On a 64-bit-machine the objects size will be doubled.

ros::DurationBase is an example for the Curiously recurring template pattern, which is a technique to implement static polymorphism.

I'd rather argue that ros::DurationBases desctuctor should be removed entirely , which would make it trivially copyable (see also #82). But this will also result in an ABI-break.

dirk-thomas commented 5 years ago

Closing in favor of #104.