-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Tracking issue for format_args_capture #67984
Comments
Progress update from me: I've been busy but have started work on the implementation of this feature. A bikeshed on the feature name: I'm leaning towards to the name I think that The feature gate error message (on nightly) would then look like this:
Is it too late to use a different feature name? |
maybe |
@davidhewitt |
FWIW at least in my mind capturing implies something closer to what e.g. closures do where the variable is stored within the returned Arguments struct. But that's not something that can be added to Arguments given our stability guarantees, I think, even though I've often wanted it -- so maybe that's not a major concern. I will note though that the naming of the feature gate and diagnostics feel like something that we can adjust over time and in the implementation PR, definitely no need to get them right immediately :) |
…varkor Add `format_args_capture` feature This is the initial implementation PR for [RFC 2795](rust-lang/rfcs#2795). Note that, as dicussed in the tracking issue (rust-lang#67984), the feature gate has been called `format_args_capture`. Next up I guess I need to add documentation for this feature. I've not written any docs before for rustc / std so I would appreciate suggestions on where I should add docs.
…varkor Add `format_args_capture` feature This is the initial implementation PR for [RFC 2795](rust-lang/rfcs#2795). Note that, as dicussed in the tracking issue (rust-lang#67984), the feature gate has been called `format_args_capture`. Next up I guess I need to add documentation for this feature. I've not written any docs before for rustc / std so I would appreciate suggestions on where I should add docs.
Some notes about this problem: There's several macros that currently that handle their no-formatting-arguments case specially, mostly for efficiency:
In all cases, this results in these two problems:
In the case of
Most of the crates from the community using a special no-arguments case are doing this for efficiency as well. Some will use In the case of Not only will
Unless breaking these examples is acceptable, this would need to be changed over a Rust edition. For both
For the macros in the community crates, there are two options w.r.t. an edition change:
My proposal:
Update: The core/std part of this is now an RFC: rust-lang/rfcs#3007 |
Thanks @m-ou-se for the detailed write-up. I'm nominating this for @rust-lang/lang discussion since it has Edition ramifications, although I guess that the design of these macros is in many ways a @rust-lang/libs decision. I agree with you that an edition is the appropriate way to manage this, and I think we should shoot for The automated migration seems easy enough, in any case, since |
For |
I can see the I'm generally in favor of removing |
Something missing from my notes/proposal above is making the behaviour consistent or at least not too unexpected on 2018/2015, after If the changes to core/std's panic!() are gated behind edition 2021, then in 2018/2015 we get: let a = 5;
println!("{a}"); // prints "5"
panic!("{a}"); // panics with "{a}"
println!("{a} {}", 5); // prints "5 5"
panic!("{a} {}", 5); // panics with "5 5" Possible ways to make this better:
None of these seem great. :( |
This could also be |
Maybe, but if |
We discussed this in today's @rust-lang/lang meeting. The sense of the meeting was:
|
My personal opinion is that One interesting question that I don't know the answer to is whether |
My preference would be to deprecate |
Supporting Anicdote: I used a non-string panic once on accident and it took me well over 30 seconds to even understand what had gone wrong (because the compiler accepted it, so "clearly" i must have typed the right thing). |
I too have only only ever experienced non-string panics in error. Especially the first time I did this it took quite a while to figure out where the @nikomatsakis when you say the lang team intends to keep all existing functionality, do you mean that all of the functionality remains on the macros? Or is moving the "panic with expression" case to a new function potentially on the table? My reservation with the proposed As for a migration strategy, I prefer @m-ou-se 's option 3. I would like to propose that we do the following things at the same time without waiting for an editon change:
After the edition change the single-argument panic case changes to exclusively expect a format string, and we're done! I think this can be implemented by changing the single-argument panic arm to forward to a new compiler-builtin macro. This macro would be the only code that needs to change behaviour with an edition change. I need to think further about the exact design of this new built-in macro. I think some downstream libraries which behave similarly to |
I agree that
In general, I think the way to "phase in" new syntax should fit this mold (and your suggestion does):
I guess my one concern with the plan as described is that it seems like it's a bit surprising that |
One idea that crossed my mind was simply making this panic-with-a-value case a function called I kind of like
👍 I don't have a strong opinion on whether it should be opt-in so happy to defer to core team members on this point. One thing to consider is that perhaps it's already surprising for a lot of people that |
This comment has been minimized.
This comment has been minimized.
Currently suggested improved error message: error: expressions may not be used inside format strings
--> .\scratch\test.rs:3:37
|
3 | println!("hello {get_person()}");
| ^^^^^^^^^^^^^^ expression is here
|
= note: if you wanted to pass an expression as an argument to a formatting macro,
try as a positional argument, e.g. println!("hello {}", get_person());
or as a named argument, e.g. println!("hello {foo}", foo=get_person()); The message should also suggest using a local variable, e.g. |
What remains to be done for this feature? Just documentation in |
Perhaps @davidhewitt or @rust-lang/lang can weigh in? |
As far as I know, yes, the information in this thread is still current. Also the suggestion to improve the diagnostic sounds like a good idea to me. |
I've posted a stabilization PR (including documentation updates) at #90473 |
proc_macro: Add an expand_expr method to TokenStream This feature is aimed at giving proc macros access to powers similar to those used by builtin macros such as `format_args!` or `concat!`. These macros are able to accept macros in place of string literal parameters, such as the format string, as they perform recursive macro expansion while being expanded. This can be especially useful in many cases thanks to helper macros like `concat!`, `stringify!` and `include_str!` which are often used to construct string literals at compile-time in user code. For now, this method only allows expanding macros which produce literals, although more expressions will be supported before the method is stabilized. In earlier versions of this PR, this method exclusively returned `Literal`, and spans on returned literals were stripped of expansion context before being returned to be as conservative as possible about permission leakage. The method's naming has been generalized to eventually support arbitrary expressions, and the context stripping has been removed (rust-lang#87264 (comment)), which should allow for more general APIs like "format_args_implicits" (rust-lang#67984) to be supported as well. ## API Surface ```rust impl TokenStream { pub fn expand_expr(&self) -> Result<Expr, ExpandError>; } #[non_exhaustive] pub struct ExpandError; impl Debug for ExpandError { ... } impl Display for ExpandError { ... } impl Error for ExpandError {} impl !Send for ExpandError {} impl !Sync for ExpandError {} ```
proc_macro: Add an expand_expr method to TokenStream This feature is aimed at giving proc macros access to powers similar to those used by builtin macros such as `format_args!` or `concat!`. These macros are able to accept macros in place of string literal parameters, such as the format string, as they perform recursive macro expansion while being expanded. This can be especially useful in many cases thanks to helper macros like `concat!`, `stringify!` and `include_str!` which are often used to construct string literals at compile-time in user code. For now, this method only allows expanding macros which produce literals, although more expressions will be supported before the method is stabilized. In earlier versions of this PR, this method exclusively returned `Literal`, and spans on returned literals were stripped of expansion context before being returned to be as conservative as possible about permission leakage. The method's naming has been generalized to eventually support arbitrary expressions, and the context stripping has been removed (rust-lang#87264 (comment)), which should allow for more general APIs like "format_args_implicits" (rust-lang#67984) to be supported as well. ## API Surface ```rust impl TokenStream { pub fn expand_expr(&self) -> Result<TokenStream, ExpandError>; } #[non_exhaustive] pub struct ExpandError; impl Debug for ExpandError { ... } impl Display for ExpandError { ... } impl Error for ExpandError {} impl !Send for ExpandError {} impl !Sync for ExpandError {} ```
…pture, r=Mark-Simulacrum stabilize format args capture Works as expected, and there are widespread reports of success with it, as well as interest in it. RFC: rust-lang/rfcs#2795 Tracking issue: rust-lang#67984 Addressing items from the tracking issue: - We don't support capturing arguments from a non-literal format string like `format_args!(concat!(...))`. We could add that in a future enhancement, or we can decide that it isn't supported (as suggested in rust-lang#67984 (comment) ). - I've updated the documentation. - `panic!` now supports capture as well. - There are potentially opportunities to further improve diagnostics for invalid usage, such as if it looks like the user tried to use an expression rather than a variable. However, such cases are all already caught and provide reasonable syntax errors now, and we can always provided even friendlier diagnostics in the future.
Are there any plans to add support for something like Python's |
The |
As I understand it, |
you can pass a reference to |
Now that #90473 has been merged, can this be closed? |
Do we have a plan about the accepting expression |
The most recent discussion I've seen on that is at https://internals.rust-lang.org/t/how-to-allow-arbitrary-expressions-in-format-strings |
i'd love to use |
This is a tracking issue for the RFC 2795 (rust-lang/rfcs#2795).
Steps:
Checklist for the tracking issue:
Steps:
format_args!
behindformat_implicit_args
feature (name to be bikeshed?)format_args!(concat!(...))
case that doesn't risk spurious macro hygienepanic!
. Behind feature gate and / or edition switch (seeunresolved question) Tracking Issue for RFC 3007: Making core and std's panic identical in Rust 2021 #80162Unresolved Questions:
format_args!(concat!(...))
- perhaps try out a new unstable macro as per this comment - Resolved by Tracking issue for format_args_capture #67984 (comment)panic!
solution - Solved by rfc 3007The text was updated successfully, but these errors were encountered: