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

"the main function must be defined at the crate level" for doc test regression 1.81->1.82 #131893

Open
joshka opened this issue Oct 18, 2024 · 4 comments
Labels
A-doctests Area: Documentation tests, run by rustdoc C-bug Category: This is a bug. I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@joshka
Copy link
Contributor

joshka commented Oct 18, 2024

The tokio.rs website tests all the code blocks in markdown files by building up a rust file with code included as doc comments on generated functions. The code is generally wrapped in a function to avoid any main functions being interpreted as a test main function. In 1.81, this worked fine, but 1.82 sees the internal main function and treats it as one which prevents the default test main being created.

Code

I tried this code:

/// ```
/// # fn dox() {
/// fn main() {}
/// # }
/// ```
fn foo() {}

I expected to see this happen: cargo test should succeed

Instead, this happened: cargo test failed

Version it worked on

It most recently worked on: 1.81

rustc --version --verbose:

rustc 1.82.0 (f6e511eec 2024-10-15)
binary: rustc
commit-hash: f6e511eec7342f59a25f7c0534f1dbea00d01b14
commit-date: 2024-10-15
host: aarch64-apple-darwin
release: 1.82.0
LLVM version: 19.1.1

@rustbot modify labels: +regression-from-stable-to-stable -regression-untriaged

@joshka joshka added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Oct 18, 2024
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. and removed regression-untriaged Untriaged performance or correctness regression. labels Oct 18, 2024
@jieyouxu jieyouxu added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Oct 18, 2024
@apiraino
Copy link
Contributor

@joshka can you provide the exact file configuration (files and directories) that reproduces the issue? I'm sorry, I'm probably missing the obvious. thanks

@Darksonn
Copy link
Contributor

Pasting it into the playground and choosing "test" hits the error, so a new project with the above in src/lib.rs should trigger it.

@apiraino
Copy link
Contributor

apiraino commented Oct 18, 2024

ok, if my bisection is correct, it points to e5b3e68 (though I'm not completely sure how it's relevant)

cc @GuillaumeGomez

@fmease fmease added A-doctests Area: Documentation tests, run by rustdoc and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Oct 18, 2024
@joshka
Copy link
Contributor Author

joshka commented Oct 18, 2024

The function for checking for functions named main() previously only looked at the top level functions, but now looks recursively for any functions named main.

// Recurse through functions body. It is necessary because the doctest source code is
// wrapped in a function to limit the number of AST errors. If we don't recurse into
// functions, we would thing all top-level items (so basically nothing).
fn check_item(item: &ast::Item, info: &mut ParseSourceInfo, crate_name: &Option<&str>) {

It likely instead needs to stop the recursion level at the automatically generated doctest function. That new function wrapper was created so that the tests can be merged into a single file and compiled once for speed rather than a file per doctest.

In addition to the change here, the line numbers in errors in the new approach make it difficult to correlate multiple failures into a source document. This seems to have always been the case, but to have gotten worse with the doc test merging in 1.82. I haven't checked whether there's another bug raised for this, but it will be related the the same code change here.

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 C-bug Category: This is a bug. I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
Status: No status
Development

No branches or pull requests

6 participants