Skip to content

Doctest doesn't print valid assert file line and column location link #126251

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

Open
x4exr opened this issue Jun 11, 2024 · 10 comments
Open

Doctest doesn't print valid assert file line and column location link #126251

x4exr opened this issue Jun 11, 2024 · 10 comments
Labels
A-doctests Area: Documentation tests, run by rustdoc C-enhancement Category: An issue proposing an enhancement or a PR with one. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@x4exr
Copy link
Contributor

x4exr commented Jun 11, 2024

I tried this code:

//! Tools for encoding and decoding instructions into and from an intuitive structure.
//! 
//! ERROR HAPPENS ON THIS LINE??: The instruction encoding and decoding format involved an intermediate format. Instructions involve 2 mandatory 
//! driver b.... 

struct SomethingToTakeLines {
// a // Rust says error happend here or so
// b
}

impl Driver {
	/// TODO: Implement test
	/// ```
	/// assert!(false); // Target should be here. 
	/// ```
	pub fn from_encoded(bytes: [u8; 2]) -> Self { ... }
}

I expected to see this happen:
I expected a link to line 1 of the doctest

Instead, this happened:
Line 3 of the entire parent file.

assertion failed: false
thread 'main' panicked at src\processor\src\instruction.rs:3:1:

Meta

rustc --version --verbose:

rustc 1.77.0-nightly (e51e98dde 2023-12-31)
binary: rustc
commit-hash: e51e98dde6a60637b6a71b8105245b629ac3fe77
commit-date: 2023-12-31
host: x86_64-pc-windows-msvc
release: 1.77.0-nightly
LLVM version: 17.0.6

Backtrace

"C:/Users/Rayyan Khan/.cargo/bin/cargo.exe" test --color=always --doc "instruction::from_encoded\ (line\ 69)" --no-fail-fast --manifest-path "C:\Users\Rayyan Khan\Desktop\The Four Elements\Processor\src\processor\Cargo.toml" -- --format=json -Z unstable-options --show-output
Testing started at 11:34 PM ...
warning: virtual workspace defaulting to `resolver = "1"` despite one or more workspace members being on edition 2021 which implies `resolver = "2"`
note: to keep the current resolver, specify `workspace.resolver = "1"` in the workspace root's manifest
note: to use the edition 2021 resolver, specify `workspace.resolver = "2"` in the workspace root's manifest
note: for more details see https://doc.rust-lang.org/cargo/reference/resolver.html#resolver-versions
   Compiling atln-processor v0.1.0 (C:\Users\Rayyan Khan\Desktop\The Four Elements\Processor\src\processor)
warning: unused variable: `memory`
  --> src\processor\src\lib.rs:41:52
   |
41 |     pub fn execute(&mut self, stream: &mut impl Read, memory: &mut Memory) -> Result<(), ExecuteError> {
   |                                                       ^^^^^^ help: if this is intentional, prefix it with an underscore: `_memory`
   |
   = note: `#[warn(unused_variables)]` on by default

warning: `atln-processor` (lib) generated 1 warning (run `cargo fix --lib -p atln-processor` to apply 1 suggestion)
    Finished test [unoptimized + debuginfo] target(s) in 1.12s
   Doc-tests atln-processor
Test executable failed (exit code: 101).

stderr:

assertion failed: false
thread 'main' panicked at src\processor\src\instruction.rs:3:1:
assertion failed: false
stack backtrace:
   0: std::panicking::begin_panic_handler
             at /rustc/e51e98dde6a60637b6a71b8105245b629ac3fe77/library\std\src\panicking.rs:645
   1: core::panicking::panic_fmt
             at /rustc/e51e98dde6a60637b6a71b8105245b629ac3fe77/library\core\src\panicking.rs:72
   2: core::panicking::panic
             at /rustc/e51e98dde6a60637b6a71b8105245b629ac3fe77/library\core\src\panicking.rs:144
   3: std::rt::lang_start
   4: std::rt::lang_start
   5: std::rt::lang_start
   6: __ImageBase
   7: std::rt::lang_start
   8: std::rt::lang_start_internal::closure$2
             at /rustc/e51e98dde6a60637b6a71b8105245b629ac3fe77/library\std\src\rt.rs:148
   9: std::panicking::try::do_call
             at /rustc/e51e98dde6a60637b6a71b8105245b629ac3fe77/library\std\src\panicking.rs:552
  10: std::panicking::try
             at /rustc/e51e98dde6a60637b6a71b8105245b629ac3fe77/library\std\src\panicking.rs:516
  11: std::panic::catch_unwind
             at /rustc/e51e98dde6a60637b6a71b8105245b629ac3fe77/library\std\src\panic.rs:142
  12: std::rt::lang_start_internal
             at /rustc/e51e98dde6a60637b6a71b8105245b629ac3fe77/library\std\src\rt.rs:148
  13: std::rt::lang_start
  14: main
  15: invoke_main
             at D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78
  16: __scrt_common_main_seh
             at D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
  17: BaseThreadInitThunk
  18: RtlUserThreadStart
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.


error: doctest failed, to rerun pass `--doc`
error: 1 target failed:
    `--doc`

Process finished with exit code 101

@x4exr x4exr added the C-bug Category: This is a bug. label Jun 11, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jun 11, 2024
@x4exr
Copy link
Contributor Author

x4exr commented Jun 11, 2024

This seems low priority. I value this feature though, so I'm willing to contribute to fixing this my self. But Id like pointers in knowing where to find the code relating to printing out file links. Like which repository.

@jieyouxu jieyouxu added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. C-enhancement Category: An issue proposing an enhancement or a PR with one. A-doctests Area: Documentation tests, run by rustdoc and removed C-bug Category: This is a bug. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jun 11, 2024
@workingjubilee
Copy link
Member

This seems low priority. I value this feature though, so I'm willing to contribute to fixing this my self. But Id like pointers in knowing where to find the code relating to printing out file links. Like which repository.

uh, rust-lang/rust?

@workingjubilee
Copy link
Member

workingjubilee commented Jun 11, 2024

aiui doctests are actually extracted into a new file by librustdoc and then compiled, with no awareness of what their line/column was in the original file and thus no ability to offset them. you would have to probably poke at https://github.com/rust-lang/rust/blob/aec67e238d366c4c41373b272f19dd79ff5ec0f0/src/librustdoc/doctest.rs at minimum to address this.

( initially also said https://github.com/rust-lang/rust/blob/aec67e238d366c4c41373b272f19dd79ff5ec0f0/compiler/rustc_builtin_macros/src/source_util.rs but I think that's a red herring, now that I've reviewed the relevant assert! code. )

@x4exr
Copy link
Contributor Author

x4exr commented Jun 11, 2024

By offset I meant I could probably first figure out what line the doctest itself is in the source file, then take that line number and add to what ever the current runner outputs. Seems simple enough? Or are there other things I need to worry about. My IDE seems to be able to figure out where a doctest lies (RustRover) assuming it is using some rust compiler API.

@workingjubilee
Copy link
Member

By offset I meant I could probably first figure out what line the doctest itself is in the source file, then take that line number and add to what ever the current runner outputs. Seems simple enough?

The output is from the assert inside the compiled binary.

@workingjubilee
Copy link
Member

The runner doesn't know anything, it is only repeating this from the binary:

#[stable(feature = "panic_hook_display", since = "1.26.0")]
impl fmt::Display for PanicInfo<'_> {
fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
formatter.write_str("panicked at ")?;
self.location.fmt(formatter)?;
formatter.write_str(":\n")?;
formatter.write_fmt(self.message)?;
Ok(())
}
}

It gets the location from here:

pub const fn caller() -> &'static Location<'static> {
crate::intrinsics::caller_location()
}

This is generated by compiler magic, about... here?

pub fn caller_location_ty(self) -> Ty<'tcx> {

But I believe the interesting code that actually determines the spans to output is here:

pub fn caller_location_span<T>(

Ideally we'd somehow convince rustdoc to use the original source spans while compiling this.

@workingjubilee
Copy link
Member

oh, goofier solution:

during extraction, insert sufficient newlines.

@x4exr
Copy link
Contributor Author

x4exr commented Jun 12, 2024

That seems be a good idea

@tbu-
Copy link
Contributor

tbu- commented Jun 12, 2024

oh, goofier solution:

during extraction, insert sufficient newlines.

Might not work if the test is sufficiently close to the top of the file, I think.

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-enhancement Category: An issue proposing an enhancement or a PR with one. 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

5 participants