-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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 ICE when rustdoc is scraping examples inside of a proc macro #90583
Conversation
Can you also test what happens if it's buggy and doesn't propagate spans? It seems fine to discard that example, I just want to make sure it doesn't cause an ICE. |
Could you separate that into another commit? It'll make it easier to understand why code was changed later on. And thanks for adding a test ❤️ |
This comment has been minimized.
This comment has been minimized.
For posterity, this fixes the following ICE when running on Pyo3 (reported on discord):
|
(FYI, I just created an |
b3ea4a4
to
4846d10
Compare
@camelid I've separated this into two commits.
@jyn514 the rustdoc-scrape-examples-macro test already does this, if I'm understanding you correctly. The |
r=me with CI passing :) @bors delegate=willcrichton |
✌️ @willcrichton can now approve this pull request |
@bors r=jyn514 |
📌 Commit 4846d10 has been approved by |
…n514 Fix ICE when rustdoc is scraping examples inside of a proc macro This PR provides a clearer semantics for how --scrape-examples interacts with macros. If an expression's span AND it's enclosing item's span both are not `from_expansion`, then the example will be scraped. The added test case `rustdoc-scrape-examples-macros` shows a variety of situations. * A macro-rules macro that takes a function call as input: good * A macro-rules macro that generates a function call as output: bad * A proc-macro that generates a function call as output: bad * An attribute macro that generates a function call as output: bad * An attribute macro that takes a function call as input: good, if the proc macro is designed to propagate the input spans I ran this updated rustdoc on pyo3 and confirmed that it successfully scrapes examples from inside a proc macro, eg <img width="1013" alt="Screen Shot 2021-11-04 at 1 11 28 PM" src="https://user-images.githubusercontent.com/663326/140412691-81a3bb6b-a448-4a1b-a293-f7a795553634.png"> (cc `@mejrs)` Additionally, this PR fixes an ordering bug in the highlighting logic. Fixes rust-lang#90567. r? `@jyn514`
Is this supposed to draw code examples from |
…n514 Fix ICE when rustdoc is scraping examples inside of a proc macro This PR provides a clearer semantics for how --scrape-examples interacts with macros. If an expression's span AND it's enclosing item's span both are not `from_expansion`, then the example will be scraped. The added test case `rustdoc-scrape-examples-macros` shows a variety of situations. * A macro-rules macro that takes a function call as input: good * A macro-rules macro that generates a function call as output: bad * A proc-macro that generates a function call as output: bad * An attribute macro that generates a function call as output: bad * An attribute macro that takes a function call as input: good, if the proc macro is designed to propagate the input spans I ran this updated rustdoc on pyo3 and confirmed that it successfully scrapes examples from inside a proc macro, eg <img width="1013" alt="Screen Shot 2021-11-04 at 1 11 28 PM" src="https://user-images.githubusercontent.com/663326/140412691-81a3bb6b-a448-4a1b-a293-f7a795553634.png"> (cc `@mejrs)` Additionally, this PR fixes an ordering bug in the highlighting logic. Fixes rust-lang#90567. r? `@jyn514`
⌛ Testing commit c62817b with merge 86e2b3bd0cb2eff1a3502fd66296a3f3d8e480da... |
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
@willcrichton every platform names the suffix for dynamic libraries something different - you can either try to make it work in the makefile (with |
Ok I did your suggested change @jyn514. |
@bors r+ Thanks! |
📌 Commit 82b23be has been approved by |
☀️ Test successful - checks-actions |
Finished benchmarking commit (0d1754e): comparison url. Summary: This change led to moderate relevant improvements 🎉 in compiler performance.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression |
I guess fetching the span was quite slow and moving it to after the expansion check (and other checks) improved performance? |
This PR provides a clearer semantics for how --scrape-examples interacts with macros. If an expression's span AND it's enclosing item's span both are not
from_expansion
, then the example will be scraped. The added test caserustdoc-scrape-examples-macros
shows a variety of situations.I ran this updated rustdoc on pyo3 and confirmed that it successfully scrapes examples from inside a proc macro, eg
(cc @mejrs)
Additionally, this PR fixes an ordering bug in the highlighting logic.
Fixes #90567.
r? @jyn514