Skip to content

Tracking Issue for fmt::Arguments::as_str() #74442

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
m-ou-se opened this issue Jul 17, 2020 · 14 comments · Fixed by #82120
Closed

Tracking Issue for fmt::Arguments::as_str() #74442

m-ou-se opened this issue Jul 17, 2020 · 14 comments · Fixed by #82120
Labels
A-fmt Area: `core::fmt` B-RFC-implemented Blocker: Approved by a merged RFC and implemented but not stabilized. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. Libs-Small Libs issues that are considered "small" or self-contained Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@m-ou-se
Copy link
Member

m-ou-se commented Jul 17, 2020

Tracking issue for fmt::Arguments::as_str() (#74056), gated by #![feature(fmt_as_str)].

@m-ou-se m-ou-se added the C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC label Jul 17, 2020
@jonas-schievink jonas-schievink added B-unstable Blocker: Implemented in the nightly compiler and unstable. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jul 17, 2020
@KodrAus KodrAus added B-RFC-implemented Blocker: Approved by a merged RFC and implemented but not stabilized. A-fmt Area: `core::fmt` Libs-Small Libs issues that are considered "small" or self-contained Libs-Tracked Libs issues that are tracked on the team's project board. labels Jul 29, 2020
@nikki-nn4
Copy link

I don't quite understand fmt internals, but Make fmt::Arguments::as_str() return a 'static str. seems unnecessarily restrictive. And it breaks runtime-fmt which depends on hidden apis e.g. Arguments::new_v1

@tesuji
Copy link
Contributor

tesuji commented Aug 7, 2020

Arguments cannot be generated at runtime because it cannot safely be done. So as_str returns &'static str makes sense.

@sfackler
Copy link
Member

I'd like to be able to use this in the log crate to correctly implement a codegen optimization that doesn't really work otherwise: rust-lang/log#446

@rfcbot fcp merge

@rfcbot
Copy link
Collaborator

rfcbot commented Jan 24, 2021

Team member @sfackler has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jan 24, 2021
@BurntSushi
Copy link
Member

This SGTM. One question though. Are we sure that the 'static bound will always be correct here?

@m-ou-se
Copy link
Member Author

m-ou-se commented Jan 24, 2021

fmt::Arguments is the result from parsing a formatting string at compile time. In the future, this might not always be a literal but might be a const too. But a non-static/constant format string will be a very different beast.

@BurntSushi
Copy link
Member

That's what I figured, but wanted to make sure it was explicitly addressed.

@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Jan 24, 2021
@rfcbot
Copy link
Collaborator

rfcbot commented Jan 24, 2021

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Jan 24, 2021
@jhpratt
Copy link
Member

jhpratt commented Jan 26, 2021

Does this make any guarantees about when Some is returned? I've been interested in redoing the format_args! macro to be much more intelligent (and thus performant), but obviously don't know when it's possible to return something statically knowable.

@m-ou-se
Copy link
Member Author

m-ou-se commented Jan 26, 2021

Currently it returns Some for format_args!("string literal") only. But my plan is to extend that also to format_args!("{}", "literal"), format_args!("{}", constant), and possibly also things like format_args!("a {} b {} c", const, format_args!("...")), etc. See #78356

@jhpratt
Copy link
Member

jhpratt commented Jan 26, 2021

Yeah, I see that from looking at the source code. I was more inquiring about whether it would be considered breaking to go from returning Some to returning None. I don't suspect anything would be out-of-reach when reimplementing the macro, but you never know.

@rfcbot rfcbot added the finished-final-comment-period The final comment period is finished for this PR / Issue. label Feb 3, 2021
@rfcbot
Copy link
Collaborator

rfcbot commented Feb 3, 2021

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@rfcbot rfcbot added to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Feb 3, 2021
@sfackler
Copy link
Member

sfackler commented Feb 3, 2021

@jhpratt I think you'd generally want to use it as an optimization only, but we can probably guarantee that it will return Some for certain cases in particular (e.g. format_args!("asdf")).

@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Feb 11, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this issue Feb 15, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this issue Feb 15, 2021
@bors bors closed this as completed in 4613b37 Feb 16, 2021
@Zoxc
Copy link
Contributor

Zoxc commented Sep 5, 2023

This function seems particularly bad for compressed formatting strings and breaks the otherwise clean abstraction.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-fmt Area: `core::fmt` B-RFC-implemented Blocker: Approved by a merged RFC and implemented but not stabilized. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. Libs-Small Libs issues that are considered "small" or self-contained Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.