Skip to content

Backport: Merged doctests unused in stable edition 2024 #138418

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
4 tasks done
notriddle opened this issue Mar 12, 2025 · 1 comment
Closed
4 tasks done

Backport: Merged doctests unused in stable edition 2024 #138418

notriddle opened this issue Mar 12, 2025 · 1 comment
Labels
C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. T-release Relevant to the release subteam, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@notriddle
Copy link
Contributor

notriddle commented Mar 12, 2025

To do:

What happened

Due to a bug in the implementation, merged doctests did not ship on stable. This is caused by the merged “runner” executable using libtest, so the fallback-to-unmerged behavior kicked in and the promised performance improvement didn’t happen.

Proposed remedy

To backport the fix to a point release, before any code accidentally relies on this behavior.

Possible breakage

Merged doctests work by string concatenating doctests into a wrapper binary that looks roughly like this (except that it uses libtest to report progress, and invokes a separate process for every test case).

mod __test_1 { pub fn main() { … } }
mod __test_2 { pub fn main() { … } }
fn main() { __test_1::main(); __test_2::main(); }

If the merged version fails to compile, we silently and automatically fall back to the old way of running them separately.

However, it is possible to write a doctest that successfully compiles as merged, but fails when run. For example, you can examine the stack trace using std::panic::Location. No automatic fallback happens if a doctest fails at runtime.

When we first developed this feature, we tested it on crater and saw that runtime failures introduced by this change were rare #130285 (the one pattern of failure that we found was caused by tests reading their command-line args, and we fixed that). This is why we consider the potential breakage an acceptable risk.

@notriddle notriddle added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Mar 12, 2025
@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. labels Mar 12, 2025
@jieyouxu jieyouxu added T-release Relevant to the release subteam, which will review and decide on the PR/issue. and removed 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. labels Mar 13, 2025
@cuviper
Copy link
Member

cuviper commented Mar 18, 2025

It's done as of 1.86-beta.6 and 1.85.1!

@cuviper cuviper closed this as completed Mar 18, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. T-release Relevant to the release subteam, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants