-
Notifications
You must be signed in to change notification settings - Fork 13.3k
ICE building docs of some crates in beta #47639
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
Comments
Crate oftlisp-anf is also affected, cc @remexre |
@rust-lang/dev-tools Can someone investigate and see if we can fix this? |
I'll give it a look. |
Both
|
I ran
|
So maybe we need a backport to beta? That may have already happened, this is a relatively old beta run. |
First we'd need to find what's crashing... |
I've tried bisecting in the range 33374fa...7d6e5b9 and the result is... #46115? 😳 Needs to reverify later. Test script: #!/bin/sh
! $CARGO_RELATIVE test --manifest-path=libsecp256k1/Cargo.toml --frozen --doc Command:
Edit: Verified, in 2f47a9e the doc test passes and in e97ba83 the doc test ICEs. cc @alexcrichton |
@GuillaumeGomez I just tried documenting
The issue is still present on the latest nightly:
|
@GuillaumeGomez Negative, it is still crashing with 21882aa.
|
This seems potentially somewhat suspicious from the commit we bisected to: https://github.com/rust-lang/rust/pull/46115/files#diff-698522bf32a92da4331b4bd3f6ed31c6R814 |
Here's a minimal example: pub fn test() {
macro_rules! foo {
() => ()
}
} We used to run everybody-loops before macro expansion. Which meant that such macros would be wiped clean (as well as all of their potential users). We now run it later. These macros get cleaned up after we've run them through resolve. This interacts badly with how we do builtin lints. Builtin lints are basically things where writing a lint pass to do the check would be annoying because it requires a global analysis, but we can instead collect up the requisite information beforehand during other global passes, and then when we are actually linting a node we can just check what lints have accumulated for that node. For example, the unused lints work by paring down a list of "used" things, and then buffering up an "unused" lint for each thing that's still unused after the resolution check. We can't warn immediately because we'd have to recompute what the lint level is. The unused macro lint is naturally a builtin lint. Macro expansion queues up a list of "this macro is unused" lints for each macro node id. Of course, with the everybody-loops pass, we delete the macros within the blocks. Whoops. And this causes the lint pass to notice that some of the accumulated builtin lints were not checked (because their nodes no longer exist), and complain and ICE. Whoops. To fix this, we can:
I'd rather lean towards conditioning or removing that |
For some reason I'm not unable to get this ICE to trigger by writing a proc macro that does this. Curious. Perhaps this occurs pre-macro resolution. |
Oh, I think the proc macro just "gets there first" because it's defined outside. Okay. So checking for rustdoc mode should be enough. |
…ulacrum Fix rustdoc ICE on macros defined within functions fixes rust-lang#47639
When building the documentation (or running the doctests) of a few crates, such as libsecp256k1, in beta or nightly, rustdoc crashes with this message (while it works fine on stable, even if with some pulldown warnings):
cc @sorpaas
The text was updated successfully, but these errors were encountered: