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

merged doctests: implicit unused overrides explicit attributes with no_run #130681

Closed
ehuss opened this issue Sep 22, 2024 · 1 comment · Fixed by #131096
Closed

merged doctests: implicit unused overrides explicit attributes with no_run #130681

ehuss opened this issue Sep 22, 2024 · 1 comment · Fixed by #131096
Labels
A-doctests Area: Documentation tests, run by rustdoc A-edition-2024 Area: The 2024 edition C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@ehuss
Copy link
Contributor

ehuss commented Sep 22, 2024

With the 2024 merged doctests, there is a slight change in behavior with regards to the unused lints with a no_run test. Because there is an implicit #![allow(unused)] on each test when the test is no_run (code), this overrides any #![doc(test(attr(...)))] attributes.

When building standalone, this doesn't happen because I believe this code checks for any doc test attributes before adding the allow(unused).

I tried this code:

#![doc(test(attr(allow(unused_variables), deny(warnings))))]

/// Example
///
/// ```rust,no_run
/// trait T { fn f(); }
/// ```
pub fn f() {}

On edition 2021, the above will fail because the deny(warnings) generates an error with #[deny(dead_code)] implied by #[deny(warnings)]. This also fails on 2024 with standalone.

On edition 2024, the above will pass.

I generally would expect the behavior to be the same. I'm not sure if it is possible to check if there are any attributes before adding the allow(unused). I'm also unclear why no_run is significant here.

Meta

rustdoc 1.83.0-nightly (da889684c 2024-09-20)
binary: rustdoc
commit-hash: da889684c80508036ff036db8c159ffdcf27648a
commit-date: 2024-09-20
host: aarch64-apple-darwin
release: 1.83.0-nightly
LLVM version: 19.1.0
@ehuss ehuss added A-doctests Area: Documentation tests, run by rustdoc A-edition-2024 Area: The 2024 edition C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Sep 22, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 22, 2024
@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 23, 2024
@GuillaumeGomez
Copy link
Member

Good catch, thanks! Taking a look.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Oct 1, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 29, 2024
…riddle

rustdoc: Remove usage of `allow(unused)` attribute on `no_run` merged doctests

Fixes [rust-lang#130681](rust-lang#130681).

It fixes the behaviour difference with the current doctests.

r? `@notriddle`
jieyouxu added a commit to jieyouxu/rust that referenced this issue Oct 30, 2024
…riddle

rustdoc: Remove usage of `allow(unused)` attribute on `no_run` merged doctests

Fixes [rust-lang#130681](rust-lang#130681).

It fixes the behaviour difference with the current doctests.

r? ``@notriddle``
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 30, 2024
Rollup merge of rust-lang#131096 - GuillaumeGomez:rm-no_unused, r=notriddle

rustdoc: Remove usage of `allow(unused)` attribute on `no_run` merged doctests

Fixes [rust-lang#130681](rust-lang#130681).

It fixes the behaviour difference with the current doctests.

r? ``@notriddle``
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-doctests Area: Documentation tests, run by rustdoc A-edition-2024 Area: The 2024 edition C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
Development

Successfully merging a pull request may close this issue.

4 participants