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.

Sign up for GitHub

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

Jump to bottom

Allow cross-compiling doctests #60387

Merged
merged 5 commits into from
Sep 10, 2019
Merged

Allow cross-compiling doctests #60387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
98bd8fd
Added ability to crosscompile doctests
Goirad Apr 26, 2019
3f76408
added feature gate enable-per-target-ignores
Goirad Jun 6, 2019
657e24c
changed target from option to plain target, populated with host tripl…
Goirad Jun 11, 2019
14110eb
added rustdoc book documentation, improved behavior when unstable fla…
Goirad Jun 12, 2019
4a2094c
address rebase changes
Goirad Aug 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions src/doc/rustdoc/src/unstable-features.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,3 +471,53 @@ Some methodology notes about what rustdoc counts in this metric:

Public items that are not documented can be seen with the built-in `missing_docs` lint. Private
items that are not documented can be seen with Clippy's `missing_docs_in_private_items` lint.

### `--enable-per-target-ignores`: allow `ignore-foo` style filters for doctests

Using this flag looks like this:

```bash
$ rustdoc src/lib.rs -Z unstable-options --enable-per-target-ignores
```

This flag allows you to tag doctests with compiltest style `ignore-foo` filters that prevent
rustdoc from running that test if the target triple string contains foo. For example:

```rust
///```ignore-foo,ignore-bar
///assert!(2 == 2);
///```
struct Foo;
```

This will not be run when the build target is `super-awesome-foo` or `less-bar-awesome`.
If the flag is not enabled, then rustdoc will consume the filter, but do nothing with it, and
the above example will be run for all targets.
If you want to preserve backwards compatibility for older versions of rustdoc, you can use

```rust
///```ignore,ignore-foo
///assert!(2 == 2);
///```
struct Foo;
```

In older versions, this will be ignored on all targets, but on newer versions `ignore-gnu` will
override `ignore`.

### `--runtool`, `--runtool-arg`: program to run tests with; args to pass to it

Using thses options looks like this:

```bash
$ rustdoc src/lib.rs -Z unstable-options --runtool runner --runtool-arg --do-thing --runtool-arg --do-other-thing
```

These options can be used to run the doctest under a program, and also pass arguments to
that program. For example, if you want to run your doctests under valgrind you might run

```bash
$ rustdoc src/lib.rs -Z unstable-options --runtool valgrind
```

Another use case would be to run a test inside an emulator, or through a Virtual Machine.
25 changes: 22 additions & 3 deletions src/librustdoc/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc::session;
use rustc::session::config::{CrateType, parse_crate_types_from_list};
use rustc::session::config::{CodegenOptions, DebuggingOptions, ErrorOutputType, Externs};
use rustc::session::config::{nightly_options, build_codegen_options, build_debugging_options,
get_cmd_lint_options, ExternEntry};
get_cmd_lint_options, host_triple, ExternEntry};
use rustc::session::search_paths::SearchPath;
use rustc_driver;
use rustc_target::spec::TargetTriple;
Expand Down Expand Up @@ -54,7 +54,7 @@ pub struct Options {
/// Debugging (`-Z`) options to pass to the compiler.
pub debugging_options: DebuggingOptions,
/// The target used to compile the crate against.
pub target: Option<TargetTriple>,
pub target: TargetTriple,
/// Edition used when reading the crate. Defaults to "2015". Also used by default when
/// compiling doctests from the crate.
pub edition: Edition,
Expand All @@ -77,6 +77,14 @@ pub struct Options {
/// Optional path to persist the doctest executables to, defaults to a
/// temporary directory if not set.
pub persist_doctests: Option<PathBuf>,
/// Runtool to run doctests with
pub runtool: Option<String>,
/// Arguments to pass to the runtool
pub runtool_args: Vec<String>,
/// Whether to allow ignoring doctests on a per-target basis
/// For example, using ignore-foo to ignore running the doctest on any target that
/// contains "foo" as a substring
pub enable_per_target_ignores: bool,

// Options that affect the documentation process

Expand Down Expand Up @@ -140,6 +148,9 @@ impl fmt::Debug for Options {
.field("show_coverage", &self.show_coverage)
.field("crate_version", &self.crate_version)
.field("render_options", &self.render_options)
.field("runtool", &self.runtool)
.field("runtool_args", &self.runtool_args)
.field("enable-per-target-ignores", &self.enable_per_target_ignores)
.finish()
}
}
Expand Down Expand Up @@ -414,7 +425,9 @@ impl Options {
}
}

let target = matches.opt_str("target").map(|target| {
let target = matches.opt_str("target").map_or(
TargetTriple::from_triple(host_triple()),
|target| {
if target.ends_with(".json") {
TargetTriple::TargetPath(PathBuf::from(target))
} else {
Expand Down Expand Up @@ -466,6 +479,9 @@ impl Options {
let codegen_options_strs = matches.opt_strs("C");
let lib_strs = matches.opt_strs("L");
let extern_strs = matches.opt_strs("extern");
let runtool = matches.opt_str("runtool");
let runtool_args = matches.opt_strs("runtool-arg");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what @jethrogb was talking about with the behavior of the runtool arguments. This just takes arguments as-is without splitting or reparsing them, which (apparently) is what compiletest does. A more robust solution would split the strings we get from opt_strs to fill out a separate Vec, rather than taking the list directly.

...however, after reviewing your companion PR on Cargo, it seems like you're going to lean on Cargo pre-parsing the runner arguments, so i'm not too concerned, in the end. I'd like to see whether/how this is going to be integrated with bootstrap - do we cross-compile other tests already? How is that orchestrated? (Who is the best person to ask these questions to? 😅)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused by your first paragraph, it is my understanding that opt_strs returns a vec of all arguments passed to (possibly many) --runtool-arg's. The intention being that arguments get passed one at a time, one per --runtool-arg.

As for the question of integration with bootstrap, I am working on that now, but wanted to get the ball rolling here in rustdoc. My plan is to add an unstable flag to bootstrap that makes it pass --Zdoctest-xcompile to cargo test invocations. The current state is that compiletests do get cross-compiled, and it already has a --runtool option. My change to compiletest removes that option and instead uses Cargo to create the merged .cargo/config for a directory and use the runner specified there, with the idea being that it will ensure consistent testing across rustdoc and compiltest.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see now what you meant in your first paragraph. You're right it would be more robust, I just omitted any error checking there, since worst case scenario one string that is in reality several instructions would just get passed wholesale to the runtool (which I think would not in the end be any different from adding them one at a time).
Also as far as who to ask, I don't know, but I welcome any suggestions since this seems to be pretty central to how the whole rust compiler building and testing process works.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(which I think would not in the end be any different from adding them one at a time)

Since we would be building up a Command instance with these args, the space-splitting wouldn't happen - the recipient would receive the blob in one "argument", as if it had all been quoted together.

One option would be to look at how many --runtool-arg arguments there are and treat it differently based on whether there are one or more than one. If there's only one argument, we could do space-splitting on that, and if there is more than one argument, we can leave them as-is. This somewhat mirrors Cargo's own parsing of the relevant configuration item, which whitespace-splits it is it's just a string, and leaves them alone if given a list. (This can backfire if someone tried to give a single argument with spaces in it, though... 🤔)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not possible to correctly space-split command-line arguments based on some heuristic if you also want to support arguments with spaces in them. Just let the cargo/the user take care of this. Most people won't be running rustdoc directly this way anyway.

let enable_per_target_ignores = matches.opt_present("enable-per-target-ignores");

let (lint_opts, describe_lints, lint_cap) = get_cmd_lint_options(matches, error_format);

Expand Down Expand Up @@ -496,6 +512,9 @@ impl Options {
show_coverage,
crate_version,
persist_doctests,
runtool,
runtool_args,
enable_per_target_ignores,
render_options: RenderOptions {
output,
external_html,
Expand Down
4 changes: 1 addition & 3 deletions src/librustdoc/core.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use rustc_interface::interface;
use rustc_driver::abort_on_err;
use rustc_resolve as resolve;
use rustc_metadata::cstore::CStore;
use rustc_target::spec::TargetTriple;

use syntax::source_map;
use syntax::attr;
Expand Down Expand Up @@ -294,7 +293,6 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt
}
}).collect();

let host_triple = TargetTriple::from_triple(config::host_triple());
let crate_types = if proc_macro_crate {
vec![config::CrateType::ProcMacro]
} else {
Expand All @@ -313,7 +311,7 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt
lint_cap: Some(lint_cap.unwrap_or_else(|| lint::Forbid)),
cg: codegen_options,
externs,
target_triple: target.unwrap_or(host_triple),
target_triple: target,
// Ensure that rustdoc works even if rustc is feature-staged
unstable_features: UnstableFeatures::Allow,
actually_rustdoc: true,
Expand Down
43 changes: 32 additions & 11 deletions src/librustdoc/html/markdown.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for CodeBlocks<'_, 'a, I> {
let ignore;
let edition;
if let Some(Event::Start(Tag::CodeBlock(lang))) = event {
let parse_result = LangString::parse(&lang, self.check_error_codes);
let parse_result = LangString::parse(&lang, self.check_error_codes, false);
if !parse_result.rust {
return Some(Event::Start(Tag::CodeBlock(lang)));
}
Expand Down Expand Up @@ -272,7 +272,7 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for CodeBlocks<'_, 'a, I> {
))
});

let tooltip = if ignore {
let tooltip = if ignore != Ignore::None {
Some(("This example is not tested".to_owned(), "ignore"))
} else if compile_fail {
Some(("This example deliberately fails to compile".to_owned(), "compile_fail"))
Expand All @@ -286,7 +286,7 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for CodeBlocks<'_, 'a, I> {
s.push_str(&highlight::render_with_highlighting(
&text,
Some(&format!("rust-example-rendered{}",
if ignore { " ignore" }
if ignore != Ignore::None { " ignore" }
else if compile_fail { " compile_fail" }
else if explicit_edition { " edition " }
else { "" })),
Expand All @@ -297,7 +297,7 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for CodeBlocks<'_, 'a, I> {
s.push_str(&highlight::render_with_highlighting(
&text,
Some(&format!("rust-example-rendered{}",
if ignore { " ignore" }
if ignore != Ignore::None { " ignore" }
else if compile_fail { " compile_fail" }
else if explicit_edition { " edition " }
else { "" })),
Expand Down Expand Up @@ -551,7 +551,8 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for Footnotes<'a, I> {
}
}

pub fn find_testable_code<T: test::Tester>(doc: &str, tests: &mut T, error_codes: ErrorCodes) {
pub fn find_testable_code<T: test::Tester>(doc: &str, tests: &mut T, error_codes: ErrorCodes,
enable_per_target_ignores: bool) {
let mut parser = Parser::new(doc);
let mut prev_offset = 0;
let mut nb_lines = 0;
Expand All @@ -564,7 +565,7 @@ pub fn find_testable_code<T: test::Tester>(doc: &str, tests: &mut T, error_codes
let block_info = if s.is_empty() {
LangString::all_false()
} else {
LangString::parse(&*s, error_codes)
LangString::parse(&*s, error_codes, enable_per_target_ignores)
};
if !block_info.rust {
continue;
Expand Down Expand Up @@ -607,7 +608,7 @@ pub struct LangString {
original: String,
pub should_panic: bool,
pub no_run: bool,
pub ignore: bool,
pub ignore: Ignore,
pub rust: bool,
pub test_harness: bool,
pub compile_fail: bool,
Expand All @@ -616,13 +617,20 @@ pub struct LangString {
pub edition: Option<Edition>
}

#[derive(Eq, PartialEq, Clone, Debug)]
pub enum Ignore {
All,
None,
Some(Vec<String>),
}

impl LangString {
fn all_false() -> LangString {
LangString {
original: String::new(),
should_panic: false,
no_run: false,
ignore: false,
ignore: Ignore::None,
rust: true, // NB This used to be `notrust = false`
test_harness: false,
compile_fail: false,
Expand All @@ -632,11 +640,16 @@ impl LangString {
}
}

fn parse(string: &str, allow_error_code_check: ErrorCodes) -> LangString {
fn parse(
string: &str,
allow_error_code_check: ErrorCodes,
enable_per_target_ignores: bool
) -> LangString {
let allow_error_code_check = allow_error_code_check.as_bool();
let mut seen_rust_tags = false;
let mut seen_other_tags = false;
let mut data = LangString::all_false();
let mut ignores = vec![];

data.original = string.to_owned();
let tokens = string.split(|c: char|
Expand All @@ -651,7 +664,11 @@ impl LangString {
seen_rust_tags = seen_other_tags == false;
}
"no_run" => { data.no_run = true; seen_rust_tags = !seen_other_tags; }
"ignore" => { data.ignore = true; seen_rust_tags = !seen_other_tags; }
"ignore" => { data.ignore = Ignore::All; seen_rust_tags = !seen_other_tags; }
x if x.starts_with("ignore-") => if enable_per_target_ignores {
ignores.push(x.trim_start_matches("ignore-").to_owned());
seen_rust_tags = !seen_other_tags;
}
"allow_fail" => { data.allow_fail = true; seen_rust_tags = !seen_other_tags; }
"rust" => { data.rust = true; seen_rust_tags = true; }
"test_harness" => {
Expand Down Expand Up @@ -679,6 +696,10 @@ impl LangString {
_ => { seen_other_tags = true }
}
}
// ignore-foo overrides ignore
if !ignores.is_empty() {
data.ignore = Ignore::Some(ignores);
}

data.rust &= !seen_other_tags || seen_rust_tags;

Expand Down Expand Up @@ -919,7 +940,7 @@ crate fn rust_code_blocks(md: &str) -> Vec<RustCodeBlock> {
let lang_string = if syntax.is_empty() {
LangString::all_false()
} else {
LangString::parse(&*syntax, ErrorCodes::Yes)
LangString::parse(&*syntax, ErrorCodes::Yes, false)
};

if lang_string.rust {
Expand Down
44 changes: 23 additions & 21 deletions src/librustdoc/html/markdown/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::{ErrorCodes, LangString, Markdown, MarkdownHtml, IdMap};
use super::{ErrorCodes, LangString, Markdown, MarkdownHtml, IdMap, Ignore};
use super::plain_summary_line;
use std::cell::RefCell;
use syntax::edition::{Edition, DEFAULT_EDITION};
Expand Down Expand Up @@ -26,10 +26,10 @@ fn test_unique_id() {
#[test]
fn test_lang_string_parse() {
fn t(s: &str,
should_panic: bool, no_run: bool, ignore: bool, rust: bool, test_harness: bool,
should_panic: bool, no_run: bool, ignore: Ignore, rust: bool, test_harness: bool,
compile_fail: bool, allow_fail: bool, error_codes: Vec<String>,
edition: Option<Edition>) {
assert_eq!(LangString::parse(s, ErrorCodes::Yes), LangString {
edition: Option<Edition>) {
assert_eq!(LangString::parse(s, ErrorCodes::Yes, true), LangString {
should_panic,
no_run,
ignore,
Expand All @@ -42,6 +42,7 @@ fn test_lang_string_parse() {
edition,
})
}
let ignore_foo = Ignore::Some(vec!("foo".to_string()));

fn v() -> Vec<String> {
Vec::new()
Expand All @@ -50,23 +51,24 @@ fn test_lang_string_parse() {
// ignore-tidy-linelength
// marker | should_panic | no_run | ignore | rust | test_harness
// | compile_fail | allow_fail | error_codes | edition
t("", false, false, false, true, false, false, false, v(), None);
t("rust", false, false, false, true, false, false, false, v(), None);
t("sh", false, false, false, false, false, false, false, v(), None);
t("ignore", false, false, true, true, false, false, false, v(), None);
t("should_panic", true, false, false, true, false, false, false, v(), None);
t("no_run", false, true, false, true, false, false, false, v(), None);
t("test_harness", false, false, false, true, true, false, false, v(), None);
t("compile_fail", false, true, false, true, false, true, false, v(), None);
t("allow_fail", false, false, false, true, false, false, true, v(), None);
t("{.no_run .example}", false, true, false, true, false, false, false, v(), None);
t("{.sh .should_panic}", true, false, false, false, false, false, false, v(), None);
t("{.example .rust}", false, false, false, true, false, false, false, v(), None);
t("{.test_harness .rust}", false, false, false, true, true, false, false, v(), None);
t("text, no_run", false, true, false, false, false, false, false, v(), None);
t("text,no_run", false, true, false, false, false, false, false, v(), None);
t("edition2015", false, false, false, true, false, false, false, v(), Some(Edition::Edition2015));
t("edition2018", false, false, false, true, false, false, false, v(), Some(Edition::Edition2018));
t("", false, false, Ignore::None, true, false, false, false, v(), None);
t("rust", false, false, Ignore::None, true, false, false, false, v(), None);
t("sh", false, false, Ignore::None, false, false, false, false, v(), None);
t("ignore", false, false, Ignore::All, true, false, false, false, v(), None);
t("ignore-foo", false, false, ignore_foo, true, false, false, false, v(), None);
t("should_panic", true, false, Ignore::None, true, false, false, false, v(), None);
t("no_run", false, true, Ignore::None, true, false, false, false, v(), None);
t("test_harness", false, false, Ignore::None, true, true, false, false, v(), None);
t("compile_fail", false, true, Ignore::None, true, false, true, false, v(), None);
t("allow_fail", false, false, Ignore::None, true, false, false, true, v(), None);
t("{.no_run .example}", false, true, Ignore::None, true, false, false, false, v(), None);
t("{.sh .should_panic}", true, false, Ignore::None, false, false, false, false, v(), None);
t("{.example .rust}", false, false, Ignore::None, true, false, false, false, v(), None);
t("{.test_harness .rust}", false, false, Ignore::None, true, true, false, false, v(), None);
t("text, no_run", false, true, Ignore::None, false, false, false, false, v(), None);
t("text,no_run", false, true, Ignore::None, false, false, false, false, v(), None);
t("edition2015", false, false, Ignore::None, true, false, false, false, v(), Some(Edition::Edition2015));
t("edition2018", false, false, Ignore::None, true, false, false, false, v(), Some(Edition::Edition2018));
}

#[test]
Expand Down
17 changes: 17 additions & 0 deletions src/librustdoc/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,23 @@ fn opts() -> Vec<RustcOptGroup> {
"show-coverage",
"calculate percentage of public items with documentation")
}),
unstable("enable-per-target-ignores", |o| {
o.optflag("",
"enable-per-target-ignores",
"parse ignore-foo for ignoring doctests on a per-target basis")
}),
unstable("runtool", |o| {
o.optopt("",
"runtool",
"",
"The tool to run tests with when building for a different target than host")
}),
unstable("runtool-arg", |o| {
o.optmulti("",
"runtool-arg",
"",
"One (of possibly many) arguments to pass to the runtool")
}),
]
}

Expand Down
5 changes: 3 additions & 2 deletions src/librustdoc/markdown.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,12 @@ pub fn test(mut options: Options, diag: &errors::Handler) -> i32 {
opts.no_crate_inject = true;
opts.display_warnings = options.display_warnings;
let mut collector = Collector::new(options.input.display().to_string(), options.clone(),
true, opts, None, Some(options.input));
true, opts, None, Some(options.input),
options.enable_per_target_ignores);
collector.set_position(DUMMY_SP);
let codes = ErrorCodes::from(UnstableFeatures::from_environment().is_nightly_build());

find_testable_code(&input_str, &mut collector, codes);
find_testable_code(&input_str, &mut collector, codes, options.enable_per_target_ignores);

options.test_args.insert(0, "rustdoctest".to_string());
testing::test_main(&options.test_args, collector.tests,
Expand Down
Loading