orocos-toolchain / orogen

Code generator for components and type handling in Rock - the Robot Construction Kit - and the Orocos Toolchain
http://rock-robotics.org
Other
4 stars 35 forks source link

opaque type equality should be checked by typename instead of type member equality #73

Closed saarnold closed 8 years ago

saarnold commented 8 years ago

Currently opaque type equality is checked using the == operator which calls do_compare in typelib. And I guess that it is intentional that it checks for equal member types and offsets and doesn't check the name of the type.

But in the case two opaques are defined having the same members. E.g.:

struct ClassAWrapper
{
    std::vector<uint8_t> bin;
};

struct ClassBWrapper
{
    std::vector<uint8_t> bin;
};

they currently both resolve in the same opaque type ClassA. So I think comparing the type name instead of the type would be the right thing to do here.

doudou commented 8 years ago

And I guess that it is intentional that it checks for equal member types and offsets and doesn't check the name of the type.

Mmmm... maybe not that intentional. However, it would be too ingrained to change the behaviour of #== right now.

The preferred solution for me would be to override Type.eql? in typelib (which in ruby is a stronger #==) so that eql? checks for both name and == (e.g. def eql?(type); self == type && type.name == name end), and use #eql? instead of #== here. Explicitely checking on name feels really too horrible.

saarnold commented 8 years ago

I did change it to use eql? and added a corresponding PR to typelib orocos-toolchain/typelib#75

saarnold commented 8 years ago

ping

doudou commented 8 years ago

Merged. Sorry for the delay