Add documentation for the target_feature attribute - #160218
Conversation
|
I don't know too much about this attribute so I prefer letting someone else review it. r? @RalfJung |
|
|
|
The attribute is already documented in the Reference. Is it a good idea to duplicate that documentation here? |
There was a problem hiding this comment.
This seems correct from the opsem side but the writing needs work.
Unfortunately I don't have the time to shepherd this all the way through.
r? @workingjubilee
| // | ||
| /// Enables extra CPU instructions for a single function. | ||
| /// | ||
| /// A program is compiled for a baseline CPU, so instructions that only newer CPUs support go |
There was a problem hiding this comment.
Where does the baseline come from?
| /// baseline. The `-C target-feature` and `-C target-cpu` compiler flags enable features for the | ||
| /// whole crate instead, which raises the minimum CPU the binary runs on. |
There was a problem hiding this comment.
This is an oddly dangling remark without flow.
It seems better to first mention the -C flags, and then pivot from there to the per-function version. Also, -Ctarget-cpu is very different from -Ctarget-fature, so throwing them into one bag here is quite confusing. -Ctarget-feature and #[target_feature] are closely related, they work on the same set of features; that should be mentioned.
|
|
|
Also, given the context in #160219, I would like to emphasize that in my view, we should not land AI-generated docs. I don't know if these docs were AI-generated, and this is just my opinion as the official policy banning AI-generated docs has not landed yet. But to inform my own judgment as a reviewer I'd like to know whether you wrote these docs entirely yourself or whether you edited a draft that was generated by AI. This is relevant because AI-generated text and code needs to be reviewed in a very different way than human-written text and code (because the kinds of mistakes AIs make are different from the mistakes humans make). |
This comment has been minimized.
This comment has been minimized.
Document the built-in `target_feature` attribute, using the `#[doc(attribute = "..."`)]
507f37d to
93b83d9
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
Thanks for reviewing and highlighting what worth attention. The initial version of this PR was AI-generated, and I hadn't disclosed that, sorry for that, I've changed how I work on contributions since getting the feedback on AI usage, and follow LLM Policy. As for the docs, I made it more guide focused, and beginner friendly, it's detailed so If you think anything should be cut out to not duplicate information from Reference, I could shrink it. |
|
Thanks for the update, I'll try to take a look soon. |
Documents the
target_featureattribute, aiming at the guide level and beginner friendly. Low level details and per-architecture feature table stay out in the Reference.The example is wrapped in a hidden
#[cfg(target_arch = "x86_64")]block to make the example compile everywhere the doctests run.Part of #157604
r? @GuillaumeGomez
Tested with
./x test library/core --doc --test-args attribute_docsand the same forlibrary/std.