Skip to content

Dedicated Display type for CStr::display - #162837

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
clarfonthey:c-str-display
Sep 23, 2026
Merged

Dedicated Display type for CStr::display#162837
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
clarfonthey:c-str-display

Conversation

@clarfonthey

@clarfonthey clarfonthey commented Sep 16, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Tracking issue: #139984

Should resolve the FCP concern.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 16, 2026
@clarfonthey
clarfonthey marked this pull request as ready for review September 16, 2026 04:00
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 16, 2026
@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 16, 2026
@rustbot

rustbot commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: libs
  • libs expanded to 12 candidates
  • Random selection from Darksonn, JohnTitor, Mark-Simulacrum, aapoalas, jhpratt

@rust-log-analyzer

This comment has been minimized.

Sign in to view
Darksonn
Darksonn reviewed Sep 16, 2026
View reviewed changes
Comment thread library/core/src/ffi/c_str.rs Outdated
@clarfonthey
clarfonthey force-pushed the c-str-display branch 2 times, most recently from 75237f3 to 2306228 Compare September 16, 2026 12:59
@rust-log-analyzer

This comment has been minimized.

Sign in to view
Darksonn
Darksonn reviewed Sep 16, 2026
View reviewed changes
Comment thread library/core/src/ffi/c_str.rs
}

#[unstable(feature = "cstr_display", issue = "139984")]
impl fmt::Debug for Display<'_> {

@Darksonn Darksonn Sep 16, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The debug impl on the return value of CStr::display() is new, untested behavior. Should we have a test about what this outputs?

View changes since the review

@clarfonthey clarfonthey Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No equivalent tests for os_str::Display and path::Display, so, it should be fine.

Comment thread library/core/src/ffi/c_str.rs
@clarfonthey

clarfonthey commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

r? Darksonn (since you're already doing review, you can defer back to Mark if you want though)

@rustbot rustbot assigned Darksonn and unassigned Mark-Simulacrum Sep 16, 2026
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Darksonn

Darksonn commented Sep 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Looks ok to me. There are still some CI failures to fix. I'll libs nominate it so the rest of the libs team has a chance to comment, but otherwise I'm happy.

@Darksonn Darksonn added the I-libs-nominated Nominated for discussion during a libs team meeting. label Sep 16, 2026
@rustbot

rustbot commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

hanna-kruppe
hanna-kruppe reviewed Sep 17, 2026
View reviewed changes
Comment thread library/core/src/ffi/c_str.rs
@Darksonn Darksonn added S-waiting-on-t-libs Status: Awaiting decision from T-libs and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 19, 2026
@clarfonthey

clarfonthey commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

Meeting says ship it.

@clarfonthey clarfonthey added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed I-libs-nominated Nominated for discussion during a libs team meeting. S-waiting-on-t-libs Status: Awaiting decision from T-libs labels Sep 22, 2026
Darksonn
Darksonn approved these changes Sep 22, 2026
View reviewed changes

@Darksonn Darksonn left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@bors r+

View changes since this review

@rust-bors

rust-bors Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 71ef655 has been approved by Darksonn

It is now in the queue for this repository.

@rust-bors rust-bors Bot 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 22, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Sep 22, 2026
@JonathanBrouwer
Rollup merge of rust-lang#162837 - clarfonthey:c-str-display, r=Darksonn
a3fab40 
Dedicated Display type for CStr::display

Tracking issue: rust-lang#139984

Should resolve the FCP concern.
This was referenced Sep 22, 2026
Closed
Merged
rust-bors Bot pushed a commit that referenced this pull request Sep 22, 2026
@bors
Auto merge of #163178 - JonathanBrouwer:rollup-NTC54oC, r=JonathanBro…
1a82b5d 
…uwer

Rollup of 13 pull requests

Successful merges:

 - #156949 (Detect missing else in let statement)
 - #160436 (stabilize `Box::take`)
 - #160570 (macro_metavar_expr_concat: support concatenating into string literals)
 - #162837 (Dedicated Display type for CStr::display)
 - #163099 (Use wrapping arithmetic in `from_str_radix`)
 - #163166 (Tiny cleanups to deferred liveness)
 - #161667 (Add `f16` inline ASM support for `nvptx64-nvidia-cuda`)
 - #163063 (Restore `Send` and `Sync` for `BorrowedCursor`)
 - #163097 (OpenBSD/sparc64 has switched from GCC to Clang)
 - #163126 (Skip redundant storage-conflict updates during coroutine layout)
 - #163135 (librustdoc: remove stale dep on base64)
 - #163146 (tests: Update `f16b` codegen test for LoongArch and RISC-V)
 - #163159 (treat inductive cycles as ambig)
@rust-bors
rust-bors Bot merged commit 6d10830 into rust-lang:main Sep 23, 2026
13 checks passed
@rustbot rustbot added this to the 1.100.0 milestone Sep 23, 2026
rust-bors Bot pushed a commit that referenced this pull request Sep 23, 2026
@bors
Unrolled build for #162837 in rollup 163178
78982c5 
Rollup merge of #162837 - clarfonthey:c-str-display, r=Darksonn

Dedicated Display type for CStr::display

Tracking issue: #139984

Should resolve the FCP concern.
@clarfonthey
clarfonthey deleted the c-str-display branch September 25, 2026 01:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hanna-kruppe hanna-kruppe hanna-kruppe left review comments
@Darksonn Darksonn Darksonn approved these changes

Assignees

@Darksonn Darksonn

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.100.0

Development

Successfully merging this pull request may close these issues.

6 participants

@clarfonthey @rustbot @rust-log-analyzer @Darksonn @hanna-kruppe @Mark-Simulacrum