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.

Sign up for GitHub

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

Jump to bottom

make /// doc comments compatible with naked functions #128380

Merged
merged 1 commit into from
Jul 31, 2024
Merged

make /// doc comments compatible with naked functions #128380

merged 1 commit into from
Jul 31, 2024

Conversation

folkertdev
Copy link
Contributor

@folkertdev folkertdev commented Jul 30, 2024 •
edited
Loading

tracking issue: #90957

reported in #127853 (comment)

it turns out /// doc comment and #[doc = "doc comment"] are represented differently, at least at the point where we perform the check for what should be allowed. The /// style doc comment is now also allowed.

r? @bjorn3

cc @hsanzg

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 30, 2024
@folkertdev
Copy link
Contributor Author

I am a bit confused by this comment here https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/ast/struct.Attribute.html#method.is_doc_comment

/// Returns `true` if it is a sugared doc comment (`///` or `//!` for example).
/// So `#[doc = "doc"]` (which is a doc comment) and `#[doc(...)]` (which is not
/// a doc comment) will return `false`.

So, what is #[doc(...)] then?

@folkertdev folkertdev force-pushed the naked-compatible-doc-comment branch from 432528c to 2ec2631 Compare July 30, 2024 08:12
@bjorn3
Copy link
Member

bjorn3 commented Jul 30, 2024

Looks like #[doc(...)] is represented as AttrKind::Normal, while /// ... is represented as AttrKind::DocComment`.

@folkertdev
Copy link
Contributor Author

yes but I'm not sure what the difference is between #[doc = "..."] and #[doc(...)]. Both of these should be AttrKind::Normal, but the comment says that #[doc(...)] is not a doc comment. What is it then, and is it relevant here, because right now anything with sym::doc gets accepted.

@folkertdev
Copy link
Contributor Author

https://doc.rust-lang.org/rustdoc/write-documentation/the-doc-attribute.html#at-the-item-level

allright, so it's directives for how documentation should be rendered. Seems fine to allow

@bjorn3
Copy link
Member

bjorn3 commented Jul 30, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Jul 30, 2024

📌 Commit 2ec2631 has been approved by bjorn3

It is now in the queue for this repository.

@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-review Status: Awaiting review from the assignee but also interested parties. labels Jul 30, 2024
@folkertdev folkertdev force-pushed the naked-compatible-doc-comment branch from 2ec2631 to 58bfd98 Compare July 30, 2024 10:20
@folkertdev
Copy link
Contributor Author

I added a test for #[doc(...)] specifically, seems to be just out of order with the approve

@bjorn3
Copy link
Member

bjorn3 commented Jul 30, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Jul 30, 2024

📌 Commit 58bfd98 has been approved by bjorn3

It is now in the queue for this repository.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 30, 2024
@matthiaskrgr
Rollup merge of rust-lang#128380 - folkertdev:naked-compatible-doc-co…
d21d3d6 
…mment, r=bjorn3

make `///` doc comments compatible with naked functions

tracking issue: rust-lang#90957

reported in rust-lang#127853 (comment)

it turns out `/// doc comment` and `#[doc = "doc comment"]` are represented differently, at least at the point where we perform the check for what should be allowed. The `///` style doc comment is now also allowed.

r? `@bjorn3`

cc `@hsanzg`
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 30, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 30, 2024
@bors
Auto merge of rust-lang#128403 - matthiaskrgr:rollup-za99w4l, r=matth…
7fb1d02 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#127543 (More unsafe attr verification)
 - rust-lang#128357 (Detect non-lifetime binder params shadowing item params)
 - rust-lang#128367 (CI: rfl: build the generated doctests and documentation)
 - rust-lang#128376 (Mark `Parser::eat`/`check` methods as `#[must_use]`)
 - rust-lang#128379 (the output in stderr expects panic-unwind)
 - rust-lang#128380 (make `///` doc comments compatible with naked functions)
 - rust-lang#128382 (cargo-miri: better error when we seem to run inside bootstrap but something is wrong)
 - rust-lang#128398 (tidy: Fix quote in error message)

r? `@ghost`
`@rustbot` modify labels: rollup
@tgross35 tgross35 mentioned this pull request Jul 30, 2024
Merged
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 30, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 30, 2024
@bors
Auto merge of rust-lang#128413 - matthiaskrgr:rollup-nrfcvdq, r=matth…
249cf71 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#128357 (Detect non-lifetime binder params shadowing item params)
 - rust-lang#128367 (CI: rfl: build the generated doctests and documentation)
 - rust-lang#128376 (Mark `Parser::eat`/`check` methods as `#[must_use]`)
 - rust-lang#128379 (the output in stderr expects panic-unwind)
 - rust-lang#128380 (make `///` doc comments compatible with naked functions)
 - rust-lang#128382 (cargo-miri: better error when we seem to run inside bootstrap but something is wrong)
 - rust-lang#128398 (tidy: Fix quote in error message)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e6a82d2 into rust-lang:master Jul 31, 2024
6 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Jul 31, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 31, 2024
@rust-timer
Unrolled build for rust-lang#128380
ec3d1dd 
Rollup merge of rust-lang#128380 - folkertdev:naked-compatible-doc-comment, r=bjorn3

make `///` doc comments compatible with naked functions

tracking issue: rust-lang#90957

reported in rust-lang#127853 (comment)

it turns out `/// doc comment` and `#[doc = "doc comment"]` are represented differently, at least at the point where we perform the check for what should be allowed. The `///` style doc comment is now also allowed.

r? ``@bjorn3``

cc ``@hsanzg``
@hsanzg
Copy link

hsanzg commented Jul 31, 2024

Thank you for the quick fix!

@Kobzol
Copy link
Contributor

Kobzol commented Jul 31, 2024

@rust-timer build ec3d1dd

@rust-timer

This comment has been minimized.

Sign in to view
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (ec3d1dd): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 769.587s -> 770.011s (0.06%)
Artifact size: 331.78 MiB -> 331.90 MiB (0.04%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@bjorn3 bjorn3

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.82.0
Development

Successfully merging this pull request may close these issues.
7 participants
@folkertdev @bjorn3 @bors @hsanzg @Kobzol @rust-timer @rustbot