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

Explicitly initialize instances of tf2::Duration #291

Merged
merged 1 commit into from
Jul 28, 2020

Conversation

mjcarroll
Copy link
Member

This should resolve test failures in Windows debug builds.

tf2::Duration is a type alias of std::chrono::nanoseconds, which has a default-ed constructor, which does not guarantee that the value will be initialized to zero. This explicitly initializes all of the instances of tf2::Duration.

Signed-off-by: Michael Carroll michael@openrobotics.org

Signed-off-by: Michael Carroll <michael@openrobotics.org>
@mjcarroll mjcarroll added this to In progress in Towards Green CI via automation Jul 28, 2020
@mjcarroll mjcarroll self-assigned this Jul 28, 2020
@mjcarroll
Copy link
Member Author

  • Windows Release: Build Status
  • Windows Debug: Build Status

@mjcarroll
Copy link
Member Author

As a follow-up, we may want to do something to warn people trying to use this undefined behavior as it can produce really subtle bugs.

@clalancette
Copy link
Contributor

As a follow-up, we may want to do something to warn people trying to use this undefined behavior as it can produce really subtle bugs.

I'm not sure of all of the implications, but could we make tf2::Duration its own class that either derives from or has-a std::chrono::nanoseconds? That way we could always do the initialization to at least zero.

@mjcarroll mjcarroll merged commit 7a295c0 into ros2 Jul 28, 2020
Towards Green CI automation moved this from In progress to Done Jul 28, 2020
@delete-merged-branch delete-merged-branch bot deleted the windows_buffer_core branch July 28, 2020 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants