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

Rustc hangs on recursive macro expansion on 1.48.0 #79242

Closed
bobbobbio opened this issue Nov 20, 2020 · 5 comments · Fixed by #79338
Closed

Rustc hangs on recursive macro expansion on 1.48.0 #79242

bobbobbio opened this issue Nov 20, 2020 · 5 comments · Fixed by #79338
Assignees
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example I-compiletime Issue: Problems and improvements with respect to compile times. P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@bobbobbio
Copy link

Code

We have a library that compiles fine on 1.47.0 but after uggrading to 1.48.0 the compilation hangs.

#![allow(unused)]

macro_rules! declare_nats {
    ($prev:ty) => {};
    ($prev:ty, $n:literal$(, $tail:literal)*) => {
        paste::item! {
            pub struct [<N $n>]($prev);
        }
        declare_nats!(Option<$prev>$(, $tail)*);
    };
    (0, $($n:literal),+) => {
        pub struct N0;
        declare_nats!(N0, $($n),+);
    };
}

declare_nats! {
    0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
    16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
    32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47,
    48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63,
    64
}

fn main() {}

This code-snippet above is reduced from the original code as much as I can. I'm having problems understanding exactly where the problem is coming from and any further modifications to reduce it haven't worked. It uses the "paste" crate, and I've tried a few different versions including the latest one but it doesn't make a difference .

I've been building it with cargo build at 1.47.0 and 1.48.0 to reproduce the issue which I got just by running rustup default

doesn't work on

rustc 1.48.0 (7eac88abb 2020-11-16)
binary: rustc
commit-hash: 7eac88abb2e57e752f3302f02be5f3ce3d7adfb4
commit-date: 2020-11-16
host: x86_64-unknown-linux-gnu
release: 1.48.0
LLVM version: 11.0

works on

rustc 1.47.0 (18bf6b4f0 2020-10-07)
binary: rustc
commit-hash: 18bf6b4f01a6feaf7259ba7cdae58031af1b7b39
commit-date: 2020-10-07
host: x86_64-unknown-linux-gnu
release: 1.47.0
LLVM version: 11.0

@Mark-Simulacrum Mark-Simulacrum added E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example regression-from-stable-to-stable Performance or correctness regression from one stable version to another. labels Nov 20, 2020
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Nov 20, 2020
@smmalis37
Copy link
Contributor

smmalis37 commented Nov 20, 2020

Some quick notes as I find them:
This reproduces on just a cargo check, so has nothing to do with codegen (unsurprising)
Compilation time scales by a little more than 2x for each additional number added (going from 0 to 20 took 11 seconds, 21 took 26, 22 took 1 min 5)
Given that I don't think this is a hard hang per se, just a really really long running compilation
Definitely something in macro expansion, compiling the post-cargo expand code is instant
Reproduces on current nightly.
-Zself-profile shows all the time is spent in expand_crate.
Unfortunately I don't have a debug build of rustc handy, but I imagine slapping a profiler on one should make the issue fairly obvious. I'd recommend cutting the provided example down from 64 to 20 though.

@jyn514 jyn514 added I-compiletime Issue: Problems and improvements with respect to compile times. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 21, 2020
@SNCPlay42
Copy link
Contributor

searched nightlies: from nightly-2020-08-24 to nightly-2020-10-03
regressed nightly: nightly-2020-10-02
searched commits: from ef663a8 to 154f1f5
regressed commit: b218b95

bisected with cargo-bisect-rustc v0.6.0

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --preserve --start 2020-08-24 --end 2020-10-03 --script ./test.sh 

Looks like #77153

@bobbygebert
Copy link

If it helps, this crate was exploring some ideas related to the typenum crate

@Aaron1011 Aaron1011 self-assigned this Nov 21, 2020
@camelid camelid added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Nov 23, 2020
@camelid
Copy link
Member

camelid commented Nov 23, 2020

Assigning P-high and removing I-prioritize as discussed in the prioritization working group.

@Aaron1011
Copy link
Member

This is caused by some of the extra logic I added in #77153 to ensure that we don't throw out the captured TokenStream unnecessarily.

I'm looking into improving the performance. In the longer term, this will be solved by getting rid of the pretty-print/retokenize hack entirely.

bors added a commit to rust-lang-ci/rust that referenced this issue Nov 26, 2020
…petrochenkov

Cache pretty-print/retokenize result to avoid compile time blowup

Fixes rust-lang#79242

If a `macro_rules!` recursively builds up a nested nonterminal
(passing it to a proc-macro at each step), we will end up repeatedly
pretty-printing/retokenizing the same nonterminals. Unfortunately, the
'probable equality' check we do has a non-trivial cost, which leads to a
blowup in compilation time.

As a workaround, we cache the result of the 'probable equality' check,
which eliminates the compilation time blowup for the linked issue. This
commit only touches a single file (other than adding tests), so it
should be easy to backport.

The proper solution is to remove the pretty-print/retokenize hack
entirely. However, this will almost certainly break a large number of
crates that were relying on hygiene bugs created by using the reparsed
`TokenStream`. As a result, we will definitely not want to backport
such a change.
@bors bors closed this as completed in 6e466ef Nov 27, 2020
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, ..) E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example I-compiletime Issue: Problems and improvements with respect to compile times. P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants