cargo miri test does not run doc-tests

[RalfJung]

I am not sure what it would take to also make it build and execute doc-tests. Probably just some tweaks in how we wrap cargo rustc?

[bjorn3]

Doc tests need rustdoc as driver, while miri needs itself as driver.

[SimonSapin]

@rust-lang/rustdoc, can rustdoc be used as a library (not a driver) to extract the doctests without running them?

[RalfJung]

Or, alternatively, can we set some environment variable to make rustdoc call miri instead of calling rustc and running the binary on each test after extraction?

[QuietMisdreavus]

@rust-lang/rustdoc, can rustdoc be used as a library (not a driver) to extract the doctests without running them?

Unfortunately, librustdoc was taken out of the sysroot a while back, and i don't think we auto-publish the source for that as a crate either, so there's not really an easy solution for that. Rustdoc itself doesn't really provide functionality to only save off the extracted/converted doctest, either.

Or, alternatively, can we set some environment variable to make rustdoc call miri instead of calling rustc and running the binary on each test after extraction?

Same response - unless the test is manually marked as ignore (or given some non-rust language string) in the source, rustdoc will always run the converted doctest through rustc via the librustc_interface interface. There's an open PR to provide an option to run the compiled binaries through a "test runner", but it doesn't sound like that will help either.

[RalfJung]

How open would the rustdoc team be to provide a way for Miri to run the extracted tests, and what would be the preferred way of achieving that (librustdoc vs controlling what happens after extraction)?

For me it seems like the second option is the simpler one.

[bjorn3]

The compilation of tests seems to be performed at https://github.com/rust-lang/rust/blob/17e62f77f954bed97aae839624bfd6dd68342daf/src/librustdoc/test.rs#L181.

[RalfJung]

Yeah, now I see what @QuietMisdreavus meant by librustc_interface -- it does not call the rustc binary, it calls the rustc library.

The "easiest" way to get Miri into this would be to provide a way to call some binary instead, but I am not sure if that is feasible.

[QuietMisdreavus]

If there were some flag we could provide to just save off all the doctests as standalone rust files, would that be enough to allow miri to attempt to build them? They're meant to be compiled as binaries and link against the regular crate and its dependency tree.

[RalfJung]

If there were some flag we could provide to just save off all the doctests as standalone rust files, would that be enough to allow miri to attempt to build them?

I guess we could make cargo miri test pick them up and run them all through Miri. The hard part would be to figure out the right command-line flags to get all the dependencies to work, but I am not sure if rustdoc can even help there?

[QuietMisdreavus]

Cargo hands rustdoc all the --extern flags it needs when running rustdoc --test from cargo test, so presumably that information would be present in the crate's metadata.

[RalfJung]

Hm, that raises the interesting question how cargo miri test would invoke rustdoc. Probably through cargo test? Can we do that targeting just the doctests of a particular binary?

Currently I wouldn't know how cargo miri test can get those flags; for the binaries we run we get them from cargo as well (cargo miri sets itself into the RUSTC env var so that cargo calls it with all the right flags and it just has to forward them).

[nox]

Does rustdoc reads RUSTC? If so, could we make miri compile a small executable that runs the test under miri indirectly so that the changes to rustdoc are minimal?

[RalfJung]

AFAIK rustdoc uses the rustc driver interface directly, it does not run a binary.

[nox]

Oh right, I actually read that earlier in the thread today, but forgot about it now that it's Friday evening… My bad.

[RalfJung]

With rust-lang/rust#63827, rustdoc now runs rustc out-of-process. That should make it easier to run Miri instead.

@andjo403 is there a way to use an env var or so to control the binary that is run?

[andjo403]

not yet working on a new PR where RUSTC env var can be set but have some test to fix

[andjo403]

@Mark-Simulacrum added the --test-builder option in rust-lang/rust#64328 for using other binary and it is merged now

[WaffleLapkin]

Is it possible now to somehow trick miri to run doctests?

[RalfJung]

rustdoc probably lets us do that these days, but things have not been wired up in Miri yet.

[teryror]

Is anyone actively working on this?

I'd be willing to invest a couple of afternoons over the coming week or two, but I could definitely use some pointers on how to get started. I never touched any of the involved projects before. I did read the contributor's guide, though.

Based on what's been said here and what I gathered from reading code, this would basically boil down to replacing this error message with a call to a new function phase_cargo_rustdoc, pretty much without changing anything else. Is that assessment correct?

The problem is I'm not clear on what that function should do exactly. Invoke rustdoc --test --test-builder {?}, then the executable that produces, presumably?

[RalfJung]

Is anyone actively working on this?

No, not that I know of.

I'd be willing to invest a couple of afternoons over the coming week or two, but I could definitely use some pointers on how to get started. I never touched any of the involved projects before. I did read the contributor's guide, though.

That's great. :) However, I'm afraid there's not a ton of guidance available on this issue -- part of the problem here is even figuring out how to best do this.

Based on what's been said here and what I gathered from reading code, this would basically boil down to replacing this error message with a call to a new function phase_cargo_rustdoc, pretty much without changing anything else. Is that assessment correct?

Yes, that sounds right.

The problem is I'm not clear on what that function should do exactly. Invoke rustdoc --test --test-builder {?}, then the executable that produces, presumably?

I don't know either what exactly it should do. ;) Probably the first step is to figure out the interaction between rustdoc and cargo, which is the thing that we are "hooking" here. Does cargo call rustdoc to build some binaries, and then the binaries are run by cargo, or does cargo run rustdoc and rustdoc runs some binaries? What kind of command-line arguments does cargo pass to rustdoc, and does Miri have to patch them in any way (besides adding --test-builder)? There will be some exploration and research needed to figure this out, and indeed that exploration and research is the largest part of the work that needs to be done to resolve this issue.

[teryror]

Alright. I've skimmed what seemed to be the relevant parts of cargo, I guess I'll have to take a closer look then.

Thanks for the guiding questions, that's already pretty helpful!

[RalfJung]

Awesome. :) Don't hesitate to ask if you have further questions. (But also, I might not always be able to answer so quickly, so when delays happen that's just because I am busy.)

Judging from --test-builder, I think rustdoc is only building binaries, not running them. So likely Miri should make rustdoc invoke miri as the builder and make sure that this rustdoc-invoked Miri enters phase_cargo_rustc. Assuming this is always a binary crate, this would make Miri emit a JSON file as the "binary" with all the required information that is needed later when cargo tells Miri to "run" that JSON file.

[teryror]

Status report!

I spent some time reading the source to librustdoc, and it looks to me like you were mistaken. After spawning a test_builder process to produce an output_file, rustdoc tries to execute it. However, it also has the --runtool and --runtool-arg options. If a run tool is provided, it invokes

$runtool ($runtool-arg)* $output_file

instead. The run tool should be the driver miri, rather than cargo-miri, correct? Under that assumption, here's a naive draft:

I can't test this quite yet, because the --test-builder option appears to be broken. I'll try forking the rust repo later today so I might give this a shot. I was still hoping I wouldn't have to do that, but c'est la vie.

EDIT: Ok, I have a local build of rustdoc that I can actually hook into. Turns out my naive draft didn't work, no big surprise there.

The way rustdoc invokes the test builder is indistinguishable from the way cargo invokes rustdoc, at least from the command line arguments. It does, however, set some UNSTABLE_ environment variables we can check for. Not a great thing to depend on, but it'll do, I suppose. The issue I'm facing now is that rustdoc passes the generated code to rustc via stdin, rather than a temporary file. I can't think of a simple way to trick cargo-miri into just dealing with that.

Does the miri driver support piping code in like that? If so, I could make phase_cargo_rustc record the contents of stdin in the temporary JSON file, and make phase_cargo_runner play them back, so to speak. Alternatively, I could add two more phases, phase_rustdoc_rustc and phase_rustdoc_runner, respectively. Thoughts, @RalfJung?

[RalfJung]

I spent some time reading the source to librustdoc, and it looks to me like you were mistaken. After spawning a test_builder process to produce an output_file, rustdoc tries to execute it. However, it also has the --runtool and --runtool-arg options.

Ah, interesting -- good find.
This will mean more work for you.^^ Yes, the runtool will have to be Miri, but it will need all the arguments that were passed to --test-builder. That's the same issue as with regular cargo builds and running them; it's why the builds create JSON files -- then when running them we have all the aguments available that were passed for the build.

The way rustdoc invokes the test builder is indistinguishable from the way cargo invokes rustdoc, at least from the command line arguments. It does, however, set some UNSTABLE_ environment variables we can check for. Not a great thing to depend on, but it'll do, I suppose.

Well, phase_cargo_rustdoc could set an env var, wouldn't that help?

The issue I'm facing now is that rustdoc passes the generated code to rustc via stdin, rather than a temporary file. I can't think of a simple way to trick cargo-miri into just dealing with that.

Ugh. Yes that will be interesting.^^

Does the miri driver support piping code in like that? If so, I could make phase_cargo_rustc record the contents of stdin in the temporary JSON file, and make phase_cargo_runner play them back, so to speak. Alternatively, I could add two more phases, phase_rustdoc_rustc and phase_rustdoc_runner, respectively.

Miri uses rustc for input file handling, so stdin should just work. It has never been tested though.
I'd prefer reusing existing code as much as possible, i.e., phase_cargo_rustc and phase_cargo_runner should coordinate to forward stdin when necessary. The tricky bit will be to not do this always but only when needed; in the "normal" case we'll still want to use the normal stdin for running Miri so that the interpreted program can read from stdin. I guess this could either be smart and figure things out based on the rustc arguments, or phase_cargo_rustdoc could set an env var which says that stdin should be handled this way.

[bjorn3]

Ugh. Yes that will be interesting.^^

It also sets a few env vars to fixup the source location shown in error messages.

[teryror]

It also sets a few env vars to fixup the source location shown in error messages.

You mean UNSTABLE_RUSTDOC_TEST_PATH and UNSTABLE_RUSTDOC_TEST_LINE? That's what I'm checking for :)

Well, phase_cargo_rustdoc could set an env var, wouldn't that help?

That would probably help in orchestrating the other phases, but I'm testing the UNSTABLE_* vars to see if I should call phase_cargo_rustdoc in the first place.

Say if MIRI_CALLED_FROM_RUSTDOC is set, phase_cargo_rustc reads all of stdin and dumps it into the JSON file, and phase_cargo_runner just needs to know about the optional field. Seems like a decent plan, no?

I'll give that a shot later today, but first I'll have to add a regression test to rustdoc to get my PR accepted :)

[RalfJung]

That would probably help in orchestrating the other phases, but I'm testing the UNSTABLE_* vars to see if I should call phase_cargo_rustdoc in the first place.

Why is that needed? This case distinction already exists in master (to emit the error message).

Say if MIRI_CALLED_FROM_RUSTDOC is set, phase_cargo_rustc reads all of stdin and dumps it into the JSON file, and phase_cargo_runner just needs to know about the optional field. Seems like a decent plan, no?

Yes that works.

[teryror]

Why is that needed? This case distinction already exists in master (to emit the error message).

It's not, you're right, my mistake. I got confused for a second there. This should work perfectly fine then.

[teryror]

I seem to have a mostly working prototype! See the full code here and the diff here.

MIRI_VERBOSE output is swallowed for passing tests, of course, but purposefully failing one shows that phase_cargo_rustc and phase_cargo_runner are indeed called appropriately, and the test output matches my expectation. Miri seems to be dealing with piped input just fine.

Problem is that compile_fail tests all fail due to being erroneously compiled without error, I'm not sure why that is yet. Actually, I know what the problem is: I'm not piping the test code into that Miri invocation yet! I'll still call it a day for now, though.

EDIT: Ugh, it's a bit more complicated than that. The issue is the early return after writing the JSON file in phase_cargo_rustc. No checking takes place there, but rustdoc expects the test-builder process to exit with an error code for a compile_fail test, in which case the runtool is never invoked. I could either add some special case logic there or try to rewire a good chunk of the function without breaking anything.

EDIT 2: I went with a special case, for now. I figure it's easier to refactor later once I have a solution figured out.

I can't seem to get the check-only build to work though, Miri keeps complaining about not being able to find the library crate the doc tests came from. I think that's because cargo-miri creates stub *.rlib files for that, but I'm not sure that's the issue, and even if it is, I have no idea what to do about it. I'm already patching the --emit arguments like the existing code does (removing link and adding metadata), and I tried patching the --extern arguments to use *.rmeta instead (like the test runner, which doesn't have any issues finding the crate), but none of that helped. I'm out of ideas here, so uh... suggestions?

@RalfJung? (not sure if you see the edits if I don't mention you)

EDIT 3: Okay, patching the --extern arguments in phase_cargo_rustdoc seems to get me somewhere. Normal tests still work, if I comment out the check-only build in phase_cargo_rustc, but leaving that enabled now causes an ICE instead. Just one example from my library coca:

[cargo-miri rustc] "/home/tld/.cargo/bin/miri" "--sysroot" "/home/tld/.cache/miri/HOST" "--crate-type" "bin"
"--cfg" "feature=\"default\"" "--edition" "2018" "-o" "/tmp/rustdoctestnmsoFZ/rust_out"
"-L" "dependency=/home/tld/Documents/coca/target/x86_64-unknown-linux-gnu/debug/deps"
"-L" "dependency=/home/tld/Documents/coca/target/debug/deps"
"--extern" "coca=/home/tld/Documents/coca/target/x86_64-unknown-linux-gnu/debug/deps/libcoca-8671386d903c290f.rmeta"
"--extern" "rand=/home/tld/Documents/coca/target/x86_64-unknown-linux-gnu/debug/deps/librand-af6e186424c4447e.rmeta"
"-Ccodegen-units=1" "-C" "embed-bitcode=no" "-Z" "unstable-options" "--target" "x86_64-unknown-linux-gnu" "--color" "always" "-"
error: internal compiler error: compiler/rustc_mir/src/monomorphize/collector.rs:826:9:
cannot create local mono-item for DefId(2:6795 ~ core[1258]::fmt::num::DEC_DIGITS_LUT)

thread 'rustc' panicked at 'Box<Any>', compiler/rustc_errors/src/lib.rs:958:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: rustc 1.51.0-nightly (7bb163095 2021-01-14) running on x86_64-unknown-linux-gnu

note: compiler flags: -Z unstable-options -C codegen-units=1 -C embed-bitcode=no --crate-type bin

query stack during panic:
#0 [collect_and_partition_mono_items] collect_and_partition_mono_items
end of query stack
error: aborting due to previous error

They all hit this exact same error, down to the DefId(2:6795 ~ core[1258]::fmt::num::DEC_DIGITS_LUT). I don't see how this could be related to the specific code the command is fed, but I'll have to investigate what's actually causing this.

Now that I think about it, this might just be because I switched from my local rustc/rustdoc build to the newest nightly, and Miri just doesn't work with that yet.

[teryror]

I got it to work! 🎉

I was missing an --emit=dep-info,metadata argument to the check-only build. The ICE still should be considered a bug, though, right?

I went ahead and opened a PR :D

[RalfJung]

That's awesome. :) Thanks a lot for your work @teryror!

[cmazakas]

I'm having trouble getting Miri to ignore certain doctests. Namely, I have some doctests that use SIMD.

  /// # Example
  /// ```
  /// #[cfg_attr(miri, ignore)]
  /// fn main() {

The above doctest winds up getting run anyway. Is there any method used to ignore a doctest?

[RalfJung]

I don't think ignore does anything for doctests.

Did you try #[cfg(not(miri))]?

[cmazakas]

Ah, perfect! That works, thank you!