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

doctests marked with 'no_run' permit ICEs #49514

Open
frewsxcv opened this issue Mar 30, 2018 · 7 comments
Open

doctests marked with 'no_run' permit ICEs #49514

frewsxcv opened this issue Mar 30, 2018 · 7 comments
Labels
A-doctests Area: Documentation tests, run by rustdoc C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@frewsxcv
Copy link
Member

frewsxcv commented Mar 30, 2018

irc discussion: https://botbot.me/mozilla/rust-docs/2018-03-30/?msg=98430464&page=1

run this example: https://doc.rust-lang.org/nightly/std/fs/fn.read_string.html

at the time of writing it ICEs, but doctests don't catch it because it's marked no_run

@frewsxcv frewsxcv changed the title doctests marked with 'ignore' permits ICEs doctests marked with 'no_run' permits ICEs Mar 30, 2018
@frewsxcv frewsxcv changed the title doctests marked with 'no_run' permits ICEs doctests marked with 'no_run' permit ICEs Mar 30, 2018
@ollie27
Copy link
Member

ollie27 commented Mar 30, 2018

rustdoc stops compilation after analysis for no_run

rust/src/librustdoc/test.rs

Lines 261 to 263 in f1c21b0

if no_run {
control.after_analysis.stop = Compilation::Stop;
}
and that ICE looks like it's happening during trans.

@QuietMisdreavus QuietMisdreavus added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. and removed T-rustdoc labels May 14, 2018
@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label May 29, 2018
@QuietMisdreavus
Copy link
Member

@rust-lang/rustdoc What do y'all think? Should we try to compile no_run doctests to completion, or is this issue moot?

@GuillaumeGomez
Copy link
Member

Sounds weird. no_run should check the whole code but I don't see the point to generate any binary.

@jyn514 jyn514 added the A-doctests Area: Documentation tests, run by rustdoc label Nov 4, 2020
@jyn514
Copy link
Member

jyn514 commented Nov 6, 2020

I don't see much point in compiling these. The tradeoff "you occasionally discover bugs in the compiler and can't run your tests" doesn't seem worth "your tests always take longer to run".

@jyn514 jyn514 removed the I-nominated label Nov 6, 2020
@jyn514
Copy link
Member

jyn514 commented Dec 30, 2020

@Manishearth suggested adding an attribute that means "compile fully, but don't run" (e.g. build).

@Manishearth
Copy link
Member

Thinking more about it I don't actually think people will want to use that, but what people might want is to have security in knowing their code isn't ice prone.

Which ... Most of the causes for ICEs specific to a certain library's code are typically related to analysis, so maybe this is fine. I'm not sure.

@ehuss
Copy link
Contributor

ehuss commented Dec 31, 2020

FWIW, this is more than just about ICEs. Any errors that only appear after analysis will not be caught. There is the opposite problem with the compile_fail attribute, where it halts compilation too early, so it can't actually catch all errors (see https://github.com/rust-lang/reference/blame/6d6debcbabdeae109e086f7db6374fcd6d7a334b/src/attributes/limits.md#L45-L58 as an example).

For example, this passes, but the code doesn't compile:

//! ```no_run
//! let a: u8 = 42;
//! let b = ((a | 0x0F) - 1) >> 31 > 0;
//! println!("{}", b);
//! ```

This is a general problem with "check" style compilation, so it is not necessarily unique to rustdoc, but it does cause confusion when it doesn't work. See #49292 and #70923 and all the duplicates linked to those for examples of code that can check but not build.

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. 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

8 participants