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

Correctly handle comments in attributes in doctests source code #134260

Merged
merged 4 commits into from
Dec 16, 2024

Conversation

GuillaumeGomez
Copy link
Member

Fixes #134221.

The problem was that attributes are "inlined" (backlines are stripped), then when there is an inline comment inside it, the attribute is never considered valid (since unclosed). Fix was to simply put back backlines in case it's a multiline attribute.

r? @notriddle

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Dec 13, 2024
@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez
Copy link
Member Author

Forgot to set only one test thread to prevent having fuzzy sorting...

@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez
Copy link
Member Author

And to replace stdout for time too...

@rust-log-analyzer

This comment has been minimized.

@cyrgani
Copy link
Contributor

cyrgani commented Dec 13, 2024

Does this also fix that

//! ```rust
//! #![
//! ```

is incorrectly treated as a sucessful test?

@GuillaumeGomez
Copy link
Member Author

Now it does. :)

@GuillaumeGomez
Copy link
Member Author

Ready for review.

Comment on lines 540 to 541
} else if mod_attr_pending.ends_with('\\') {
mod_attr_pending.push('n');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand this correctly, these lines aren't really needed any more. They would have turned

#[doc="first\
second"]

into

#[doc="first\nsecond"]

But that only works with double-quoted strings, not raw strings, and is unnecessary now that newlines are preserved here.

Suggested change
} else if mod_attr_pending.ends_with('\\') {
mod_attr_pending.push('n');

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a test just in case.

@GuillaumeGomez
Copy link
Member Author

Removed the backline handling and also added a regression test for it.

@notriddle
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 16, 2024

📌 Commit c367cc3 has been approved by notriddle

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 Dec 16, 2024
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Dec 16, 2024
…triddle

Correctly handle comments in attributes in doctests source code

Fixes rust-lang#134221.

The problem was that attributes are "inlined" (backlines are stripped), then when there is an inline comment inside it, the attribute is never considered valid (since unclosed). Fix was to simply put back backlines in case it's a multiline attribute.

r? `@notriddle`
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Dec 16, 2024
…triddle

Correctly handle comments in attributes in doctests source code

Fixes rust-lang#134221.

The problem was that attributes are "inlined" (backlines are stripped), then when there is an inline comment inside it, the attribute is never considered valid (since unclosed). Fix was to simply put back backlines in case it's a multiline attribute.

r? ``@notriddle``
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Dec 16, 2024
…triddle

Correctly handle comments in attributes in doctests source code

Fixes rust-lang#134221.

The problem was that attributes are "inlined" (backlines are stripped), then when there is an inline comment inside it, the attribute is never considered valid (since unclosed). Fix was to simply put back backlines in case it's a multiline attribute.

r? ```@notriddle```
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 16, 2024
…llaumeGomez

Rollup of 9 pull requests

Successful merges:

 - rust-lang#132056 (Stabilize `#[diagnostic::do_not_recommend]`)
 - rust-lang#134124 (CI: use free runners for x86_64-gnu-llvm jobs)
 - rust-lang#134197 (rustc_mir_build: Clarify that 'mirrored' does not mean 'flipped' or 'reversed')
 - rust-lang#134260 (Correctly handle comments in attributes in doctests source code)
 - rust-lang#134277 (rustdoc-search: handle `impl Into<X>` better)
 - rust-lang#134284 (Keep track of patterns that could have introduced a binding, but didn't)
 - rust-lang#134337 (reject unsound toggling of RISCV target features)
 - rust-lang#134385 (tests/ui/asm: Remove uses of rustc_attrs, lang_items, and decl_macro features by using minicore)
 - rust-lang#134386 (Some trait method vs impl method signature difference diagnostic cleanups)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 16, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#134124 (CI: use free runners for x86_64-gnu-llvm jobs)
 - rust-lang#134197 (rustc_mir_build: Clarify that 'mirrored' does not mean 'flipped' or 'reversed')
 - rust-lang#134260 (Correctly handle comments in attributes in doctests source code)
 - rust-lang#134277 (rustdoc-search: handle `impl Into<X>` better)
 - rust-lang#134284 (Keep track of patterns that could have introduced a binding, but didn't)
 - rust-lang#134337 (reject unsound toggling of RISCV target features)
 - rust-lang#134371 (Check for array lengths that aren't actually `usize`)
 - rust-lang#134385 (tests/ui/asm: Remove uses of rustc_attrs, lang_items, and decl_macro features by using minicore)
 - rust-lang#134386 (Some trait method vs impl method signature difference diagnostic cleanups)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 9451a61 into rust-lang:master Dec 16, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 16, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 16, 2024
Rollup merge of rust-lang#134260 - GuillaumeGomez:doctest-attrs, r=notriddle

Correctly handle comments in attributes in doctests source code

Fixes rust-lang#134221.

The problem was that attributes are "inlined" (backlines are stripped), then when there is an inline comment inside it, the attribute is never considered valid (since unclosed). Fix was to simply put back backlines in case it's a multiline attribute.

r? ``@notriddle``
@GuillaumeGomez GuillaumeGomez deleted the doctest-attrs branch December 17, 2024 09:59
@GuillaumeGomez
Copy link
Member Author

Nominating it for beta backporting.

@GuillaumeGomez GuillaumeGomez added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-nominated Nominated for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding a comment next to #![feature(ptr_metadata,)] silences failing doctests
6 participants