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

Change in behavior of debug format of strings containing \0 #95732

Closed
carols10cents opened this issue Apr 6, 2022 · 3 comments
Closed

Change in behavior of debug format of strings containing \0 #95732

carols10cents opened this issue Apr 6, 2022 · 3 comments
Labels
C-bug Category: This is a bug. regression-from-stable-to-beta Performance or correctness regression from stable to beta.
Milestone

Comments

@carols10cents
Copy link
Member

I understand debug formatting is generally unstable, but wanted to raise this to make sure the change was intentional.

Code

I tried this test:

#[test]
fn quote_not_printable() {
    assert_eq!(format!("{:?}", "foo\0bar"), r#""foo\u{0}bar""#);
}

This test passes with stable Rust 1.60.0 (7737e0b 2022-04-04) (the release candidate that's going to be released tomorrow)

I expected this test to continue passing with 1.61.0-beta.1

Instead, I get this failure:


---- quote_not_printable stdout ----
thread 'quote_not_printable' panicked at 'assertion failed: `(left == right)`
  left: `"\"foo\\0bar\""`,
 right: `"\"foo\\u{0}bar\""`', src/lib.rs:3:5

Version it worked on

It most recently worked on: stable release candidate 1.60.0 (7737e0b 2022-04-04)

Version with regression

rustc --version --verbose:

rustc 1.61.0-beta.1 (0f231250b 2022-04-05)
binary: rustc
commit-hash: 0f231250ba7fe3a3f98a18aee9abfd65b104fb92
commit-date: 2022-04-05
host: aarch64-apple-darwin
release: 1.61.0-beta.1
LLVM version: 14.0.0

@rustbot modify labels: +regression-from-stable-to-beta -regression-untriaged

@carols10cents carols10cents added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Apr 6, 2022
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. and removed regression-untriaged Untriaged performance or correctness regression. labels Apr 6, 2022
@carols10cents
Copy link
Member Author

I see #95345 now. Context: this real-world test: https://github.com/influxdata/influxdb_iox/blob/c244b0328125b3fd1773b58d4e1b1aaf47c83fd3/logfmt/src/lib.rs#L387

I see this has caused some churn in ripgrep too, up to you all if this change is worth the churn.

@carols10cents carols10cents changed the title Change in behavior of debug format of strings containing escapes Change in behavior of debug format of strings containing \0 Apr 6, 2022
@pietroalbini pietroalbini added this to the 1.61.0 milestone Apr 6, 2022
@dtolnay
Copy link
Member

dtolnay commented Apr 13, 2022

I understand debug formatting is generally unstable, but wanted to raise this to make sure the change was intentional.

This change was intentional. Indeed all debug formattings are always subject to change.

I see this has caused some churn in ripgrep too, up to you all if this change is worth the churn.

I'm not sure what this refers to — the ripgrep master branch has had no new commits since when this change landed in nightly, no PRs have been opened on ripgrep either, and no relevant-looking issues have been opened as far as I can tell. Ripgrep has been using the \0 formatting in its own output since 1.5 years ago.

@dtolnay dtolnay closed this as completed Apr 13, 2022
@carols10cents
Copy link
Member Author

For anyone else who's curious (I'm not trying to relitigate this issue or anything), the ripgrep churn I was referring to was https://github.com/BurntSushi/ripgrep/pull/1722/files#diff-4dda3b4b1094bbd540a7548319bd9b4367e93ce4662873d863c219b1b0799adbL43 that I found through this comment.

krtab added a commit to krtab/xmlparser that referenced this issue Oct 12, 2022
This follows this PR in rustc:
<rust-lang/rust#95345>

Which breakage was discussed here:
<rust-lang/rust#95732>

Because this landed in 1.61.0, the MSRV has to be updated accordingly.
krtab added a commit to krtab/xmlparser that referenced this issue Oct 12, 2022
This follows this PR in rustc:
<rust-lang/rust#95345>

Which breakage was discussed here:
<rust-lang/rust#95732>

Still relying on \0 would require bumping the MSRV to 1.61.0
krtab added a commit to krtab/xmlparser that referenced this issue Oct 12, 2022
This follows this PR in rustc:
<rust-lang/rust#95345>

Which breakage was discussed here:
<rust-lang/rust#95732>

Still relying on \0 would require bumping the MSRV to 1.61.0
@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. regression-from-stable-to-beta Performance or correctness regression from stable to beta.
Projects
None yet
Development

No branches or pull requests

5 participants