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

Test failures when building with clang #939

Closed
dsobek opened this issue Feb 25, 2025 · 7 comments
Closed

Test failures when building with clang #939

dsobek opened this issue Feb 25, 2025 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@dsobek
Copy link

dsobek commented Feb 25, 2025

Describe the bug
When building with clang on master, tests are failing with c++ exception:
C++ exception with description "Value outside numeric limits" thrown in the test body.

Any.Cast appears to be the most minimal testcase that is failing. It comes down to casts from any integral type to a double type.

How to Reproduce

Please provide a specific description of how to reproduce the issue or source code that can be compiled and executed. Please attach a file/project that is easy to compile, don't copy and paste code snippets!

From project root:

CC=clang CXX=clang++ colcon build
./build/behaviortree_cpp/tests/behaviortree_cpp_test

Root cause

I've found that the root cause is in checkTruncation

In particular if To has a larger max value than From (say for example To is double and From is int), static_cast<From>(std::numeric_limits<To>::max()) does not always evaluate to the max value of From. In clang, with any optimization (I see behaviortree.cpp is built with -O3), this evaluates to 0. I am not sure if this is meant to be undefined behavior but it sure is acting like it.

@dsobek
Copy link
Author

dsobek commented Feb 25, 2025

Potential solution in #940

@Aglargil
Copy link
Contributor

This might help too #932

@facontidavide
Copy link
Collaborator

I am aware of the issue.

@facontidavide
Copy link
Collaborator

the change in the code was introduced mased on #919 , as suggested by @cktii

but clearly that is not a solution that works on all platforms.

@Aglargil how is #932 related?

@dsobek your PR #940 doesn't pass the tests in wondows

@facontidavide facontidavide self-assigned this Feb 25, 2025
@facontidavide facontidavide added the bug Something isn't working label Feb 25, 2025
@Aglargil
Copy link
Contributor

@Aglargil how is #932 related?

@facontidavide
I found this error when I was adding the _onStart precondition function to the script, so I fixed it, you can take a look at this commit: d8ca1f1

@facontidavide
Copy link
Collaborator

fixed with #941 .Thanks @Aglargil you were right!

@dsobek
Copy link
Author

dsobek commented Feb 25, 2025

Thanks for fixing @facontidavide!

@dsobek your PR #940 doesn't pass the tests in wondows

For the record, I think you were looking at Picknik's fork in PickNikRobotics#6 where we haven't gotten around to fixing the windows build because we don't support it. I don't think #940 would've shown the same failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants