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

Restoring tf2_eigen Tests #670

Open
wants to merge 2 commits into
base: rolling
Choose a base branch
from

Conversation

CursedRock17
Copy link
Contributor

This pull request is meant to solve this TODO in which convert needed to be added to complete the tests. The convert function was added in PR #167, the test should pass.

Signed-off-by: CursedRock17 <[email protected]>
Signed-off-by: CursedRock17 <[email protected]>
@ahcorde
Copy link
Contributor

ahcorde commented Apr 9, 2024

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

Looks good to me with green CI.

@clalancette
Copy link
Contributor

@CursedRock17 Oof, unfortunately MSVC thinks that we have an ambiguous overload here. Can you take a look?

@CursedRock17
Copy link
Contributor Author

Sorry for the delay, so it looks like that ambigous functions call has been documented before in Pull Request #434. That solution continues to work even with clang, which was the main issues that we were having due to adl in other compilers. But we could also enable the permissive flag in the tf2_eigen CMakeLists.txt to prevent

forc[ing] everyone using MSVC to use the /permissive- flag

Either way I don't think we're getting around this because two phase lookup hasn't been diminished from MSVC and ADL will be the default on other compilers.

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