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

syntax: unify all MacResult's into a single trait. #13527

Merged
merged 1 commit into from
Apr 16, 2014

Conversation

huonw
Copy link
Member

@huonw huonw commented Apr 15, 2014

There's now one unified way to return things from a macro, instead of
being able to choose the AnyMacro trait or the MRItem/MRExpr
variants of the MacResult enum. This does simplify the logic handling
the expansions, but the biggest value of this is it makes macros in (for
example) type position easier to implement, as there's this single thing
to modify.

By my measurements (using -Z time-passes on libstd and librustc etc.),
this appears to have little-to-no impact on expansion speed. There are
presumably larger costs than the small number of extra allocations and
virtual calls this adds (notably, all macro_rules!-defined macros have
not changed in behaviour, since they had to use the AnyMacro trait
anyway).


Summary of changes for dynamic syntax extension maintainers:

  • MacResult is now a trait, and is returned as ~MacResult
  • MRExpr & MRItem are now MacExpr::new and MacItem:new respectively (which return ~MacResults)
  • MacResult::dummy_... is DummyResult::any or DummyResult::expr

@alexcrichton alexcrichton assigned sfackler and unassigned sfackler Apr 16, 2014
e: @ast::Expr
}
impl MacExpr {
pub fn new(e: @ast::Expr) -> ~MacResult: {
Copy link
Member

Choose a reason for hiding this comment

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

The : shouldn't be necessary anymore now that there are no default bounds.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, right. I was just copying of the surrounding code.

@sfackler
Copy link
Member

Any reason to keep the explicit empty bounds? Otherwise, r=me.

There's now one unified way to return things from a macro, instead of
being able to choose the `AnyMacro` trait or the `MRItem`/`MRExpr`
variants of the `MacResult` enum. This does simplify the logic handling
the expansions, but the biggest value of this is it makes macros in (for
example) type position easier to implement, as there's this single thing
to modify.

By my measurements (using `-Z time-passes` on libstd and librustc etc.),
this appears to have little-to-no impact on expansion speed. There are
presumably larger costs than the small number of extra allocations and
virtual calls this adds (notably, all `macro_rules!`-defined macros have
not changed in behaviour, since they had to use the `AnyMacro` trait
anyway).
bors added a commit that referenced this pull request Apr 16, 2014
There's now one unified way to return things from a macro, instead of
being able to choose the `AnyMacro` trait or the `MRItem`/`MRExpr`
variants of the `MacResult` enum. This does simplify the logic handling
the expansions, but the biggest value of this is it makes macros in (for
example) type position easier to implement, as there's this single thing
to modify.

By my measurements (using `-Z time-passes` on libstd and librustc etc.),
this appears to have little-to-no impact on expansion speed. There are
presumably larger costs than the small number of extra allocations and
virtual calls this adds (notably, all `macro_rules!`-defined macros have
not changed in behaviour, since they had to use the `AnyMacro` trait
anyway).

---

Summary of changes for dynamic syntax extension maintainers:

- `MacResult` is now a trait, and is returned as `~MacResult`
- `MRExpr` & `MRItem` are now `MacExpr::new` and `MacItem:new` respectively (which return `~MacResult`s)
- `MacResult::dummy_...` is `DummyResult::any` or `DummyResult::expr`
@bors bors closed this Apr 16, 2014
@bors bors merged commit 99dd591 into rust-lang:master Apr 16, 2014
@huonw huonw deleted the macro-expander-trait branch June 27, 2014 06:47
notriddle pushed a commit to notriddle/rust that referenced this pull request Nov 10, 2022
…o-guarded-return-assist, r=jonas-schievink

Use let-else statements in `Convert to guarded return` assist

Follow up for rust-lang/rust-analyzer#13516, addresses remaining part of rust-lang/rust-analyzer#13254 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants