Accept `m!{ .. }.method()` and `m!{ .. }?` statements. #88690

m-ou-se · 2021-09-06T16:26:05Z

This PR fixes something that I keep running into when using quote!{}.into() in a proc macro to convert the proc_macro2::TokenStream to a proc_macro::TokenStream:

Before:

error: expected expression, found `.`
 --> src/lib.rs:6:6
  |
4 |     quote! {
5 |         ...
6 |     }.into()
  |      ^ expected expression

After:

(No output, compiles fine.)

Context:

For expressions like { 1 } and if true { 1 } else { 2 }, we accept them as full statements without a trailing ;, which means the following is not accepted:

{ 1 } - 1 // error

since that is parsed as two statements: { 1 } and -1. Syntactically correct, but the type of { 1 } should be () as there is no ;.

However, for specifically . and ? after the }, we do continue parsing it as an expression:

{ "abc" }.len(); // ok

For braced macro invocations, we do not do this:

vec![1, 2, 3].len(); // ok
vec!{1, 2, 3}.len(); // error

(It parses vec!{1, 2, 3} as a full statement, and then complains about .len() not being a valid expression.)

This PR changes this to also look for a . and ? after a braced macro invocation. We can be sure the macro is an expression and not a full statement in those cases, since no statement can start with a . or ?.

The text was updated successfully, but these errors were encountered:

rust-highfive · 2021-09-06T16:26:09Z

r? @nagisa

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

m-ou-se · 2021-09-06T16:29:01Z

@rust-lang/lang I suppose this requires an FCP?

joshtriplett · 2021-09-06T20:30:42Z

I think, before approving something like this, we'd want to know if this restriction is important for any particular reason (e.g. interactions with parsing and follow sets). I don't think it would be, given the examples you gave, but it'd be helpful to have some guidance from someone with clear knowledge of how macros interact with the parser.

For my part, assuming it wouldn't break anything, I think this seems reasonable.

petrochenkov · 2021-09-08T11:50:29Z

I think this is the right thing to do and don't see any reasons for braced macros m! { ... } to be different from blocks { ... } in this position.
This shouldn't break anything since we are only making previously illegal code legal, and I don't see issues with expansion either.

cc @Aaron1011 just in case

nikomatsakis · 2021-09-08T14:38:38Z

+1 from me, I agree that this seems more consistent.

nagisa

I'm okay with this change on the implementation side of things.

On the language design side of things I feel like with this adjustment we end up neither here nor there for this syntax element appears to be treated in a fairly inconsistent manner compared to the rest of Rust's syntactic constructs. That is it now acts kind of like an expression but only in very specific contexts. I think it would be a good idea to go all the way to making it act like all the other ExpressionWithBlocks.

compiler/rustc_parse/src/parser/stmt.rs

src/test/ui/parser/macro-braces-dot-question.rs

scottmcm · 2021-09-10T20:06:57Z

+1 from me as well for making it consistent. Given that things like

let x = loop { break 7_u32 }.count_ones();

work, then those same things working after macro_rules with braces seems like the right choice.

nagisa · 2021-09-12T19:50:12Z

r=me with a negative test added. At this point a FCP seems redundant given that we already had +1 from 3/5 T-lang members.

m-ou-se · 2021-09-13T15:57:33Z

@bors r= nagisa

bors · 2021-09-13T15:57:35Z

📌 Commit 7d8d7a0 has been approved by ``

m-ou-se · 2021-09-13T15:57:49Z

@bors r-

m-ou-se · 2021-09-13T15:57:53Z

@bors r=nagisa

bors · 2021-09-13T15:57:54Z

📌 Commit 7d8d7a0 has been approved by nagisa

…xpr-parse, r=nagisa Accept `m!{ .. }.method()` and `m!{ .. }?` statements. This PR fixes something that I keep running into when using `quote!{}.into()` in a proc macro to convert the `proc_macro2::TokenStream` to a `proc_macro::TokenStream`: Before: ``` error: expected expression, found `.` --> src/lib.rs:6:6 | 4 | quote! { 5 | ... 6 | }.into() | ^ expected expression ``` After: ``` ``` (No output, compiles fine.) --- Context: For expressions like `{ 1 }` and `if true { 1 } else { 2 }`, we accept them as full statements without a trailing `;`, which means the following is not accepted: ```rust { 1 } - 1 // error ``` since that is parsed as two statements: `{ 1 }` and `-1`. Syntactically correct, but the type of `{ 1 }` should be `()` as there is no `;`. However, for specifically `.` and `?` after the `}`, we do [continue parsing it as an expression](https://github.com/rust-lang/rust/blob/13db8440bbbe42870bc828d4ec3e965b38670277/compiler/rustc_parse/src/parser/expr.rs#L864-L876): ```rust { "abc" }.len(); // ok ``` For braced macro invocations, we do not do this: ```rust vec![1, 2, 3].len(); // ok vec!{1, 2, 3}.len(); // error ``` (It parses `vec!{1, 2, 3}` as a full statement, and then complains about `.len()` not being a valid expression.) This PR changes this to also look for a `.` and `?` after a braced macro invocation. We can be sure the macro is an expression and not a full statement in those cases, since no statement can start with a `.` or `?`.

…arth Rollup of 9 pull requests Successful merges: - rust-lang#87320 (Introduce -Z remap-cwd-prefix switch) - rust-lang#88619 (Remove `cfg(doc)` from std::os module reexports to fix rustdoc linking issues) - rust-lang#88690 (Accept `m!{ .. }.method()` and `m!{ .. }?` statements. ) - rust-lang#88775 (Revert anon union parsing) - rust-lang#88781 (Tokenize emoji as if they were valid identifiers ) - rust-lang#88841 (feat(rustc_typeck): suggest removing bad parens in `(recv.method)()`) - rust-lang#88892 (Move object safety suggestions to the end of the error) - rust-lang#88915 (`Wrapping<T>` has the same layout and ABI as `T`) - rust-lang#88933 (Remove implementation of `min_align_of` intrinsic) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…xpr-parse, r=nagisa Accept `m!{ .. }.method()` and `m!{ .. }?` statements. This PR fixes something that I keep running into when using `quote!{}.into()` in a proc macro to convert the `proc_macro2::TokenStream` to a `proc_macro::TokenStream`: Before: ``` error: expected expression, found `.` --> src/lib.rs:6:6 | 4 | quote! { 5 | ... 6 | }.into() | ^ expected expression ``` After: ``` ``` (No output, compiles fine.) --- Context: For expressions like `{ 1 }` and `if true { 1 } else { 2 }`, we accept them as full statements without a trailing `;`, which means the following is not accepted: ```rust { 1 } - 1 // error ``` since that is parsed as two statements: `{ 1 }` and `-1`. Syntactically correct, but the type of `{ 1 }` should be `()` as there is no `;`. However, for specifically `.` and `?` after the `}`, we do [continue parsing it as an expression](https://github.com/rust-lang/rust/blob/13db8440bbbe42870bc828d4ec3e965b38670277/compiler/rustc_parse/src/parser/expr.rs#L864-L876): ```rust { "abc" }.len(); // ok ``` For braced macro invocations, we do not do this: ```rust vec![1, 2, 3].len(); // ok vec!{1, 2, 3}.len(); // error ``` (It parses `vec!{1, 2, 3}` as a full statement, and then complains about `.len()` not being a valid expression.) This PR changes this to also look for a `.` and `?` after a braced macro invocation. We can be sure the macro is an expression and not a full statement in those cases, since no statement can start with a `.` or `?`.

…arth Rollup of 8 pull requests Successful merges: - rust-lang#87320 (Introduce -Z remap-cwd-prefix switch) - rust-lang#88619 (Remove `cfg(doc)` from std::os module reexports to fix rustdoc linking issues) - rust-lang#88690 (Accept `m!{ .. }.method()` and `m!{ .. }?` statements. ) - rust-lang#88775 (Revert anon union parsing) - rust-lang#88781 (Tokenize emoji as if they were valid identifiers ) - rust-lang#88841 (feat(rustc_typeck): suggest removing bad parens in `(recv.method)()`) - rust-lang#88915 (`Wrapping<T>` has the same layout and ABI as `T`) - rust-lang#88933 (Remove implementation of `min_align_of` intrinsic) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…xpr-parse, r=nagisa Accept `m!{ .. }.method()` and `m!{ .. }?` statements. This PR fixes something that I keep running into when using `quote!{}.into()` in a proc macro to convert the `proc_macro2::TokenStream` to a `proc_macro::TokenStream`: Before: ``` error: expected expression, found `.` --> src/lib.rs:6:6 | 4 | quote! { 5 | ... 6 | }.into() | ^ expected expression ``` After: ``` ``` (No output, compiles fine.) --- Context: For expressions like `{ 1 }` and `if true { 1 } else { 2 }`, we accept them as full statements without a trailing `;`, which means the following is not accepted: ```rust { 1 } - 1 // error ``` since that is parsed as two statements: `{ 1 }` and `-1`. Syntactically correct, but the type of `{ 1 }` should be `()` as there is no `;`. However, for specifically `.` and `?` after the `}`, we do [continue parsing it as an expression](https://github.com/rust-lang/rust/blob/13db8440bbbe42870bc828d4ec3e965b38670277/compiler/rustc_parse/src/parser/expr.rs#L864-L876): ```rust { "abc" }.len(); // ok ``` For braced macro invocations, we do not do this: ```rust vec![1, 2, 3].len(); // ok vec!{1, 2, 3}.len(); // error ``` (It parses `vec!{1, 2, 3}` as a full statement, and then complains about `.len()` not being a valid expression.) This PR changes this to also look for a `.` and `?` after a braced macro invocation. We can be sure the macro is an expression and not a full statement in those cases, since no statement can start with a `.` or `?`.

…arth Rollup of 8 pull requests Successful merges: - rust-lang#87320 (Introduce -Z remap-cwd-prefix switch) - rust-lang#88619 (Remove `cfg(doc)` from std::os module reexports to fix rustdoc linking issues) - rust-lang#88690 (Accept `m!{ .. }.method()` and `m!{ .. }?` statements. ) - rust-lang#88775 (Revert anon union parsing) - rust-lang#88841 (feat(rustc_typeck): suggest removing bad parens in `(recv.method)()`) - rust-lang#88907 (Highlight the `const fn` if error happened because of a bound on the impl block) - rust-lang#88915 (`Wrapping<T>` has the same layout and ABI as `T`) - rust-lang#88933 (Remove implementation of `min_align_of` intrinsic) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…xpr-parse, r=nagisa Accept `m!{ .. }.method()` and `m!{ .. }?` statements. This PR fixes something that I keep running into when using `quote!{}.into()` in a proc macro to convert the `proc_macro2::TokenStream` to a `proc_macro::TokenStream`: Before: ``` error: expected expression, found `.` --> src/lib.rs:6:6 | 4 | quote! { 5 | ... 6 | }.into() | ^ expected expression ``` After: ``` ``` (No output, compiles fine.) --- Context: For expressions like `{ 1 }` and `if true { 1 } else { 2 }`, we accept them as full statements without a trailing `;`, which means the following is not accepted: ```rust { 1 } - 1 // error ``` since that is parsed as two statements: `{ 1 }` and `-1`. Syntactically correct, but the type of `{ 1 }` should be `()` as there is no `;`. However, for specifically `.` and `?` after the `}`, we do [continue parsing it as an expression](https://github.com/rust-lang/rust/blob/13db8440bbbe42870bc828d4ec3e965b38670277/compiler/rustc_parse/src/parser/expr.rs#L864-L876): ```rust { "abc" }.len(); // ok ``` For braced macro invocations, we do not do this: ```rust vec![1, 2, 3].len(); // ok vec!{1, 2, 3}.len(); // error ``` (It parses `vec!{1, 2, 3}` as a full statement, and then complains about `.len()` not being a valid expression.) This PR changes this to also look for a `.` and `?` after a braced macro invocation. We can be sure the macro is an expression and not a full statement in those cases, since no statement can start with a `.` or `?`.

…arth Rollup of 7 pull requests Successful merges: - rust-lang#88690 (Accept `m!{ .. }.method()` and `m!{ .. }?` statements. ) - rust-lang#88775 (Revert anon union parsing) - rust-lang#88841 (feat(rustc_typeck): suggest removing bad parens in `(recv.method)()`) - rust-lang#88907 (Highlight the `const fn` if error happened because of a bound on the impl block) - rust-lang#88915 (`Wrapping<T>` has the same layout and ABI as `T`) - rust-lang#88933 (Remove implementation of `min_align_of` intrinsic) - rust-lang#88951 (Update books) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…arth Rollup of 8 pull requests Successful merges: - rust-lang#87320 (Introduce -Z remap-cwd-prefix switch) - rust-lang#88690 (Accept `m!{ .. }.method()` and `m!{ .. }?` statements. ) - rust-lang#88775 (Revert anon union parsing) - rust-lang#88841 (feat(rustc_typeck): suggest removing bad parens in `(recv.method)()`) - rust-lang#88907 (Highlight the `const fn` if error happened because of a bound on the impl block) - rust-lang#88915 (`Wrapping<T>` has the same layout and ABI as `T`) - rust-lang#88933 (Remove implementation of `min_align_of` intrinsic) - rust-lang#88951 (Update books) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

m-ou-se added 2 commits Sep 6, 2021

Accept `m!{ .. }.method()` and `m!{ .. }?` statements.

3caf0bc

Add test for braced-macro followed by `.` or `?`.

ebf1252

rust-highfive assigned nagisa Sep 6, 2021

rust-highfive added the S-waiting-on-review label Sep 6, 2021

m-ou-se added A-macros A-parser needs-fcp T-lang and removed S-waiting-on-review labels Sep 6, 2021

m-ou-se changed the title ~~Macro braces dot question expr parse~~ Accept m!{ .. }.method() and m!{ .. }? statements. Sep 6, 2021

petrochenkov self-assigned this Sep 6, 2021

petrochenkov added the S-waiting-on-review label Sep 6, 2021

petrochenkov removed their assignment Sep 8, 2021

nagisa approved these changes Sep 10, 2021

View changes

compiler/rustc_parse/src/parser/stmt.rs Show resolved Hide resolved

src/test/ui/parser/macro-braces-dot-question.rs Show resolved Hide resolved

m-ou-se added relnotes and removed needs-fcp labels Sep 13, 2021

nagisa added S-waiting-on-author and removed S-waiting-on-review labels Sep 13, 2021

Add negative test in macro-braces-dot-question.rs.

7d8d7a0

bors added S-waiting-on-bors and removed S-waiting-on-author labels Sep 13, 2021

bors added S-waiting-on-author and removed S-waiting-on-bors labels Sep 13, 2021

bors added S-waiting-on-bors and removed S-waiting-on-author labels Sep 13, 2021

Manishearth mentioned this pull request Sep 14, 2021

Rollup of 8 pull requests #88939

Closed

Manishearth mentioned this pull request Sep 14, 2021

Rollup of 9 pull requests #88940

Closed

Manishearth mentioned this pull request Sep 14, 2021

Rollup of 8 pull requests #88943

Closed

Manishearth mentioned this pull request Sep 14, 2021

Rollup of 8 pull requests #88948

Closed

Manishearth mentioned this pull request Sep 15, 2021

Rollup of 7 pull requests #88958

Closed

Manishearth mentioned this pull request Sep 15, 2021

Rollup of 8 pull requests #88992

Merged

bors merged commit 4b56840 into rust-lang:master Sep 16, 2021
10 checks passed

rustbot added this to the 1.57.0 milestone Sep 16, 2021

ehuss mentioned this pull request Sep 16, 2021

No-brace restrictions should mention that they are recursive. rust-lang/reference#569

Open

m-ou-se deleted the macro-braces-dot-question-expr-parse branch Sep 17, 2021

rust-lang / rust Public

Accept `m!{ .. }.method()` and `m!{ .. }?` statements. #88690

Accept `m!{ .. }.method()` and `m!{ .. }?` statements. #88690

m-ou-se commented Sep 6, 2021 •

edited

rust-highfive commented Sep 6, 2021

m-ou-se commented Sep 6, 2021

joshtriplett commented Sep 6, 2021

petrochenkov commented Sep 8, 2021

nikomatsakis commented Sep 8, 2021

nagisa left a comment

scottmcm commented Sep 10, 2021

nagisa commented Sep 12, 2021 •

edited

m-ou-se commented Sep 13, 2021

bors commented Sep 13, 2021

m-ou-se commented Sep 13, 2021

m-ou-se commented Sep 13, 2021

bors commented Sep 13, 2021

rust-lang / rust Public

Accept m!{ .. }.method() and m!{ .. }? statements. #88690

Accept m!{ .. }.method() and m!{ .. }? statements. #88690

Conversation

m-ou-se commented Sep 6, 2021 • edited

rust-highfive commented Sep 6, 2021

m-ou-se commented Sep 6, 2021

joshtriplett commented Sep 6, 2021

petrochenkov commented Sep 8, 2021

nikomatsakis commented Sep 8, 2021

nagisa left a comment

scottmcm commented Sep 10, 2021

nagisa commented Sep 12, 2021 • edited

m-ou-se commented Sep 13, 2021

bors commented Sep 13, 2021

m-ou-se commented Sep 13, 2021

m-ou-se commented Sep 13, 2021

bors commented Sep 13, 2021

Accept `m!{ .. }.method()` and `m!{ .. }?` statements. #88690

Accept `m!{ .. }.method()` and `m!{ .. }?` statements. #88690

m-ou-se commented Sep 6, 2021 •

edited

nagisa commented Sep 12, 2021 •

edited