Nearly all compiler/ doctests are broken

[jyn514]

test-output.log

I haven't tried --stage 2 but I don't expect it to matter; most of these aren't build errors, they're just because the tests aren't valid syntax.

Found while investigating #95515.

@rustbot label +T-compiler +A-testsuite +C-bug

[jyn514]

The scary thing to me is how long these have been broken - I guess we never run library tests in CI? I think it might be worth considering adding them.

[Mark-Simulacrum]

We "should" be running them, but it's relatively easy today for us to accidentally stop running tests. I have a long-term project in mind to track all executed tests (and target info) to give us positive presence (tests passing) indication rather than just negative (tests failing) as we have today. But that's a somewhat involved endeavor that I haven't had the time to invest in yet - probably a few weeks work.

[jyn514]

Mentoring instructions: For each test, decide whether it's meant to be a test or just a code comment. Then, add ignore to comments or fix the failing test.

For example, this test is just a comment, because it's not meant to be valid syntax:

    /// Desugar `ExprKind::Try` from: `<expr>?` into:
    /// ```
    /// match Try::branch(<expr>) {
    ///     ControlFlow::Continue(val) => #[allow(unreachable_code)] val,,
    ///     ControlFlow::Break(residual) =>
    ///         #[allow(unreachable_code)]
    ///         // If there is an enclosing `try {...}`:
    ///         break 'catch_target Try::from_residual(residual),
    ///         // Otherwise:
    ///         return Try::from_residual(residual),
    /// }
    /// ```
    fn lower_expr_try(&mut self, span: Span, sub_expr: &Expr) -> hir::ExprKind<'hir> {

but this one can easily be a doctest:

#[derive(Clone, Copy, PartialEq)]
enum IndentStyle {
    /// Vertically aligned under whatever column this block begins at.
    ///
    ///     fn demo(arg1: usize,
    ///             arg2: usize);
    Visual,
}

@rustbot label +E-help-wanted +E-mentor

[Elliot-Roberts]

Hi I could try to do this.
I understand how to find the failing doctests, but for the second example (code that is in a doc-comment but not a code block) is there any good way to find those? And in that particular case, while it could be a made a doctest, it was probably written just as text because it's the style not the function that is relevant. Is it important to have those converted to doctests in all cases?

[Elliot-Roberts]

@rustbot claim

[jyn514]

@Elliot-Roberts it already is a doctest. Rustdoc treats any 4-space indented block as a code block. The problem with it is that the test fails - the fix is simple in this case, just change ; to {}. But in general it might not be clear whether a code block should be fixed or just ignored.

[Elliot-Roberts]

Oh I see, thanks for the explanation. Just noticed it in the list of failing tests haha.
It looks like there are ~280 failing doctests in compiler/

[jyn514]

@Elliot-Roberts btw you don't need to fix all of them at once, if you get halfway through and it gets tedious that's still super useful :)

[Elliot-Roberts]

Ok cool. Here's a summary of the different types of failing doctest I've found so far:

1. Ones intended to fail to compile, which are super easy to fix by just adding compile_fail,[error number] (should I always add the error number?)
2. Ones that demonstrate rustc internal stuff or unstable features without the necessary #[whatever]. (should I keep those annotations hidden (with # ...)?)
3. Ones that are pseudo-rust to demonstrate a pattern like your first example. I mark those with ignore (pseudo-rust) instead of text so that we can keep the syntax highlighting.
4. Ones that are "abbreviated" rust with lots of .. and undeclared types (Foo etc.) to illustrate an idea. In these cases, they likely could be made to run, but it would add clutter such that reading the code without proper rendering to hide lines would be much harder, and possibly reading even with rendering would still be worse. Given that they weren't meant to run in the first place, I'm a bit torn. I've been marking them with ignore (illustrative)
5. Ones I don't know how to fix (doing fancy type stuff, and also importing things within rustc is confusing me). I've been marking these with ignore HELP and once I start a (maybe draft) PR I'll list their locations and ask you for help.

Please let me know if my described approach to each of those sounds reasonable to you.

[jyn514]

That all sounds great! No need to hide the attributes on type 2 tests, I think it's illustrative to show what features they exercise.

For type 5 tests, anything that uses rustc_xxx crates can be ignored, it will be too much of a headache to get bootstrap to give them the proper sysroot with rustc_private available. Happy to help you get the fancy type issues working :)