-
-
Notifications
You must be signed in to change notification settings - Fork 45
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
Updated to bitflags 2.2.1. #138
Conversation
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 for the PR!
As you've identified, this is indeed a breaking change, which means we'll need to land it in the dev/0.7
branch for now.
Though, before we land it... I would like to get to the bottom of why check_size.sh
went up by ~700 bytes. That's quite a lot for something as simple as bitflags!
Any theories?
One of the major changes between |
Do you have a pressing need to merge this PR in? If so, I don't mind landing it in |
Thanks for the reply. For context, I work on Android. We vendor in all the Rust crates that we use (you can see them at https://cs.android.com/android/platform/superproject/+/master:external/rust/crates/ if you're curious), and
So from my point of view the sooner this change is in and released the better, but there are still a number of other crates in the same position or otherwise blocking the update so if we have to maintain local patches then we will. |
Thanks for the context! As someone who often takes point manicuring the dependency tree at $work, I completely understand. I've sent out my fair share of PRs and DMs asking projects to bump Unfortunately, I don't have a great estimate for when I'll be cutting Let me know if you think it'd be helpful to land this PR in |
Okay, I had a chance to dig in here, and I've found the culprit.
I've tested this theory by reverting the bitflags changes, and adding a simple call to Not gonna lie, this is pretty funny, and I'm glad I could figure this out haha. Anyways, in terms of "action items" - I think the best thing to do would be to swap out the current |
Thanks for digging into that! It sounds rather like a compiler bug, might be worth filing it somewhere? |
Maybe... but this might be niche enough that I'm not gonna do it myself. Activation energy and whatnot ¯\_(ツ)_/¯ In any case... I think we should leave this PR open until bitflags/bitflags#348 (comment) resolves, at which point, we can switch over to the more lightweight macro, and sidestep this weird compiler interaction entirely. |
Okay, thanks. |
I've updated to bitflags 2.3.1 and the external version of the macro. Before/After `./example_no_std/check_size.sh` outputBefore
After
|
Sweet, that's awesome. Thanks again! |
FYI, gdbstub 0.7 has just been released. Cheers! |
Great, thanks! |
Description
This updates the
bitflags
dependency to the latest version. As it is a major version change, some small code changes are required.API Stability
This is a breaking API change, because it changes the API of the public types
HostIoOpenFlags
andHostIoOpenMode
which are generated by thebitflags!
macro.Checklist
rustdoc
formatting looks good (viacargo doc
)examples/armv4t
withRUST_LOG=trace
+ any relevant GDB output under the "Validation" section below./example_no_std/check_size.sh
before/after changes under the "Validation" section belowexamples/armv4t
./example_no_std/check_size.sh
)Arch
implementationValidation
GDB output
armv4t output
Before/After `./example_no_std/check_size.sh` output
Before
After