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

rustdoc: expose #[target_feature] attributes as doc(cfg) flags #48759

Merged
merged 2 commits into from
Mar 22, 2018

Conversation

QuietMisdreavus
Copy link
Member

This change exposes #[target_feature(enable = "feat")] attributes on an item as if they were also #[doc(cfg(target_feature = "feat"))] attributes. This gives them a banner on their documentation listing which feature is required to use the item. It also modifies the rendering code for doc(cfg) tags to handle target_feature tags. I made it print just the feature name on "short" printings (as in the function listing on a module page), and use "target feature feat" in the full banner on the item page itself.

This way, the function listing in std::arch shows which feature is required for each function:

image

image

@rust-highfive
Copy link
Collaborator

r? @GuillaumeGomez

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 5, 2018
@QuietMisdreavus
Copy link
Member Author

r? @GuillaumeGomez

cc @BurntSushi, @killercup

(btw, expect a PR to stdsimd to add some doc(cfg) so we can render as many arches as possible :3)

@QuietMisdreavus
Copy link
Member Author

One matter this raises is that it uses the #[doc(cfg(...))] feature without using the attribute or feature flag. Is that... valid? >_>

@GuillaumeGomez
Copy link
Member

I think we should precise this is a target feature. Like this, if I didn't know I'd have no idea what id'd be about.

@pietroalbini pietroalbini added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 12, 2018
@bors
Copy link
Contributor

bors commented Mar 14, 2018

☔ The latest upstream changes (presumably #48811) made this pull request unmergeable. Please resolve the merge conflicts.

@QuietMisdreavus
Copy link
Member Author

Fixed the merge conflict.

@GuillaumeGomez I'm not sure i understand. Are you saying we should say "target feature" all the time? My assumption for abbreviating it on the module view was that you're already looking in std::arch or stdsimd, which describe how to use target_feature. Also, none of the other pretty-printed forms of doc(cfg) (target_arch, target_os, target_env, etc) include which "category" of configuration item it is. Strictly speaking, to make it fit in I would need to pretty-print all the known feature names by substituting their "proper name" in, like how it's done with all the other options.

@GuillaumeGomez
Copy link
Member

When looking and the page, if I didn't know what this text was for, I'd be completely lost. Any kind of indication would help users a lot.

@QuietMisdreavus
Copy link
Member Author

And you can click through to see what it means. The "full version" is printed on the function's page. For configurations like target_arch or target_env the current pretty-printer doesn't even provide that luxury. To me, it feels like adding "target feature" to the short form would add an enormous amount of noise, considering that the x86_64 listing has pages and pages worth of functions that would now all say the same thing.

@GuillaumeGomez
Copy link
Member

Thanks!

@bors: r+

@bors
Copy link
Contributor

bors commented Mar 22, 2018

📌 Commit b3fb0d1 has been approved by GuillaumeGomez

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 22, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Mar 22, 2018
…r=GuillaumeGomez

rustdoc: expose #[target_feature] attributes as doc(cfg) flags

This change exposes `#[target_feature(enable = "feat")]` attributes on an item as if they were also `#[doc(cfg(target_feature = "feat"))]` attributes. This gives them a banner on their documentation listing which feature is required to use the item. It also modifies the rendering code for doc(cfg) tags to handle `target_feature` tags. I made it print just the feature name on "short" printings (as in the function listing on a module page), and use "target feature `feat`" in the full banner on the item page itself.

This way, the function listing in `std::arch` shows which feature is required for each function:

![image](https://user-images.githubusercontent.com/5217170/37003222-f41b9d66-2091-11e8-9656-8719e5b34832.png)

![image](https://user-images.githubusercontent.com/5217170/37003234-feb1a7a2-2091-11e8-94de-6d1d76a2d3ee.png)
bors added a commit that referenced this pull request Mar 22, 2018
@alexcrichton alexcrichton merged commit b3fb0d1 into rust-lang:master Mar 22, 2018
@QuietMisdreavus QuietMisdreavus deleted the simd-feature-docs branch April 6, 2018 13:57
kennytm added a commit to kennytm/rust that referenced this pull request Apr 14, 2018
…ures, r=GuillaumeGomez

add target features when extracting and running doctests

When rendering documentation, rustdoc will happily load target features into the cfg environment from the current target, but fails to do this when doing anything with doctests. This would lead to situations where, thanks to rust-lang#48759, functions tagged with `#[target_feature]` couldn't run doctests, thanks to the automatic `#[doc(cfg(target_feature = "..."))]`.

Currently, there's no way to pass codegen options to rustdoc that will affect its rustc sessions, but for now this will let you use target features that come default on the platform you're targeting.

Fixes rust-lang#49723
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants