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

Mark std::fmt::from_fn as #[must_use] #136502

Merged
merged 1 commit into from
Feb 5, 2025

Conversation

yotamofek
Copy link
Contributor

While working on #135494 I managed to shoot my own foot a few times by forgetting to actually use the result of fmt::from_fn, so I think a #[must_use] could be appropriate!

Didn't have a good message to put in the attr so left it blank, still unstable so we can come back to it I guess?

cc #117729 (and a huge +1 for getting it stabilized, it's very useful IMHO)

@rustbot
Copy link
Collaborator

rustbot commented Feb 3, 2025

r? @tgross35

rustbot has assigned @tgross35.
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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 3, 2025
@tgross35
Copy link
Contributor

tgross35 commented Feb 3, 2025

I think this is fine but it should get an ack from the api team

r? libs-api

Didn't have a good message to put in the attr so left it blank, still unstable so we can come back to it I guess?

This should be simple enough to figure out now, maybe something along the lines of "result does nothing unless used in a formatter".

@rustbot rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Feb 3, 2025
@rustbot rustbot assigned BurntSushi and unassigned tgross35 Feb 3, 2025
@workingjubilee
Copy link
Member

"returns a type implementing Debug and Display, which do not have any effects unless they are used".

@tgross35
Copy link
Contributor

tgross35 commented Feb 3, 2025

I think burntsushi has been taking a break from reviews

r? libs-api

@rustbot rustbot assigned dtolnay and unassigned BurntSushi Feb 3, 2025
@yotamofek
Copy link
Contributor Author

yotamofek commented Feb 3, 2025

"result does nothing unless used in a formatter"

Maybe "in a formatting operation"?
Though I'm not sure that's any more informative than leaving it blank, and the std has an abundance of must_uses with no message so it wouldn't be a first.

@yotamofek
Copy link
Contributor Author

"returns a type implementing Debug and Display, which do not have any effects unless they are used".

I like that one..
I'll put it in.
Thanks!

@workingjubilee
Copy link
Member

workingjubilee commented Feb 3, 2025

The need to avoid "formatter" or "formatting operation" becomes obvious considering you could just call .to_string() on it, after all. (this technically does call a "formatter" but in an obscured way that people won't really think about).

@yotamofek yotamofek force-pushed the pr/fmt-from-fn-must-use branch from 0474182 to 6b016d7 Compare February 3, 2025 20:19
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thank you!

@dtolnay
Copy link
Member

dtolnay commented Feb 3, 2025

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 3, 2025

📌 Commit 6b016d7 has been approved by dtolnay

It is now in the queue for this repository.

@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 Feb 3, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Feb 4, 2025
… r=dtolnay

Mark `std::fmt::from_fn` as `#[must_use]`

While working on rust-lang#135494 I managed to shoot my own foot a few times by forgetting to actually use the result of `fmt::from_fn`, so I think a `#[must_use]` could be appropriate!

Didn't have a good message to put in the attr so left it blank, still unstable so we can come back to it I guess?

cc rust-lang#117729 (and a huge +1 for getting it stabilized, it's very useful IMHO)
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 4, 2025
Rollup of 9 pull requests

Successful merges:

 - rust-lang#128045 (#[contracts::requires(...)]  + #[contracts::ensures(...)])
 - rust-lang#136263 (rustdoc: clean up a bunch of ts-expected-error declarations in main)
 - rust-lang#136375 (cg_llvm: Replace some DIBuilder wrappers with LLVM-C API bindings (part 1))
 - rust-lang#136392 (bootstrap: add wrapper macros for `feature = "tracing"`-gated `tracing` macros)
 - rust-lang#136405 (rustdoc-book: Clean up section on `--output-format`)
 - rust-lang#136497 (Report generic mismatches when calling bodyless trait functions)
 - rust-lang#136502 (Mark `std::fmt::from_fn` as `#[must_use]`)
 - rust-lang#136509 (Add tests for nested macro_rules edition behavior)
 - rust-lang#136526 (mir_build: Rename `thir::cx::Cx` to `ThirBuildCx` and remove `UserAnnotatedTyHelpers`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 5, 2025
Rollup of 8 pull requests

Successful merges:

 - rust-lang#128045 (#[contracts::requires(...)]  + #[contracts::ensures(...)])
 - rust-lang#136263 (rustdoc: clean up a bunch of ts-expected-error declarations in main)
 - rust-lang#136375 (cg_llvm: Replace some DIBuilder wrappers with LLVM-C API bindings (part 1))
 - rust-lang#136392 (bootstrap: add wrapper macros for `feature = "tracing"`-gated `tracing` macros)
 - rust-lang#136396 (rustdoc-json-types: Document that crate name isn't package name.)
 - rust-lang#136405 (rustdoc-book: Clean up section on `--output-format`)
 - rust-lang#136502 (Mark `std::fmt::from_fn` as `#[must_use]`)
 - rust-lang#136509 (Add tests for nested macro_rules edition behavior)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ba42006 into rust-lang:master Feb 5, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Feb 5, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 5, 2025
Rollup merge of rust-lang#136502 - yotamofek:pr/fmt-from-fn-must-use, r=dtolnay

Mark `std::fmt::from_fn` as `#[must_use]`

While working on rust-lang#135494 I managed to shoot my own foot a few times by forgetting to actually use the result of `fmt::from_fn`, so I think a `#[must_use]` could be appropriate!

Didn't have a good message to put in the attr so left it blank, still unstable so we can come back to it I guess?

cc rust-lang#117729 (and a huge +1 for getting it stabilized, it's very useful IMHO)
@yotamofek yotamofek deleted the pr/fmt-from-fn-must-use branch February 5, 2025 12:01
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. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants