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

Make some lint doctests compatible with --stage=0 #130353

Merged
merged 1 commit into from
Sep 15, 2024

Conversation

Zalathar
Copy link
Contributor

Currently, running x test compiler --stage=0 (with rust.parallel-compiler=false to avoid other problems) results in two failures, because these lint doctests aren't compatible with the current stage0 compiler.

In theory, the more “correct” solution would be to wrap the opening triple-backtick line in #[cfg_attr(not(bootstrap), doc = "..."]. However, that causes a few practical problems:

  • tidy doesn't understand that syntax, and miscounts the number of backticks in the comment block.
  • lint-docs doesn't understand that syntax, and thinks it's trying to declare the lint name.
  • Working around the above problems would cause more work and more confusion for whoever does the next bootstrap beta bump.

So instead this PR adds some bootstrap gates inside the individual doctests, which end up producing the desired behaviour, and are straightforward to remove.

@rustbot
Copy link
Collaborator

rustbot commented Sep 14, 2024

r? @michaelwoerister

rustbot has assigned @michaelwoerister.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 Sep 14, 2024
@Zalathar
Copy link
Contributor Author

I originally did things the other way in #130345, but when splitting this off into its own PR it occurred to me that this way is nicer.

Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Oh right, this is indeed unfortunate...

@jieyouxu
Copy link
Member

Thanks!
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Sep 15, 2024

📌 Commit 14ed979 has been approved by jieyouxu

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 Sep 15, 2024
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 15, 2024
Make some lint doctests compatible with `--stage=0`

Currently, running `x test compiler --stage=0` (with `rust.parallel-compiler=false` to avoid other problems) results in two failures, because these lint doctests aren't compatible with the current stage0 compiler.

In theory, the more “correct” solution would be to wrap the opening triple-backtick line in  `#[cfg_attr(not(bootstrap), doc = "..."]`. However, that causes a few practical problems:
- `tidy` doesn't understand that syntax, and miscounts the number of backticks in the comment block.
- `lint-docs` doesn't understand that syntax, and thinks it's trying to declare the lint name.
- Working around the above problems would cause more work and more confusion for whoever does the next bootstrap beta bump.

So instead this PR adds some bootstrap gates inside the individual doctests, which end up producing the desired behaviour, and are straightforward to remove.
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 15, 2024
Rollup of 6 pull requests

Successful merges:

 - rust-lang#130042 (properly handle EOF in BufReader::peek)
 - rust-lang#130061 (Add `NonNull` convenience methods to `Box` and `Vec`)
 - rust-lang#130202 (set `download-ci-llvm = true` by default on "library" and "tools" profiles)
 - rust-lang#130214 (MaybeUninit::zeroed: mention that padding is not zeroed)
 - rust-lang#130353 (Make some lint doctests compatible with `--stage=0`)
 - rust-lang#130370 (unstable-book: `trait_upcasting` example should not have `#![allow(incomplete_features)]`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 15, 2024
Rollup of 6 pull requests

Successful merges:

 - rust-lang#130042 (properly handle EOF in BufReader::peek)
 - rust-lang#130061 (Add `NonNull` convenience methods to `Box` and `Vec`)
 - rust-lang#130202 (set `download-ci-llvm = true` by default on "library" and "tools" profiles)
 - rust-lang#130214 (MaybeUninit::zeroed: mention that padding is not zeroed)
 - rust-lang#130353 (Make some lint doctests compatible with `--stage=0`)
 - rust-lang#130370 (unstable-book: `trait_upcasting` example should not have `#![allow(incomplete_features)]`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 12fb8e4 into rust-lang:master Sep 15, 2024
6 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Sep 15, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 15, 2024
Rollup merge of rust-lang#130353 - Zalathar:lint-zero, r=jieyouxu

Make some lint doctests compatible with `--stage=0`

Currently, running `x test compiler --stage=0` (with `rust.parallel-compiler=false` to avoid other problems) results in two failures, because these lint doctests aren't compatible with the current stage0 compiler.

In theory, the more “correct” solution would be to wrap the opening triple-backtick line in  `#[cfg_attr(not(bootstrap), doc = "..."]`. However, that causes a few practical problems:
- `tidy` doesn't understand that syntax, and miscounts the number of backticks in the comment block.
- `lint-docs` doesn't understand that syntax, and thinks it's trying to declare the lint name.
- Working around the above problems would cause more work and more confusion for whoever does the next bootstrap beta bump.

So instead this PR adds some bootstrap gates inside the individual doctests, which end up producing the desired behaviour, and are straightforward to remove.
@Zalathar Zalathar deleted the lint-zero branch September 15, 2024 07:54
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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants