proc_macro: Add an expand_expr method to TokenStream

[mystor]

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 (#87264 (comment)), which should allow for more general APIs like "format_args_implicits" (#67984) to be supported as well.

API Surface

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 {}

[rust-highfive]

r? @petrochenkov

(rust-highfive has picked a reviewer for you, use r? to override)

[mystor]

I've left this as a draft PR for now as I'm unsure whether a feature like this would be desired, and/or whether an RFC would be required before landing.

[petrochenkov]

I'm unsure whether a feature like this would be desired, and/or whether an RFC would be required before landing.

There were some RFCs about eager expansion (of which expand_literal is a special case), all postponed - rust-lang/rfcs#2320, rust-lang/rfcs#1628, rust-lang/rfcs#2628.

[mystor]

There were some RFCs about eager expansion (of which expand_literal is a special case), all postponed - rust-lang/rfcs#2320, rust-lang/rfcs#1628, rust-lang/rfcs#2628.

Ah, that's good to know. Thanks for the links (I should've probably looked them up earlier). I take it that that means a change like this would require a corresponding RFC :-)

[petrochenkov]

I'm mostly ok with merging something like this in general.
The proc_macro crate contains a lot of experimental stuff with unclear path to stabilization.

Why stabilizing this may be simpler than eager expansion in general:

• We reserve the right to return Err on a whim. For example if the macro to expand is not yet resolvable at this point in compilation, but may become resolvable later.
  • It's not clear whether it's ok to continue compilation after receiving Err from expand_literal. Err not becoming Ok in the future must not be something that users can rely on.
  • If expand_literal returns Ok, but compiler produces errors while evaluating it, then it's still a compilation error, so we are good.
• All the issues with complex module structures existing during eager expansion are avoided, if the expansion does not result in a single literal token in the end, then it's an error (although see the previous item).
• This is a very limited and special-purpose version of some potential more general future eager-expansion API, if such API appears at some point, then this function can be easily deprecated and reimplemented in terms of that API, I think.

[petrochenkov]

@mystor
I'd like to have some second opinion before merging this.
Could you make a compiler team MCP from this PR, or a library team proposal, I'm not sure.

[m-ou-se]

We discussed this in the library-api meeting just now. We do like to see macro-expanding functionality on TokenStream. We do see the concerns with a general .expand() as already mentioned above, but would like to explore solutions that allow expanding to more than just literals in the near future. So we'd like to propose:

1. Rename this to not be specific to just literals and return a TokenStream instead of a Literal; and
2. Keep the implementation as restrictive as it is, failing if the result is not a single literal.

This way we sidestep the potential issues for now, but leave space for expanding the use cases of this function in the near future to include more expansions than only literals.

@mystor I'd like to hear your thoughts on this.

[joshtriplett]

One other thought from that discussion (not something that needs to be done in this PR, just a possibility for solving the more general case in the future):

Could we provide a way to expand all macros recursively, and then if any macro expands to code that uses any unstable features internally, we expand that particular macro to an Opaque object whose output can be included in the output stream but not introspected or modified? That way, you can expand something like format_args! if you're only going to consume its output, but if you want to introspect the output of something (e.g. walk the tokens from an included file) it needs to be stable.

[mystor]

We discussed this in the library-api meeting just now. We do like to see macro-expanding functionality on TokenStream. We do see the concerns with a general .expand() as already mentioned above, but would like to explore solutions that allow expanding to more than just literals in the near future. So we'd like to propose:

1. Rename this to not be specific to just literals and return a TokenStream instead of a Literal; and

2. Keep the implementation as restrictive as it is, failing if the result is not a single literal.

This way we sidestep the potential issues for now, but leave space for expanding the use cases of this function in the near future to include more expansions than only literals.

I'm not sure a general expand method would make sense, as macro expansion can be dependent on the context where it's occuring, and the TokenStream passed in needs to be parsed into an expression/type/whatever before expansion can start. This method could be generalized to expand_expr() rather than expand_literal(), but we'd probably also need expand_type(), expand_pat(), expand_items() etc. methods to cover the different types of macro expansions which are supported by rust. A general expand would probably need to either infer what is being expanded from how the macro being invoked is being expanded (though that feels sketchy to me, and I'd prefer not to do that), or would need to pick an expansion type like Expr and require wrapping other types in blocks or similar to parse items, types, or patterns.

If we do generalize to expand_expr(), I suppose we could also relax the requirements slightly to also allow true and false literal tokens now that they don't need to be valid Literals.

Could we provide a way to expand all macros recursively, and then if any macro expands to code that uses any unstable features internally, we expand that particular macro to an Opaque object whose output can be included in the output stream but not introspected or modified? That way, you can expand something like format_args! if you're only going to consume its output, but if you want to introspect the output of something (e.g. walk the tokens from an included file) it needs to be stable.

Having some way to represent an opaque sequence of tokens in the TokenStream would definitely be quite useful for this, but it would be quite a hassle to do that backwards compatibly at this point. The TokenTree enum is stable and only has the Ident, Literal, Punct, and Group options, so if we were to encode an opaque stream in, it would need to be done using one of those variants, as we can't add an Opaque variant.

The approach which seems most likely to me to not break existing macros would be to re-wrap the expanded AST as an unexpanded macro (e.g. opaque_expr!{}) with the specific expansion encoded into the span's context. This could theoretically be wrapped in a special Delimiter::None-delimited group with an as_opaque(&self) -> Option<Opaque> getter if we wanted to add special methods for operating on opaque expansions in the future.

Either way, this is probably something which should be handled in a separate issue or RFC.

[joshtriplett]

@mystor Thanks for clarifying the issue. We discussed this in today's @rust-lang/libs-api meeting, and we understand and agree that a fully general expand isn't an option. It sounds like it'd be reasonable to have an expand_expr method (that then can only return a literal for now). Assuming there's no blocker to doing so, we'd like to go that route.

[camelid]

The raw output appears to be "D:\\a\\rust\\rust\\src/test\\ui\\proc-macro\\expand-expr.rs", and post-processing it's: "D:\a\rust\rust\src/test/ui/proc-macro/expand-expr.rs", which has changed all of the \\ to / after the src/test but misses the ones before that, which are just collapsed to \ instead of \\ for some reason.

Maybe open an issue for that?

[mystor]

Maybe open an issue for that?

I'm not sure if it's a serious issue in general, because normally this would work fine. The replacement of paths to $DIR appears to happen before any platform normalization, and be quite aware that it's flakey:

rust/src/tools/compiletest/src/runtest.rs

Lines 3515 to 3518 in 220ed09

	/// Used to find Windows paths.
	///
	/// It's not possible to detect paths in the error messages generally, but this is a
	/// decent enough heuristic.

I think my particular case here of printing a string literal token and hoping that the path inside of it is normalized just isn't handled by compiletest and only worked on linux because it didn't contain any escaped characters.

[petrochenkov]

@bors r+

[bors]

📌 Commit 3e4d3d2 has been approved by petrochenkov

[bors]

⌛ Testing commit 3e4d3d2 with merge 3e018ce...

[bors]

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 3e018ce to master...

[rust-timer]

Finished benchmarking commit (3e018ce): comparison url.

Summary: This change led to very large relevant regressions 😿 in compiler performance.

• Very large regression in instruction counts (up to 5.3% on incr-unchanged builds of inflate)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression

[mystor]

Huh, I'm quite surprised this caused a perf regression... Only reason I can think of off the top of my head is because accessing sess on the proc macro server now requires a little bit more indirection:

rust/compiler/rustc_expand/src/proc_macro_server.rs

Lines 380 to 382 in 1b12d01

	fn sess(&self) -> &ParseSess {
	self.ecx.parse_sess()
	}

Should be fairly straightforward to reduce the interactions back down I suppose. We could add a field:

sess: &'b ParseSess,

and fetch it during the constructor before storing the &'a mut ExtCtxt<'b> into the type. We could also store the span_debug bool and the &'b dyn ResolverExpand if we're worried about either of those extra indirections as well.

I can't think of what else could be causing the regression of the top of my head, as the vast majority of the changes here should not be in any critical path.