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

cannot end function with item-generating macro invoked with curly braces #34418

Closed
durka opened this issue Jun 23, 2016 · 9 comments
Closed

cannot end function with item-generating macro invoked with curly braces #34418

durka opened this issue Jun 23, 2016 · 9 comments

Comments

@durka
Copy link
Contributor

durka commented Jun 23, 2016

Consider the following macro and all the ways to invoke it:

macro_rules! make_item {
    ($a:ident) => { struct $a; }
}

make_item!(A); // ok (semicolon required)
make_item! { B } // ok (semicolon not required)

fn c() { make_item!(C); } // ok
fn d() { make_item!(D) } // bad (expected: missing semicolon)
fn e() { make_item! { E } } // bad (BUG: semicolon should not be required)
fn f() { make_item! { F }; } // ok (workaround)
fn gh() { make_item! { G } make_item!(H); } // ok (further evidence case E is a bug)

I believe that case E should be accepted, however it gives the error that the macro is invalid in expression position. My theory (before investigating) is that the tail of the function is considered as expression position because the macro expander forgets the possibility that a macro may generate items.

@durka durka changed the title cannot end function with item-genering macro invoked with curly braces cannot end function with item-generating macro invoked with curly braces Jun 23, 2016
@durka
Copy link
Contributor Author

durka commented Jun 23, 2016

I'm working on a fix.

@jseyfried
Copy link
Contributor

cc me

@durka
Copy link
Contributor Author

durka commented Jun 23, 2016

OK, a little harder to fix than we thought. By the time we get to expanding macros in a block, it's already an ast::Block which consists of statements and an optional expression. I think the decision of whether the last thing in a block is the last statement, or the expression, is purely syntactical -- whether it ends in a semicolon. We can change the expansion to be .fold_stmt() instead of .fold_expr(), but I believe that will break fn foo() -> Vec<i32> { vec! { 1, 2 } } because StmtKind::Expr must have unit type. So I don't know how to do what I want, which is:

  • try expanding the macro as an expression (somehow suppress diagnostics)
    • success: done
    • fail: try expanding the macro as a statement
      • success: add it to the end of the block's stmt list and turn the expr to None
      • fail: error

We could change expand_block to always expand the trailing expr as a stmt, and if it happens to expand to a StmtKind::Expr, then keep it as a trailing expr, otherwise move it to the stmt list. The docs for StmtKind say that StmtKind::Expr must have unit type (thus my assumption that this strategy would break the above Vec example), but it was pointed out that types aren't known at expansion time, so maybe it's okay to blatantly disregard this documented invariant. I won't really be able to try this until next week.

@jseyfried
Copy link
Contributor

@durka

We could change expand_block to always expand the trailing expr as a stmt, and if it happens to expand to a StmtKind::Expr, then keep it as a trailing expr, otherwise move it to the stmt list.

I like this idea.

@durka
Copy link
Contributor Author

durka commented Jun 23, 2016

Why does Block even have the Option<P<Expr>>, one wonders, when StmtKind::Expr exists?

@jseyfried
Copy link
Contributor

Good question...

@durka
Copy link
Contributor Author

durka commented Jun 23, 2016

fold_stmt can return a number of stmts, so really it's: expand the trailing expr as statements, if the last one is StmtKind::Expr then peel it off as the trailing expr, append the rest to the stmt list.

@jseyfried
Copy link
Contributor

Exactly!
Ideally we'll refactor away the Option<P<Expr>> from the block so it's all statements.

@jseyfried
Copy link
Contributor

I'm going to try to remove the field expr of Block in the next breaking batch, which should land in a day or two.

jseyfried added a commit to jseyfried/rust that referenced this issue Jun 26, 2016
To allow these braced macro invocation, this PR removes the optional expression from `ast::Block` and instead uses a `StmtKind::Expr` at the end of the statement list.

Currently, braced macro invocations in blocks can expand into statements (and items) except when they are last in a block, in which case they can only expand into expressions.

For example,
```rust
macro_rules! make_stmt {
    () => { let x = 0; }
}

fn f() {
    make_stmt! {} //< This is OK...
    let x = 0; //< ... unless this line is commented out.
}
```

Fixes rust-lang#34418.
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

No branches or pull requests

2 participants