1. OpenSourceRoboticsFoundation
  2. Simulation
  3. gazebo
  4. Pull requests

Pull requests

#1340 Merged at d00353d
Repository
Branch
cpp11_default
Repository
Branch
default

C++11 support (take 2)

Author
  1. Carlos Agüero
Reviewers
Description

This pull request enables the C++11 support. There is an issue with GTEST when using EXPECT, ASSERT, QVERIFY, and we pass a shared pointer as an argument. The boost::shared_ptr bool operator is explicit and the compiler does not make an implicit conversion for us when we're using these GTEST macros.

I would say that the cleanest solution would be to use the bool operator in the form of a static_cast<bool>(mysharedPtr) or bool(mysharedPtr). However we have lots of lines where we are comparing the shared pointer with NULL. So, for consistency this is the way I chose here to fix the problem.

There are other ways as redeclaring ASSERT_TRUE with a different name and wrap it with a new ASSERT_TRUE but I don't think it's worth it.

For reference, there was some discussion in pull request #1297.

Comments (25)

    1. Steven Peters

      Trusty build has bunch of test failures:

      159: [ RUN      ] PhysicsEngines/FactoryTest.Box/0
      159: INTEGRATION_factory: /usr/include/boost/smart_ptr/shared_ptr.hpp:653: typename boost::detail::sp_member_access<T>::type boost::shared_ptr<T>::operator->() const [with T = gazebo::transport::Publisher; typename boost::detail::sp_member_access<T>::type = gazebo::transport::Publisher*]: Assertion `px != 0' failed.
      159: [Msg] Waiting for master.
      159: [Msg] Connected to gazebo master @ http://127.0.0.1:11345
      159: [Msg] Publicized address: 172.23.3.214
      159: [Dbg] [ServerFixture.cc:144] ServerFixture load in 1 seconds, timeout after 600 seconds
      159: 
       11/106 Test #159: INTEGRATION_factory ...................................***Exception: Other 51.37 sec
      
      1. Carlos Agüero author

        I think I've fixed this test in f7fe3a9.

        Node::ProcessPublishers() is using an iterator to loop through all the publishers for calling SendMessage(). The loop was protected with Node::publisherDeleteMutex but not with Node::publisherMutex. If other thread adds a new publisher to this->publishers (for example executing Node::Advertise()) can cause a new internal allocation in the std::vector Node::publishers and invalidate the iterator in the for loop.

  1. Steven Peters

    mac build failed

    /Users/jenkins/workspace/gazebo-any-devel-homebrew-amd64/gazebo/tools/gz_TEST.cc:305:5: error: no matching conversion for functional-style cast from 'std::ifstream' (aka 'basic_ifstream<char>') to '::testing::AssertionResult'
        EXPECT_TRUE(ifs);
        ^~~~~~~~~~~~~~~~
    …
    /Users/jenkins/workspace/gazebo-any-devel-homebrew-amd64/gazebo/tools/gz_TEST.cc:330:5: error: no matching conversion for functional-style cast from 'std::ifstream' (aka 'basic_ifstream<char>') to '::testing::AssertionResult'
        EXPECT_TRUE(ifs);
        ^~~~~~~~~~~~~~~~
    
    1. Steven Peters

      It's looking pretty good. I'm just seeing one of the world clone tests failing consistently (jenkins build 378 and jenkins build 379)

      245: [ RUN      ] WorldClone.Clone
      245: [Msg] Waiting for master.
      245: [Msg] Connected to gazebo master @ http://127.0.0.1:11345
      245: [Msg] Publicized address: 172.23.3.214
      245: [Dbg] [ServerFixture.cc:144] ServerFixture load in 0.8 seconds, timeout after 600 seconds
      245: /var/lib/jenkins/workspace/gazebo-any-devel-trusty-amd64-gpu-nvidia/gazebo/test/integration/world_clone.cc:190: Failure
      245: Value of: std::string::npos
      245:   Actual: 18446744073709551615
      245: Expected: output.find("/gazebo/default/camera/")
      245: Which is: 43
      245: [Dbg] [ServerFixture.cc:95] ServerFixture::Unload
      245: [  FAILED  ] WorldClone.Clone (1651 ms)
      
  2. Steven Peters

    Something to note, just as we won't be supporting gazebo5 on precise due to an old default gcc, we will also have problems with Mac OSX 10.8 since it uses libstdc++ by default, while c++11 requires libc++. We should support OSX 10.9+ for gazebo5.

        1. Jose Luis Rivero

          We could replace all boost::shared_ptr by std::shared_ptr and that could work, yes.

          But, in that case we will be probably ignoring the concept of ownership, present in std::unique_ptr and std::shared_ptr. I can bet 5 eurocents that we have a good bunch of boost::shared_ptr that could be converted into unique_ptr.