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.

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

Account for rounding errors when deciding the diagnostic boundaries #64029

Merged
merged 3 commits into from
Sep 1, 2019

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Aug 31, 2019

Fix Miri by fixing the bug raised in #63402 (comment).

Fixes #64020

@rust-highfive
Copy link
Collaborator

r? @zackmdavis

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 31, 2019
@estebank
Copy link
Contributor Author

Screen Shot 2019-08-30 at 5 47 08 PM

Screen Shot 2019-08-30 at 5 46 53 PM

CC @RalfJung

Properly account for left margin when setting terminal width through
CLI flag and don't trim code by default if we can't get the terminal's
dimensions.
@RalfJung
Copy link
Member

Also I wonder, is there any way you could add a test for this? (But IMO that's not a blocker.)

@RalfJung
Copy link
Member

I can confirm that this fixes Miri. r=me with the comment about saturating_sub resolved (one way or another).

@zackmdavis zackmdavis removed their assignment Aug 31, 2019
@estebank
Copy link
Contributor Author

Also I wonder, is there any way you could add a test for this?

I haven't found why the compile-flags doesn't propagate -Zterminal-width properly to tests. I'll get back to it to increase test coverage, but for now I'm letting it slide with some manual testing 😬

@estebank
Copy link
Contributor Author

estebank commented Sep 1, 2019

r? @RalfJung @bors r=RalfJung rollup

@bors
Copy link
Contributor

bors commented Sep 1, 2019

📌 Commit 8456719 has been approved by RalfJung

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 1, 2019
Centril added a commit to Centril/rust that referenced this pull request Sep 1, 2019
Account for rounding errors when deciding the diagnostic boundaries

Fix Miri by fixing the bug raised in rust-lang#63402 (comment).

Fixes rust-lang#64020
bors added a commit that referenced this pull request Sep 1, 2019
Rollup of 5 pull requests

Successful merges:

 - #63410 (Update BufWriter example to include call to flush())
 - #64029 (Account for rounding errors when deciding the diagnostic boundaries)
 - #64032 (rustdoc use -Ccodegen-units=1 by default for test compile)
 - #64039 (Update sync condvar doc style)
 - #64042 (Fix word repetition in str documentation)

Failed merges:

r? @ghost
@bors bors merged commit 8456719 into rust-lang:master Sep 1, 2019
term_size::dimensions().map(|(w, _)| w - code_offset).unwrap_or(140)
term_size::dimensions()
.map(|(w, _)| w.saturating_sub(code_offset))
.unwrap_or(std::usize::MAX)
Copy link
Member

Choose a reason for hiding this comment

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

Why was this changed to usize::MAX?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bjorn3 If the dimensions aren't supplied and can't be queried, I decided that keeping the current uncropped behavior makes more sense. By setting it to MAX it'll never crop.

@estebank estebank deleted the fix-miri branch November 9, 2023 05:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

miri no longer builds after rust-lang/rust#63402
6 participants