search

ros / urdf

Repository for URDF parsing code
63 stars 41 forks source link

Style fixes from ros2 #10

Closed clalancette closed 6 years ago

clalancette commented 6 years ago

Basically what it says on the tin. These are a bunch of compatibility and style fixes from https://github.com/ros2/urdf . By merging them in here, we reduce the differences between the two forks and make it easier to (eventually) merge them back into one repository. No functional changes.

mikaelarguedas commented 6 years ago

(genuine question as I never thought of it before): Does the ABI stay the same when adding the visibility attribute to all the functions ?

clalancette commented 6 years ago

(genuine question as I never thought of it before): Does the ABI stay the same when adding the visibility attribute to all the functions ?

Huh, good question. I never thought about it before either, but I don't think so. The visibility attribute mostly controls whether the symbol is T or t in the final binary/shared object, not what its ABI is. But I took a look to be sure. Before this change, here are the symbols that are in model.h:

urdf::Model::initXml(TiXmlElement*)  -- T _ZN4urdf5Model7initXmlEP12TiXmlElement
urdf::Model::initXml(TiXmlDocument*) -- T _ZN4urdf5Model7initXmlEP13TiXmlDocument
urdf::Model::initFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) -- T _ZN4urdf5Model8initFileERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE
urdf::Model::initParamWithNodeHandle(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, ros::NodeHandle const&) -- T _ZN4urdf5Model23initParamWithNodeHandleERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKN3ros10NodeHandleE
urdf::Model::initParam(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) -- T _ZN4urdf5Model9initParamERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE
urdf::Model::initString(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) -- _ZN4urdf5Model10initStringERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE

After the change, this same set looks like:

urdf::Model::initXml(TiXmlElement*) -- T _ZN4urdf5Model7initXmlEP12TiXmlElement
urdf::Model::initXml(TiXmlDocument*) -- T _ZN4urdf5Model7initXmlEP13TiXmlDocument
urdf::Model::initFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) -- T _ZN4urdf5Model8initFileERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE
urdf::Model::initParamWithNodeHandle(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, ros::NodeHandle const&) -- T _ZN4urdf5Model23initParamWithNodeHandleERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKN3ros10NodeHandleE
urdf::Model::initParam(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) -- T _ZN4urdf5Model9initParamERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE
urdf::Model::initString(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)
 -- T _ZN4urdf5Model10initStringERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE

So as far as I can tell, no change to the ABI.

mikaelarguedas commented 6 years ago

good to know! thanks for checking

clalancette commented 6 years ago

Thanks, merging.