-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Fix another ICE in rustdoc scrape_examples #90611
Conversation
d3ecdae
to
51073d1
Compare
@fee1-dead can you please add a test for this? |
Thanks @fee1-dead !! One request. Can you add a |
I found the The file is available to download from here |
Please try to make the test case an MCVE too. I don't think we should have a 500 line test case if we can avoid it. |
@fee1-dead if you like, I know that https://github.com/chengniansun/perses is a tool for trying to reduce test cases, it may or may not be easier than trying to reduce this manually. |
964eced
to
9c14d82
Compare
Thanks @jyn514! Perses is a very powerful tool and it reduced it to |
Hmm, that doesn't actually give me a lot of confidence that we know how this crash happens ... but I guess it's not super important as long as it's fixed and we have a test it won't regress. @bors r=jyn514,willcrichton |
📌 Commit 9c14d82 has been approved by |
…askrgr Rollup of 7 pull requests Successful merges: - rust-lang#90611 (Fix another ICE in rustdoc scrape_examples) - rust-lang#91197 (rustdoc: Rename `Type::ResolvedPath` to `Type::Path` and don't re-export it) - rust-lang#91223 (Fix headings indent) - rust-lang#91240 (Saner formatting for UTF8_CHAR_WIDTH table) - rust-lang#91248 (Bump compiler-builtins to 0.1.53) - rust-lang#91252 (Fix bug where submodules wouldn't be updated when running x.py from a subdirectory) - rust-lang#91259 (Remove `--display-doctest-warnings`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
@fee1-dead
All test outputs should be limited to the build directory. |
@@ -0,0 +1,4 @@ | |||
// compile-flags: -Z unstable-options --scrape-examples-output-path t.calls --scrape-examples-target-crate foobar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can just remove output-path
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't think so, it will error if either of the two options are unset. Perhaps it should point to build/tmp/?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it will error if either of the two options are unset.
well that's odd ... @willcrichton is there a reason you made that a hard error instead of giving it a default value?
anyway, in the meantime, try {{build-base}}/t.calls
: https://rustc-dev-guide.rust-lang.org/tests/adding.html#input-normalization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There isn't an obvious default value IMO. Eg doing {crate_name}.calls
isn't desirable due to the many conflicting crate names in a workspace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could do {crate_name}-{crate_type}
maybe. I don't think it has to be amazing, just some reasonable default to match --output-format
json.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any tool that cares about conflicts should be using output-path
like you suggested.
Maybe it would be useful to write a tidy check for this? |
Change output path to {{build-base}} for rustdoc scrape_examples ui test See rust-lang#90611 (comment) r? `@jyn514` cc `@petrochenkov`
This has occurred to me when documenting a crate with the arguments. Not sure what could have caused it.
r? @willcrichton