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

Vendor stdbool.h from clang 15.0.2 #2380

Merged
merged 1 commit into from
Jan 5, 2023
Merged

Vendor stdbool.h from clang 15.0.2 #2380

merged 1 commit into from
Jan 5, 2023

Conversation

cole-miller
Copy link
Contributor

@cole-miller cole-miller commented Oct 4, 2022 •
edited

This follows the example of the existing headers in resources, copying stdbool.h from the latest tagged release of LLVM.

Checklist
  • Language changes are updated in man/adoc/bpftrace.adoc and if needed in docs/reference_guide.md
  • User-visible and non-trivial changes updated in CHANGELOG.md
  • The new behaviour is covered by tests

@fbs
Copy link
Contributor

fbs commented Oct 4, 2022

o/ thanks for the contribution. Can you explain what kind of problem the vendoring solves?

@cole-miller
Copy link
Contributor Author

Ah, I should've linked the discussion: #2378. Hopefully @viktormalik can confirm that I'm on the right track here.

viktormalik
viktormalik reviewed Oct 7, 2022
View reviewed changes
Copy link
Contributor

@viktormalik viktormalik left a comment

Choose a reason for hiding this comment

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

Thanks! This is useful when including headers that include <stdbool.h>. Could you perhaps add a runtime test with such a header?

@viktormalik
Copy link
Contributor

ping @cole-miller, I'd like to see a runtime test for this before merging it

@fbs
Copy link
Contributor

fbs commented Dec 9, 2022

shall we just merge this one? it looks ok

@viktormalik
Copy link
Contributor

yeah, should be fine, let's merge

viktormalik
viktormalik approved these changes Jan 5, 2023
View reviewed changes
Copy link
Contributor

@viktormalik viktormalik left a comment

Choose a reason for hiding this comment

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

I resolved the CHANGELOG merge conflict and will merge once the tests pass.

@viktormalik viktormalik merged commit 8f3af22 into bpftrace:master Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@viktormalik viktormalik viktormalik approved these changes

@ajor ajor Awaiting requested review from ajor ajor is a code owner

@danobi danobi Awaiting requested review from danobi danobi is a code owner

@fbs fbs Awaiting requested review from fbs fbs is a code owner

@BurntBrunch BurntBrunch Awaiting requested review from BurntBrunch

Assignees
No one assigned
Labels
waiting-on-author
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@cole-miller @fbs @viktormalik @danobi