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

[DO NOT MERGE] Consistent handling of semicolons in macro expansions #78685

Closed
wants to merge 7 commits into from

Conversation

Aaron1011
Copy link
Member

Implements 'Proposal 2' from #61733 (comment)

This PR treats semicolon-less macro invocations like statements. This allows the following code to compile:

fn main() {
    macro_rules! a {
        ($e:expr) => { $e; }
    }
    a!(true)
}

To avoid making macro behavior more inconsistent, macros expanded in expression position no longer ignore trailing semicolons in the expansion. This broke several things in the standard library - in fact, we can't even get rustfmt to pass until rust-lang/rustfmt#4507 is merged

Given all of the issues this has expoesd with the standard library (and many test cases), I expect this to cause a significant amount of ecosystem breakage. I'm opening this for a Crater run - we'll almost certainly need to do a gradual rollout if this is accepted. Fortunately, it's easy to convert the macro_rules! error into a warning.

@rust-highfive
Copy link
Collaborator

r? @varkor

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

  • These commits modify submodules.

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

@bors try

@Aaron1011
Copy link
Member Author

r? @ghost

@bors
Copy link
Contributor

bors commented Nov 2, 2020

⌛ Trying commit 6522b7a82e35fe64fa731578724e8e861c5dbd84 with merge 1a1b32118e83a86f430f0c6665d870751ca08118...

@rust-log-analyzer

This comment has been minimized.

@bors

This comment has been minimized.

@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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 2, 2020
@Aaron1011
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Nov 2, 2020

⌛ Trying commit 2f6d12cf5b3469a4b1cd57098ae1366f3fe47f16 with merge ea2a67936e3f9b71d6067d67723b66bac8d387b4...

@jyn514 jyn514 added A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 2, 2020
@bors

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@Aaron1011
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Nov 2, 2020

⌛ Trying commit 003157547b12ef2b81db5aaa223070614b3a4327 with merge bc19ff65d7d862e3bca14037b3c0cb818328cfcc...

@@ -125,7 +125,7 @@ pub(super) fn run_utf8_validation(v: &[u8]) -> Result<(), Utf8Error> {
let old_offset = index;
macro_rules! err {
($error_len: expr) => {
return Err(Utf8Error { valid_up_to: old_offset, error_len: $error_len });
return Err(Utf8Error { valid_up_to: old_offset, error_len: $error_len })
Copy link

Choose a reason for hiding this comment

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

Suggested change
return Err(Utf8Error { valid_up_to: old_offset, error_len: $error_len })
Err(Utf8Error { valid_up_to: old_offset, error_len: $error_len })

Is return required here?

Copy link
Member Author

Choose a reason for hiding this comment

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

@rust-log-analyzer

This comment has been minimized.

@bors

This comment has been minimized.

@Aaron1011
Copy link
Member Author

The build doesn't even complete because of anyhow:

https://github.com/dtolnay/anyhow/blob/c25be95f1a24f7497d2b4530ffcb4f90d3871975/src/macros.rs#L54-L64

Opened dtolnay/anyhow#120. The fact that three different crates (libstd, stdarch, and anyhow) have all been broken by this change doesn't seem to bode well for the Crater results.

cc rust-lang#33953

Currently, the following code produces an error

```rust
fn main() {
    macro_rules! a {
        ($e:expr) => { $e; }
    }
    a!(true)
}
```
@Aaron1011 Aaron1011 force-pushed the feature/stmt-bang-macro branch from 0031575 to 677d6cb Compare November 10, 2020 23:02
@Aaron1011
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Nov 10, 2020

⌛ Trying commit 677d6cb with merge 1cd838451943835da5228ec71c7bae5b8268efdd...

@rust-log-analyzer
Copy link
Collaborator

The job dist-x86_64-linux of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
##[debug]Evaluating job defaults
Prepare all required actions
Getting action download info
Download action repository 'actions/checkout@v2'
##[debug]Download 'https://api.github.com/repos/actions/checkout/tarball/5a4ac9002d0be2fb38bd78e4b4dbde5606d7042f' to '/home/runner/work/_actions/_temp_f1546009-49e5-4d74-88f0-3707a744473a/a70b31de-6dfe-49c9-897c-8e9f09119fac.tar.gz'
##[debug]Unwrap 'actions-checkout-5a4ac90' to '/home/runner/work/_actions/actions/checkout/v2'
##[debug]Archive '/home/runner/work/_actions/_temp_f1546009-49e5-4d74-88f0-3707a744473a/a70b31de-6dfe-49c9-897c-8e9f09119fac.tar.gz' has been unzipped into '/home/runner/work/_actions/actions/checkout/v2'.
Download action repository 'rust-lang/simpleinfra@master'
##[debug]Download 'https://api.github.com/repos/rust-lang/simpleinfra/tarball/caeabfd7dcd0342420d12a285e4a50b0a3ae2cc8' to '/home/runner/work/_actions/_temp_b2a0a2fb-e406-4c30-b766-9e7f68e73516/e702b4f1-4660-49cf-be72-20c2a78e698c.tar.gz'
##[debug]Unwrap 'rust-lang-simpleinfra-caeabfd' to '/home/runner/work/_actions/rust-lang/simpleinfra/master'
##[debug]Archive '/home/runner/work/_actions/_temp_b2a0a2fb-e406-4c30-b766-9e7f68e73516/e702b4f1-4660-49cf-be72-20c2a78e698c.tar.gz' has been unzipped into '/home/runner/work/_actions/rust-lang/simpleinfra/master'.
##[debug]action.yml for action: '/home/runner/work/_actions/rust-lang/simpleinfra/master/github-actions/cancel-outdated-builds/action.yml'.
##[debug]Set step 'run1' display name to: 'disable git crlf conversion'
##[debug]Set step 'actionscheckout' display name to: 'checkout the source code'
##[debug]Set step 'run2' display name to: 'configure the PR in which the error message will be posted'
---
::endgroup::
##[endgroup]
::group::Fetching the repository
##[group]Fetching the repository
[command]/usr/bin/git -c protocol.version=2 fetch --no-tags --prune --progress --no-recurse-submodules --depth=2 origin +1cd838451943835da5228ec71c7bae5b8268efdd:refs/remotes/origin/try
---
   Compiling memchr v2.3.3
   Compiling lazy_static v1.4.0
   Compiling libc v0.2.79
   Compiling serde_derive v1.0.115
   Compiling log v0.4.11 (https://github.com/Aaron1011/log?branch=fix/semi#ed7a622a)
   Compiling ryu v1.0.5
   Compiling regex-syntax v0.6.18
   Compiling cfg-if v0.1.10
   Compiling cfg-if v1.0.0
---
   Compiling serde v1.0.115
   Compiling cfg-if v0.1.10
   Compiling ppv-lite86 v0.2.8
   Compiling siphasher v0.3.3
   Compiling log v0.4.11 (https://github.com/Aaron1011/log?branch=fix/semi#ed7a622a)
   Compiling cfg-if v1.0.0
   Compiling itoa v0.4.6
   Compiling mac v0.1.1
   Compiling precomputed-hash v0.1.1
---
   Compiling syn v1.0.38
   Compiling cc v1.0.60
   Compiling scopeguard v1.1.0
   Compiling smallvec v1.4.2
   Compiling log v0.4.11 (https://github.com/Aaron1011/log?branch=fix/semi#ed7a622a)
   Compiling instant v0.1.6
   Compiling typenum v1.12.0
   Compiling version_check v0.9.1
   Compiling hashbrown v0.9.0
---
   Compiling syn v1.0.38
   Compiling cc v1.0.60
   Compiling scopeguard v1.1.0
   Compiling smallvec v1.4.2
   Compiling log v0.4.11 (https://github.com/Aaron1011/log?branch=fix/semi#ed7a622a)
   Compiling instant v0.1.6
   Compiling typenum v1.12.0
   Compiling version_check v0.9.1
   Compiling getrandom v0.1.14
---
Documenting stage2 std (x86_64-unknown-linux-gnu)
Uplifting stage1 std (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
Copying stage2 std from stage1 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
 Documenting core v0.0.0 (/checkout/library/core)
warning: unresolved link to `2:0`
    --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:2065:44
     |
2065 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
     |                                            ^^^ no item named `2:0` in scope
     |
     = note: `#[warn(broken_intra_doc_links)]` on by default
     = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
    --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:2094:44
     |
2094 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
     |                                            ^^^ no item named `2:0` in scope
     |
     = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
    --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:2123:44
     |
2123 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
     |                                            ^^^ no item named `2:0` in scope
     |
     = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
    --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:2152:44
     |
2152 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
     |                                            ^^^ no item named `2:0` in scope
     |
     = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
    --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:2181:44
     |
2181 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
     |                                            ^^^ no item named `2:0` in scope
     |
     = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
    --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:2215:44
     |
2215 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
     |                                            ^^^ no item named `2:0` in scope
     |
     = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
    --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:5512:44
     |
5512 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
     |                                            ^^^ no item named `2:0` in scope
     |
     = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
    --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:5542:44
     |
5542 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
     |                                            ^^^ no item named `2:0` in scope
     |
     = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
    --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:5572:44
     |
5572 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
     |                                            ^^^ no item named `2:0` in scope
     |
     = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
    --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:5607:44
     |
5607 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
     |                                            ^^^ no item named `2:0` in scope
     |
     = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
    --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:5637:44
     |
5637 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
     |                                            ^^^ no item named `2:0` in scope
     |
     = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
    --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:5667:44
     |
5667 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
     |                                            ^^^ no item named `2:0` in scope
     |
     = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
     --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:19448:44
      |
19448 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
      |                                            ^^^ no item named `2:0` in scope
      |
      = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
     --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:19478:44
      |
19478 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
      |                                            ^^^ no item named `2:0` in scope
      |
      = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
     --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:19514:44
      |
19514 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
      |                                            ^^^ no item named `2:0` in scope
      |
      = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
     --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:19544:44
      |
19544 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
      |                                            ^^^ no item named `2:0` in scope
      |
      = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
     --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:19574:44
      |
19574 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
      |                                            ^^^ no item named `2:0` in scope
      |
      = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
     --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:19610:44
      |
19610 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
      |                                            ^^^ no item named `2:0` in scope
      |
      = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
     --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:21403:44
      |
21403 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
      |                                            ^^^ no item named `2:0` in scope
      |
      = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
     --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:21434:44
      |
21434 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
      |                                            ^^^ no item named `2:0` in scope
      |
      = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
     --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:21465:44
      |
21465 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
      |                                            ^^^ no item named `2:0` in scope
      |
      = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
     --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:21502:44
      |
21502 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
      |                                            ^^^ no item named `2:0` in scope
      |
      = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
     --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:21533:44
      |
21533 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
      |                                            ^^^ no item named `2:0` in scope
      |
      = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
     --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:21564:44
      |
21564 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
      |                                            ^^^ no item named `2:0` in scope
      |
      = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `2:0`
warning: unresolved link to `2:0`
    --> library/core/src/../../stdarch/crates/core_arch/src/x86/avx512f.rs:2065:44
     |
2065 | /// Rounding is done according to the imm8[2:0] parameter, which can be one of:
     |                                            ^^^ no item named `2:0` in scope
     |
     = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: 25 warnings emitted

    Finished release [optimized + debuginfo] target(s) in 13.26s
   Compiling cc v1.0.60
---
    Checking smallvec v1.4.2
   Compiling unicode-xid v0.2.1
    Checking scopeguard v1.1.0
   Compiling cc v1.0.60
   Compiling log v0.4.11 (https://github.com/Aaron1011/log?branch=fix/semi#ed7a622a)
    Checking hashbrown v0.9.0
    Checking instant v0.1.6
   Compiling typenum v1.12.0
   Compiling getrandom v0.1.14

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @rust-lang/infra. (Feature Requests)

@bors
Copy link
Contributor

bors commented Nov 10, 2020

💔 Test failed - checks-actions

@petrochenkov
Copy link
Contributor

r? @petrochenkov

@bors
Copy link
Contributor

bors commented Nov 18, 2020

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

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@crlf0710 crlf0710 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 4, 2020
@crlf0710 crlf0710 removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 29, 2021
@petrochenkov petrochenkov added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 29, 2021
@JohnCSimon JohnCSimon 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 14, 2021
@petrochenkov
Copy link
Contributor

Closing due to inactivity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants