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

Fix tf2_eigen for MSVC #434

Open
wants to merge 1 commit into
base: rolling
Choose a base branch
from
Open

Fix tf2_eigen for MSVC #434

wants to merge 1 commit into from

Conversation

Ace314159
Copy link

Trying to use any of the convert functions results in an ambiguous call compile error on MSVC. Here is an example:

geometry_msgs::msg::Quaternion quat;
tf2::convert(Eigen::Quaterniond(), quat);

This is because the Eigen namespace contains some implementations of toMsg and fromMsg. These implementations seem to be required on other compilers like Clang and GCC because of something called two-phase name lookup, which is explained in a comment in the code as well as this Microsoft blog post.

However, as stated in that blog post, two-phase name look up is not enabled by default on MSVC because it was recently added. In order to enable it, the compiler flag /permissive- needs to be used. With that flag, the existing code does compile, but having to use a compiler flag just to use this library doesn't sound ideal.

Instead, I think it would be better to just not have those implementations in the Eigen namespace for MSVC as I have done.

@Ace314159 Ace314159 mentioned this pull request Jul 1, 2021
6 tasks
@traversaro
Copy link
Contributor

As this modification is done in a header file, it may be eventually included in a compilation unit that is compiled with /permissive- . Do you have any idea if there is a macro to detect in the preprocessor if /permissive- is used, so the header will work both for MSVC without /permissive- and with /permissive-?

@Ace314159
Copy link
Author

@traversaro Good catch! I hadn't considered that. Regarding a macro, all I could find was a request to add a macro here, but it looks they haven't actually added it.

Perhaps, the solution might be to just force everyone using MSVC to use the /permissive- flag. Do you have any other ideas?

@clalancette clalancette self-assigned this Aug 12, 2021
@audrow audrow changed the base branch from ros2 to rolling June 28, 2022 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants