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.

Sign up for GitHub

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

Jump to bottom

conditionally undefine NO_ERROR for Windows. #247

Closed
wants to merge 2 commits into from
Closed

conditionally undefine NO_ERROR for Windows. #247

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7e46eb9
conditionally undefine NO_ERROR for Windows.
seanyen Apr 8, 2020
a6fd94c
Merge branch 'ros2' into seanyen/windows
seanyen Aug 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions tf2/include/tf2/exceptions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@

#include "tf2/visibility_control.h"

#if defined(_WIN32) && defined(NO_ERROR)
#undef NO_ERROR
#endif
Comment on lines +40 to +42
Copy link
Contributor

Choose a reason for hiding this comment

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

@seanyen I think the right way to fix this is as follows:

  1. Modify the enum class TF2Error below so that it looks something like:
enum class TF2Error : std::uint8_t
{
  TF2_NO_ERROR = 0,
  TF2_LOOKUP_ERROR = 1,
  TF2_CONNECTIVITY_ERROR = 2,
  TF2_EXTRAPOLATION_ERROR = 3,
  TF2_INVALID_ARGUMENT_ERROR = 4,
  TF2_TIMEOUT_ERROR = 5,
  TF2_TRANSFORM_ERROR = 6,

  NO_ERROR [[deprecated]] = 0,
  LOOKUP_ERROR [[deprecated]] = 1,
  CONNECTIVITY_ERROR [[deprecated]] = 2,
  EXTRAPOLATION_ERROR [[deprecated]] = 3,
  INVALID_ARGUMENT_ERROR [[deprecated]] = 4,
  TIMEOUT_ERROR [[deprecated]] = 5,
  TRANSFORM_ERROR [[deprecated]] = 6
};

(my understanding is that the [[deprecated]] syntax is standard in C++14, so should work with MSVC as well, but please let me know if that is not the case)

  1. Fix the callers in the tree that use the deprecated enum values.
  2. Still put the workaround above in for Galactic.
  3. After Galactic, remove the deprecated enum values.

Does that make sense?

Copy link
Author

Choose a reason for hiding this comment

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

This proposal is totally making sense to me. I like the idea to prefix the enum to avoid the collision entirely. [[deprecated]] is supported by the latest MSVC, so we should be good too. Do you want me to rework this pull request or you already have something on the way?

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want me to rework this pull request or you already have something on the way?

If you could rework this PR, that would be great. Keep in mind you'll probably have to change the users of the enum in other packages within geometry2 (it looks like at least tf2_ros, tf2_py, and tf2_ros_py).

Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

See #349, which implements this (I needed it for other reasons). I'm going to go ahead and close this PR out in favor of that one.


namespace tf2
{

Expand Down