Skip to content
This repository has been archived by the owner on Feb 3, 2025. It is now read-only.

Fix namespace usage in console macros #2892

Merged

Conversation

stefanbuettner
Copy link
Contributor

@stefanbuettner stefanbuettner commented Nov 27, 2020

Hey there,

This code did not work

namespace foo::gazebo
{

class bar
{
public:
    void doSomething() {
        gzmsg << "doSomething" << std::endl;
    }
};

}  // foo::gazebo

because gzmsg resolves to (gazebo::common::Console::msg()) which
resolves in this error:

error: ‘foo::gazebo::common’ has not been declared
     #define gzmsg (gazebo::common::Console::msg())

Should this also be fixed in older gazebo version?

This solves #2896

Cheers,
Stefan

cc @Pro

@stefanbuettner stefanbuettner force-pushed the fix_namespace_in_console_macros branch from 7dd3d39 to 7c8831a Compare November 27, 2020 13:54
@chapulina
Copy link
Contributor

Interesting, thank you for the fix. Do you think you could add a test case that wouldn't compile without the fix?

Should this also be fixed in older gazebo version?

Yeah that would be great. We can backport this to gazebo9 after it's merged. I wouldn't worry about Gazebo 7 and 10 since they only have 2 months until EOL.

@stefanbuettner
Copy link
Contributor Author

Yes, I'll add a regression test. I also opened up an issue as a test reference.

Stefan Büttner added 2 commits December 1, 2020 16:02
This code did not work
```cpp
namespace foo::gazebo
{

class bar
{
public:
    void doSomething() {
        gzmsg << "doSomething" << std::endl;
    }
};

}  // foo::gazebo
```

because gzmsg resolves to (gazebo::common::Console::msg()).
It results in this error:
```
error: ‘foo::gazebo::common’ has not been declared
     #define gzmsg (gazebo::common::Console::msg())
```
@stefanbuettner stefanbuettner force-pushed the fix_namespace_in_console_macros branch from 7c8831a to 9ff7bc0 Compare December 1, 2020 16:05
Copy link
Member

@scpeters scpeters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the test and bug fix! you rock!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants