Skip to content

Display an extra note for trailing semicolon lint with trailing macro #87381

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Jul 25, 2021

Conversation

Aaron1011
Copy link
Member

Currently, we parse macros at the end of a block
(e.g. fn foo() { my_macro!() }) as expressions, rather than
statements. This means that a macro invoked in this position
cannot expand to items or semicolon-terminated expressions.

In the future, we might want to start parsing these kinds of macros
as statements. This would make expansion more 'token-based'
(i.e. macro expansion behaves (almost) as if you just textually
replaced the macro invocation with its output). However,
this is a breaking change (see PR #78991), so it will require
further discussion.

Since the current behavior will not be changing any time soon,
we need to address the interaction with the
SEMICOLON_IN_EXPRESSIONS_FROM_MACROS lint. Since we are parsing
the result of macro expansion as an expression, we will emit a lint
if there's a trailing semicolon in the macro output. However, this
results in a somewhat confusing message for users, since it visually
looks like there should be no problem with having a semicolon
at the end of a block
(e.g. fn foo() { my_macro!() } => fn foo() { produced_expr; })

To help reduce confusion, this commit adds a note explaining
that the macro is being interpreted as an expression. Additionally,
we suggest adding a semicolon after the macro invocation - this
will cause us to parse the macro call as a statement. We do not
use a structured suggestion for this, since the user may actually
want to remove the semicolon from the macro definition (allowing
the block to evaluate to the expression produced by the macro).

@rust-highfive
Copy link
Contributor

r? @wesleywiser

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 22, 2021
@Aaron1011
Copy link
Member Author

r? @petrochenkov

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 23, 2021

📌 Commit 30112a821c5c4ddd60a5414ef570025576827184 has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 23, 2021
@bors
Copy link
Collaborator

bors commented Jul 24, 2021

☔ The latest upstream changes (presumably #87296) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 24, 2021
@Aaron1011 Aaron1011 force-pushed the note-semi-trailing-macro branch from 30112a8 to fc83fe4 Compare July 24, 2021 16:42
@Aaron1011
Copy link
Member Author

@bors r=petrochenkov

@bors
Copy link
Collaborator

bors commented Jul 24, 2021

📌 Commit fc83fe4094eab64fb7e77eccdd0afbb82ac183fa has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 24, 2021
@rust-log-analyzer

This comment has been minimized.

Currently, we parse macros at the end of a block
(e.g. `fn foo() { my_macro!() }`) as expressions, rather than
statements. This means that a macro invoked in this position
cannot expand to items or semicolon-terminated expressions.

In the future, we might want to start parsing these kinds of macros
as statements. This would make expansion more 'token-based'
(i.e. macro expansion behaves (almost) as if you just textually
replaced the macro invocation with its output). However,
this is a breaking change (see PR rust-lang#78991), so it will require
further discussion.

Since the current behavior will not be changing any time soon,
we need to address the interaction with the
`SEMICOLON_IN_EXPRESSIONS_FROM_MACROS` lint. Since we are parsing
the result of macro expansion as an expression, we will emit a lint
if there's a trailing semicolon in the macro output. However, this
results in a somewhat confusing message for users, since it visually
looks like there should be no problem with having a semicolon
at the end of a block
(e.g. `fn foo() { my_macro!() }` => `fn foo() { produced_expr; }`)

To help reduce confusion, this commit adds a note explaining
that the macro is being interpreted as an expression. Additionally,
we suggest adding a semicolon after the macro *invocation* - this
will cause us to parse the macro call as a statement. We do *not*
use a structured suggestion for this, since the user may actually
want to remove the semicolon from the macro definition (allowing
the block to evaluate to the expression produced by the macro).
@Aaron1011
Copy link
Member Author

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 24, 2021
@Aaron1011 Aaron1011 force-pushed the note-semi-trailing-macro branch from fc83fe4 to 0df5ac8 Compare July 24, 2021 16:53
@Aaron1011
Copy link
Member Author

@bors r=petrochenkov

@bors
Copy link
Collaborator

bors commented Jul 24, 2021

📌 Commit 0df5ac8 has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 24, 2021
@bors
Copy link
Collaborator

bors commented Jul 25, 2021

⌛ Testing commit 0df5ac8 with merge 71a6c7c...

@bors
Copy link
Collaborator

bors commented Jul 25, 2021

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 71a6c7c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 25, 2021
@bors bors merged commit 71a6c7c into rust-lang:master Jul 25, 2021
@rustbot rustbot added this to the 1.56.0 milestone Jul 25, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants