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

Tracking Issue for proc_macro_expand #90765

Open
1 of 5 tasks
mystor opened this issue Nov 10, 2021 · 5 comments
Open
1 of 5 tasks

Tracking Issue for proc_macro_expand #90765

mystor opened this issue Nov 10, 2021 · 5 comments
Labels
A-proc-macros Area: Procedural macros C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. WG-macros Working group: Macros

Comments

@mystor
Copy link
Contributor

mystor commented Nov 10, 2021

Feature gate: #![feature(proc_macro_expand)]

This is a tracking issue for the TokenStream::expand_expr function for use in proc-macros. This function allows eagerly expanding TokenStreams containing expressions from within a proc-macro, such that they can be modified, giving proc-macros similar functionality to built-in macros like format_args!, concat! and include!.

Public API

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

Steps / History

Unresolved Questions

  • What types of expressions beyond literals should be supported?
  • Should expanding non-expression macros (e.g. types, items, etc.) be supported?
  • Should certain macros (e.g. those with #[allow_internal_unstable]) be expandable or left unexpanded?
  • How can we evolve which macros are expanded or not within the rustc stability model? Can downstream crates opt-in to additional macro expansion on nightly channels? Should an additional argument be provided to customize what should be expanded?
  • Should compilation be allowed to proceed after an expand method errors?
  • Are there concerns about stabilizing the span information generated by internal macros like include!?
  • How can we properly preserve span information when expanding expression macros?
@mystor mystor added C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Nov 10, 2021
@camelid camelid added the A-proc-macros Area: Procedural macros label Nov 10, 2021
@Kestrer
Copy link
Contributor

Kestrer commented Apr 26, 2022

FYI: allowing expand_expr to expand to any arbitrary expression would be unsound, because unsafe macros rely on their output not being tampered with for soundness. Take for example this innocuous-looking proc macro:

#[proc_macro]
pub fn expand_into(stream: TokenStream) -> TokenStream {
    let mut parts = stream.into_iter();
    let Some(TokenTree::Group(unexpanded)) = parts.next() else { panic!() };
    let into = parts.collect::<TokenStream>();
    let expanded = unexpanded.expand_expr().unwrap();
    quote!(#into!(#expanded)).into()
}

By combining it with a safe macro that emits unsafe like pin_utils::pin_mut, one can violate pinning guarantees:

macro_rules! helper {
    ({
        let $output:ident;
        let mut $x1:ident = $x2:ident;
        let mut $x3:ident = $e:expr;
    }) => {
        let mut $output = $e;
    };
}

let future = async {
    let value = 5;
    let reference = &value;
    yield_now().await;
    println!("{reference}");
};

expand_into!([{ let pinned; pin_mut!(future) }] helper);
// expands to: let pinned = unsafe { Pin::new_unchecked(&mut future) };
let _ = pinned.poll(cx);

let moved_future = future;
pin_mut!(moved_future);
moved_future.poll(cx); // oops!

So if the functionality of expand_expr were to be extended to any arbitrary expression, we would have to emit an error if the expansion emits any unsafe blocks. The method can also be supplemented with an expand_expr_unsafe if the user guarantees they won't change the macro's output in an unsound way. Alternatively, macros could somehow mark themselves as safe or unsafe to eagerly expand.

@danielhenrymantilla
Copy link
Contributor

danielhenrymantilla commented Apr 26, 2022

Or a simpler albeit more conceptual example:

//! crate a
#[macro_export]
macro_rules! fancy_block {() => (
    unsafe {} // needless but harmless unsafe: SOUND
)}
//! dependent, which uses no `unsafe` code:

fn main() {
    match! ::a::fancy_block!() {
        ( $some_ident:ident {} ) => (
            $some_ident {
                ::core::hint::unreachable_unchecked()
            }
        );
    }
}
  • (where I am using match! as the shorthand for eager expansion, but feel free to imagine it being a combination of expand_into + a helper should match! not be clear).

Nemo157 added a commit to Nullus157/stylish-rs that referenced this issue Jul 28, 2022
Nemo157 added a commit to Nullus157/stylish-rs that referenced this issue Jul 28, 2022
Nemo157 added a commit to Nullus157/stylish-rs that referenced this issue Jul 28, 2022
bors bot added a commit to Nullus157/stylish-rs that referenced this issue Jul 28, 2022
5: Add experimental cfg for using the proc-macro-expand API r=Nemo157 a=Nemo157

rust-lang/rust#90765

Co-authored-by: Wim Looman <git@nemo157.com>
@mystor
Copy link
Contributor Author

mystor commented Sep 4, 2022

I noticed an issue with this feature while working on the bridge, and opened a PR to fix it (#101414), so it should be fixed shortly.

Effectively, because expand_expr allows running proc macros recursively, it made the re-use of the same thread for multiple macro invocations more visible in a way which probably isn't desirable. The fix in that bug for now is to use a cross-thread executor for nested macro invocations.

@ilonachan
Copy link

ilonachan commented Sep 28, 2023

Just dropping in here, because this feature is precisely what I need for my own project. I didn't understand the problem outlined in the two examples at first, but I think if I had to sum it up it would be this: some macros expand to code snippets that really shouldn't be tampered with, and expand_expr would basically always allow doing that.

I'm not really a fan of the "emit an error if the expansion contains an unsafe block" idea: there are probably unsafe blocks that would be fine even when muddled around a bit (though the second example kinda calls that into question a bit), or there would be safe code that you might not want to be modified for semantic reasons. Heck, macros don't really need to emit valid Rust code at all! Especially if this general feature became a thing, chaining together macros that emit nonstandard syntax could be a cool pattern, and in those cases "checking for an unsafe block" doesn't even seem feasible anymore.

Instead of that, here's my wild idea: maybe there's a way to mark these un-tamperable macros as sealed or something. A proc-macro can expand them normally, but not tamper with their contents... the problem is that "tampering with the contents of a macro expansion" is very easy as long as you can "read the contents of a macro expansion": you can just clone the TokenStream and transform it, then replace the original tamper-proof expansion data in the output stream.

So that means when a sealed macro is expanded, that expansion is properly done and can be passed forward, and will appear in the final generated code (somehow)... but the proc-macro doing the expanding can't actually read its contents. Which is inconvenient, so maybe that's where an unsafe accessor could come in: when trying to read this stuff, the user has to intentionally opt-in and promise that they won't be changing the content at all, they'll just read it for informational purposes. Or something along those lines.

Now I have zero experience with the compiler development, so I don't really know how this could or should be implemented, but the idea I had was this:

  • Calling TokenStream::expand_expr() just emits all the usual tokens.
  • Then it encounters a macro call, and expands it. If that's a normal safe macro that you can play around with, the tokens are just dumped into the stream.
  • But what if it's a sealed macro? Then a dummy token of type SealedMacroExpansion could be put into the stream:
    • It stores the actual expansion in a way that someone consuming the TokenStream can't access it (unless they explicitly do the "I know what I'm doing" bit, or are the compiler writing the final resulting code into the tree)
    • But for the regular user it could also provide an alternate TokenStream which could be a basic replacement: for example, the unexpanded macro call. But maybe a proc-macro can arbitrarily decide what this will be: for example, it could itself expand the macros in its arguments, and have those expanded arguments in the SealedMacroExpansion for maximum information.
    • There could also be information about the actual proc-macro function or MBE rule that was used to expand the macro. Like a full absolute path, which I could see being useful for letting the compiler help a proc-macro identify macros more clearly.
  • And everything after continues as normal, until another macro to expand is found.

I don't know if putting "dummy tokens" into TokenStreams is a good idea, or even possible. But if it is, this seems like a very flexible system that expands as much as possible, while clearly communicating danger to a proc-macro author. And I think just this communication alone could solve some problems of the type outlined above:

  • In the first example, pin_mut! would probably be sealed. Which means that once its expansion (sealed) is passed into the MBE, it can't match the actual expansion, instead it'd just match the unexpanded call... or it wouldn't, because that rule doesn't exist. Even if it did though, I don't think this is any risk.
  • The second example would also be solved by marking fancy_block as sealed... so maybe it would actually be a good idea to enforce the whole "unsafe blocks should never be tampered with" rule when writing MBEs?

Sorry that this got long. Also I have no idea how anything works, so if this is complete nonsense feel free to disregard. I'm just really looking forward to this feature, because I think it'd really boost the capabilities of proc macros!

Edit: Doubly sorry for not realizing the discussion had moved to #87264. Because I think my SealedMacroExpansion idea is exactly the Opaque object described in this comment, and adding it to TokenStream would indeed be tricky.

@charles-r-earp
Copy link

It seems like this feature hasn't had much progress, and appears stalled on the more general cases.
Suppose a new feature #![proc_macro_expand_literals] is introduced, which only allows expanding macros which resolve to literals, as is currently implemented. This could then be stabilized once any potential concerns are resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-proc-macros Area: Procedural macros C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. WG-macros Working group: Macros
Projects
None yet
Development

No branches or pull requests

7 participants