-
Notifications
You must be signed in to change notification settings - Fork 3
remove the need for clang by fixing issue in or-tools code #66
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: William Woodall <[email protected]>
|
Also, I went to make the change upstream, but weirdly it's not on their main branch, only on their release branches, so maybe it's already fixed? I'm honestly not sure and having trouble following their branch -> release workflow. |
|
It looks like there is some sort of issue in |
Signed-off-by: William Woodall <[email protected]>
|
Sorry, missed adding the new patch file. |
| public: | ||
| // Makes a vector with initial elements all committed to value. | ||
| - CommittableArray<T>(size_t num_elements, const T& value) | ||
| + CommittableArray(size_t num_elements, const T& value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@codebot this is the required change, AFAIK this is just not valid c++ code, but clang seems to allow it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I just tested this in a "normal" colcon build, and it works great!
codebot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Thanks for tracking this down. It builds perfectly for me now 🎉 using a typical colcon invocation.
I think this was added in a recent sdk version update, but this can be avoided by patching one line in
or-toolswhich this pr does.