Uh oh!
There was an error while loading. Please reload this page.
Try to ensure usize marker does not get merged - #69651
Conversation
There was a problem hiding this comment.
If you make this return Ok(()), you could actually call it before reading the usize from Argument, ensuring that what this closure does is guaranteed to happen, giving more confidence that if there was UB, it was invoked by calling the fn pointer which should be safe to call.
There was a problem hiding this comment.
It's a bit annoying to call the formatter as we don't have a &mut Formatter<'_> around and I'd rather not manufacture one just for this. I think we could likely thread it through but I think that hurts readability and such so I'd rather not.
There was a problem hiding this comment.
Oh, right, that method doesn't take the formatter, fair enough.
Uh oh!
There was an error while loading. Please reload this page.
976aff7 to
03e4bb2CompareMark-Simulacrum
commented
Mar 3, 2020
Changed to |
There was a problem hiding this comment.
Are we using black_box here for functional correctness?
There was a problem hiding this comment.
Yes, we were, I think I agree we probably shouldn't be. Switched to a volatile read.
03e4bb2 to
a9259fbCompareeddyb
commented
Mar 5, 2020
@bors r+ |
bors
commented
Mar 5, 2020
📌 Commit a9259fb has been approved by |
…=eddyb Try to ensure usize marker does not get merged This follows up on [this conversation](rust-lang#69209 (comment)). However, I'm not confident this is quite correct, so feedback is appreciated, as always.
Rollup of 8 pull requests Successful merges: - #69631 (remove non-sysroot sources from rust-src component) - #69646 (Miri visitor: detect primitive types based on type, not layout (also, more tests)) - #69651 (Try to ensure usize marker does not get merged) - #69668 (More documentation and simplification of BTreeMap's internals) - #69685 (unix: Don't override existing SIGSEGV/BUS handlers) - #69771 (Cleanup E0390 explanation) - #69777 (Add missing ` in doc for File::with_options()) - #69812 (Refactorings to method/probe.rs and CrateId) Failed merges: r? @ghost
Rollup of 7 pull requests Successful merges: - #69631 (remove non-sysroot sources from rust-src component) - #69646 (Miri visitor: detect primitive types based on type, not layout (also, more tests)) - #69651 (Try to ensure usize marker does not get merged) - #69668 (More documentation and simplification of BTreeMap's internals) - #69771 (Cleanup E0390 explanation) - #69777 (Add missing ` in doc for File::with_options()) - #69812 (Refactorings to method/probe.rs and CrateId) Failed merges: r? @ghost
This follows up on this conversation. However, I'm not confident this is quite correct, so feedback is appreciated, as always.