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

Collision between YARP and Casadi #2067

Closed
bemilio opened this issue Jul 31, 2019 · 11 comments · Fixed by #2548
Closed

Collision between YARP and Casadi #2067

bemilio opened this issue Jul 31, 2019 · 11 comments · Fixed by #2548
Labels
Affects: YARP v3.2.0 This is a known issue affecting YARP v3.2.0 Affects: YARP v3.3.0 This is a known issue affecting YARP v3.3.0 Affects: YARP v3.4.0 This is a known issue affecting YARP v3.4.0 Component: Library - YARP_os Fixed in: YARP v3.5.0 Issue Type: API Bug This is a bug in the API Resolution: Fixed

Comments

@bemilio
Copy link

bemilio commented Jul 31, 2019

Describe the bug
When including both YARP and Casadi there is a redefinition of the operator
std::ostream& std::operator<<(std::ostream& os, const std::vector<T>& t)

here is the error that I get on compilation:

/home/emilio/Repositories/robotology-superbuild/build/install/include/yarp/os/LogStream.h:195:22: error: redefinition of ‘template<class T> std::ostream& std::operator<<(std::ostream&, const std::vector<T>&)’
 inline std::ostream& std::operator<<(std::ostream& os, const std::vector<T>& t)
                      ^~~
In file included from /usr/local/include/casadi/core/matrix.hpp:31:0,
                 from /usr/local/include/casadi/core/sx_elem.hpp:33,
                 from /usr/local/include/casadi/core/core.hpp:30,
                 from /usr/local/include/casadi/casadi.hpp:29,
                 from /home/emilio/Repositories/pinocchio-integration/main.cpp:5:
/usr/local/include/casadi/core/casadi_misc.hpp:323:12: note: ‘template<class T> std::ostream& std::operator<<(std::ostream&, const std::vector<T>&)’ previously declared here
   ostream& operator<<(ostream& stream, const vector<T>& v) {

To Reproduce
Compile the code:

#include <yarp/os/all.h>
#include "casadi/casadi.hpp"
int main(){}

Expected behavior
Correct compilation with both YARP and Casadi libraries available

Screenshots
NA

Configuration (please complete the following information):

  • OS: Ubuntu 18.04
  • yarp version: 3.2.0+11-20190716.9+gitd484fdcba
  • compiler: cmake + g++

Additional context
NA

@traversaro
Copy link
Member

I suspect this is a bug in both Casadi and YARP, as extending the std namespace without using program-defined type is undefined behavior in C++ (see https://en.cppreference.com/w/cpp/language/extending_std). I think the correct way of doing this is to define the operator<<(std::ostream& os, const std::vector& t) in a non-std namespace, and use it in all the compilation unit in which you want to use it via a using directive.

@drdanz drdanz added Issue Type: API Bug This is a bug in the API Affects: YARP v3.2.0 This is a known issue affecting YARP v3.2.0 Component: Library - YARP_os Severity: Major labels Aug 1, 2019
@drdanz
Copy link
Member

drdanz commented Aug 1, 2019

To be honest, I don't remember why this was here, it shouldn't be here at all... I suspect this is something I've been using for debugging when there was no LogStream, and the robotinterface was still in iCub.
I vote for removing it, and consider that this should have never been there, I don't think anyone should be using it...

Also this bug should also be reported in casadi...

@traversaro
Copy link
Member

Also this bug should also be reported in casadi...

Yes, we wanted this issue on YARP to get some nice comments so we could link it from the casadi issue. :D

@traversaro
Copy link
Member

As mentioned in #2067 (comment), the long term solution is just to remove that function from both YARP and Casadi . On the YARP side, I think that if someone could propose a PR to get rid of that function in master it would be great, so we would have solved this issue for the next YARP feature release @Giulero @GiulioRomualdi .

@GiulioRomualdi
Copy link
Member

Unfortunately removing those lines causes this compilation error

/home/gromualdi/robot-code/robotology-superbuild/robotology/YARP/src/libYARP_robotinterface/src/yarp/robotinterface/experimental/Action.cpp: In function ‘std::ostream& std::operator<<(std::ostream&, const yarp::robotinterface::experimental::Action&)’:
/home/gromualdi/robot-code/robotology-superbuild/robotology/YARP/src/libYARP_robotinterface/src/yarp/robotinterface/experimental/Action.cpp:39:13: error: no match for ‘operator<<’ (operand types are ‘std::ostream {aka std::basic_ostream<char>}’ and ‘const ParamList {aka const std::vector<yarp::robotinterface::experimental::Param>}’)
         oss << t.params();
         ~~~~^~~~~~~~~~~~~

@traversaro
Copy link
Member

Probably the function can be reintroduced just in that compilation unit for the ParamList type?

@GiulioRomualdi
Copy link
Member

another way is to change the namespace. By the way, removing the function breaks the api so probably it cannot be done in yarp 3

@traversaro
Copy link
Member

another way is to change the namespace. By the way, removing the function breaks the api so probably it cannot be done in yarp 3

I don't think that YARP follows semantic versioning if I am not wrong.

@drdanz
Copy link
Member

drdanz commented Dec 2, 2020

I don't think that YARP follows semantic versioning if I am not wrong.

Actually we are trying to follow semantic versioning, with some exceptions for dev

@traversaro
Copy link
Member

traversaro commented Dec 2, 2020

I don't think that YARP follows semantic versioning if I am not wrong.

Actually we are trying to follow semantic versioning, with some exceptions for dev

Ah ok, good to know. I wrote that because I remember that back when YARP 3.4 there were breaking changes w.r.t. 3.3, but YARP was not released as YARP 4. Just to clarify, does this means that if deprecated methods are removed in libYARP_os or libYARP_math, the following release would be YARP 4 ? I guess this applies just to API, while not for ABI?

@drdanz
Copy link
Member

drdanz commented Dec 2, 2020

Yes, but you are right, we broke something in latest release... I think we are not completely strict on the rule yet 😉
Let's say that whenever it is possible, we deprecate the things to be removed and remove them in the following major version.

@drdanz drdanz added Affects: YARP v3.3.0 This is a known issue affecting YARP v3.3.0 Affects: YARP v3.4.0 This is a known issue affecting YARP v3.4.0 labels Dec 9, 2020
drdanz added a commit to drdanz/yarp that referenced this issue Apr 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Affects: YARP v3.2.0 This is a known issue affecting YARP v3.2.0 Affects: YARP v3.3.0 This is a known issue affecting YARP v3.3.0 Affects: YARP v3.4.0 This is a known issue affecting YARP v3.4.0 Component: Library - YARP_os Fixed in: YARP v3.5.0 Issue Type: API Bug This is a bug in the API Resolution: Fixed
Projects
None yet
4 participants