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

Add check to not doc-scrape proc-macro units. Fixes #10571. #11423

Closed
wants to merge 1 commit into from

Conversation

willcrichton
Copy link
Contributor

What does this PR try to resolve?

This PR fixes #10571, where the new doc-scrape feature panics when asked to scrape a proc-macro library. A missing check was added to create_bcx.

How should we test and review this PR?

A corresponding unit test has been added to demonstrate the fix.

@rustbot
Copy link
Collaborator

rustbot commented Nov 25, 2022

r? @epage

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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 25, 2022
@willcrichton
Copy link
Contributor Author

r? @weihanglo

@willcrichton
Copy link
Contributor Author

Closing this in favor of #11430.

bors added a commit that referenced this pull request Nov 29, 2022
Improve strategy for selecting targets to be scraped for examples

### What does this PR try to resolve?

After #10343, we have identified a clear set of conditions for whether a particular target should be considered by `-Zrustdoc-scrape-examples`. These conditions are described more clearly in #11425.

However, after some testing with complex Cargo workspaces (e.g. [wasmtime](https://github.com/bytecodealliance/wasmtime/)), I realized that the current approach of modifying the `CompileFilter` did not correctly implement this new specification. For example, a target with `doc = false` should not be scraped by default since it is not a documented unit, but the current approach would potentially include such a target for scraping.

This PR provides a new approach which I believe correctly implements the specification:
1. `generate_targets` is called with the same parameters except the `mode` which becomes `CompileMode::Docscrape` instead of `CompileMode::Doc`. `filter_default_targets` generates the same targets for `Docscrape` as for `Doc`.
2. Inside `generate_targets`, an initial set of `Proposal`s are created. This set of proposals is extended with further proposals based on targets identified as `doc-scrape-examples = true`, or Example targets where possible.

This PR subsumes #11423, and also fixes #10571.

### How should we test and review this PR?

I have added another test `docscrape::only_scrape_documented_targets` to verify that only documented or explicitly-enabled targets are included for scraping.

r? `@weihanglo`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cargo panics when using -Z rustdoc-scrape-examples=examples on proc-macro crates
4 participants