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

Fix clang-tidy and use optimizations #2809

Closed
tylerjw opened this issue Aug 12, 2021 · 1 comment · Fixed by #2820
Closed

Fix clang-tidy and use optimizations #2809

tylerjw opened this issue Aug 12, 2021 · 1 comment · Fixed by #2820
Labels
bug help wanted please consider improving this request! more work needed Everyone is invited to contribute

Comments

@tylerjw
Copy link
Member

tylerjw commented Aug 12, 2021

Description

#2792 Changes the optimization level of one package to fix a clang-tidy issue. This seems gross to me and we should be able to figure out another way of fixing this.
@tylerjw tylerjw added the bug label Aug 12, 2021
@v4hn
Copy link
Contributor

v4hn commented Aug 12, 2021 •
edited

it's worse than gross because the file is at the core of collision checking with FCL. So the slowdown is probably quite big.

The issue is not with clang-tidy though.
It's that building libmoveit_collision_detection_fcl.so with clang 10-12 with optimizations on results in broken methods and we recently added a test that points this out.

@v4hn v4hn added help wanted please consider improving this request! more work needed Everyone is invited to contribute labels Aug 13, 2021
@rhaschke rhaschke linked a pull request Aug 17, 2021 that will close this issue
Fix issue #2809 (broken test with clang) #2820
Merged
rhaschke added a commit that referenced this issue Aug 18, 2021
@rhaschke
Fix issue #2809 (broken test with clang) (#2820)
0e9844d 
Because std::make_pair uses the decayed type (std::string), the strings were actually copied into a temporary, which was subsequently referenced by the elements of std::pair, leading to a stack-use-after-scope error.
Explicitly passing const references into std::make_pair via std::cref() resolves the issue mentioned in #2809.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug help wanted please consider improving this request! more work needed Everyone is invited to contribute
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix issue #2809 (broken test with clang) ubi-agni/moveit
2 participants
@v4hn @tylerjw