Skip to content

Run doctests via out-of-process rustc #63827

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

Merged
merged 2 commits into from
Aug 30, 2019
Merged
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions src/librustc_driver/lib.rs
Original file line number Diff line number Diff line change
@@ -439,6 +439,15 @@ fn make_input(free_matches: &[String]) -> Option<(Input, Option<PathBuf>, Option
} else {
None
};
if let Ok(path) = env::var("UNSTABLE_RUSTDOC_TEST_PATH") {
let line = env::var("UNSTABLE_RUSTDOC_TEST_LINE").
expect("when UNSTABLE_RUSTDOC_TEST_PATH is set \
UNSTABLE_RUSTDOC_TEST_LINE also needs to be set");
let line = isize::from_str_radix(&line, 10).
expect("UNSTABLE_RUSTDOC_TEST_LINE needs to be an number");
let file_name = FileName::doc_test_source_code(PathBuf::from(path), line);
return Some((Input::Str { name: file_name, input: src }, None, err));
}
Some((Input::Str { name: FileName::anon_source_code(&src), input: src },
None, err))
} else {
12 changes: 12 additions & 0 deletions src/librustdoc/config.rs
Original file line number Diff line number Diff line change
@@ -39,12 +39,18 @@ pub struct Options {
pub error_format: ErrorOutputType,
/// Library search paths to hand to the compiler.
pub libs: Vec<SearchPath>,
/// Library search paths strings to hand to the compiler.
pub lib_strs: Vec<String>,
/// The list of external crates to link against.
pub externs: Externs,
/// The list of external crates strings to link against.
pub extern_strs: Vec<String>,
/// List of `cfg` flags to hand to the compiler. Always includes `rustdoc`.
pub cfgs: Vec<String>,
/// Codegen options to hand to the compiler.
pub codegen_options: CodegenOptions,
/// Codegen options strings to hand to the compiler.
pub codegen_options_strs: Vec<String>,
/// Debugging (`-Z`) options to pass to the compiler.
pub debugging_options: DebuggingOptions,
/// The target used to compile the crate against.
@@ -461,6 +467,9 @@ impl Options {
let generate_search_filter = !matches.opt_present("disable-per-crate-search");
let persist_doctests = matches.opt_str("persist-doctests").map(PathBuf::from);
let generate_redirect_pages = matches.opt_present("generate-redirect-pages");
let codegen_options_strs = matches.opt_strs("C");
let lib_strs = matches.opt_strs("L");
let extern_strs = matches.opt_strs("extern");

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

@@ -470,9 +479,12 @@ impl Options {
proc_macro_crate,
error_format,
libs,
lib_strs,
externs,
extern_strs,
cfgs,
codegen_options,
codegen_options_strs,
debugging_options,
target,
edition,
7 changes: 2 additions & 5 deletions src/librustdoc/markdown.rs
Original file line number Diff line number Diff line change
@@ -142,11 +142,8 @@ pub fn test(mut options: Options, diag: &errors::Handler) -> i32 {
let mut opts = TestOptions::default();
opts.no_crate_inject = true;
opts.display_warnings = options.display_warnings;
let mut collector = Collector::new(options.input.display().to_string(), options.cfgs,
options.libs, options.codegen_options, options.externs,
true, opts, options.maybe_sysroot, None,
Some(options.input),
options.linker, options.edition, options.persist_doctests);
let mut collector = Collector::new(options.input.display().to_string(), options.clone(),
true, opts, None, Some(options.input));
collector.set_position(DUMMY_SP);
let codes = ErrorCodes::from(UnstableFeatures::from_environment().is_nightly_build());

214 changes: 68 additions & 146 deletions src/librustdoc/test.rs
Original file line number Diff line number Diff line change
@@ -2,24 +2,19 @@ use rustc_data_structures::sync::Lrc;
use rustc_interface::interface;
use rustc::hir;
use rustc::hir::intravisit;
use rustc::hir::def_id::LOCAL_CRATE;
use rustc::session::{self, config, DiagnosticOutput};
use rustc::session::config::{OutputType, OutputTypes, Externs, CodegenOptions};
use rustc::session::search_paths::SearchPath;
use rustc::util::common::ErrorReported;
use syntax::ast;
use syntax::with_globals;
use syntax::source_map::SourceMap;
use syntax::edition::Edition;
use syntax::feature_gate::UnstableFeatures;
use std::env;
use std::io::prelude::*;
use std::io;
use std::panic::{self, AssertUnwindSafe};
use std::io::{self, Write};
use std::panic;
use std::path::PathBuf;
use std::process::{self, Command};
use std::process::{self, Command, Stdio};
use std::str;
use std::sync::{Arc, Mutex};
use syntax::symbol::sym;
use syntax_pos::{BytePos, DUMMY_SP, Pos, Span, FileName};
use tempfile::Builder as TempFileBuilder;
@@ -89,18 +84,11 @@ pub fn run(options: Options) -> i32 {
opts.display_warnings |= options.display_warnings;
let mut collector = Collector::new(
compiler.crate_name()?.peek().to_string(),
options.cfgs,
options.libs,
options.codegen_options,
options.externs,
options,
false,
opts,
options.maybe_sysroot,
Some(compiler.source_map().clone()),
None,
options.linker,
options.edition,
options.persist_doctests,
);

let mut global_ctxt = compiler.global_ctxt()?.take();
@@ -189,20 +177,14 @@ fn run_test(
cratename: &str,
filename: &FileName,
line: usize,
cfgs: Vec<String>,
libs: Vec<SearchPath>,
cg: CodegenOptions,
externs: Externs,
options: Options,
should_panic: bool,
no_run: bool,
as_test_harness: bool,
compile_fail: bool,
mut error_codes: Vec<String>,
opts: &TestOptions,
maybe_sysroot: Option<PathBuf>,
linker: Option<PathBuf>,
edition: Edition,
persist_doctests: Option<PathBuf>,
) -> Result<(), TestFailure> {
let (test, line_offset) = match panic::catch_unwind(|| {
make_test(test, Some(cratename), as_test_harness, opts, edition)
@@ -223,61 +205,6 @@ fn run_test(
_ => PathBuf::from(r"doctest.rs"),
};

let input = config::Input::Str {
name: FileName::DocTest(path, line as isize - line_offset as isize),
input: test,
};
let outputs = OutputTypes::new(&[(OutputType::Exe, None)]);

let sessopts = config::Options {
maybe_sysroot,
search_paths: libs,
crate_types: vec![config::CrateType::Executable],
output_types: outputs,
externs,
cg: config::CodegenOptions {
linker,
..cg
},
test: as_test_harness,
unstable_features: UnstableFeatures::from_environment(),
debugging_opts: config::DebuggingOptions {
..config::basic_debugging_options()
},
edition,
..config::Options::default()
};

// Shuffle around a few input and output handles here. We're going to pass
// an explicit handle into rustc to collect output messages, but we also
// want to catch the error message that rustc prints when it fails.
//
// We take our thread-local stderr (likely set by the test runner) and replace
// it with a sink that is also passed to rustc itself. When this function
// returns the output of the sink is copied onto the output of our own thread.
//
// The basic idea is to not use a default Handler for rustc, and then also
// not print things by default to the actual stderr.
struct Sink(Arc<Mutex<Vec<u8>>>);
impl Write for Sink {
fn write(&mut self, data: &[u8]) -> io::Result<usize> {
Write::write(&mut *self.0.lock().unwrap(), data)
}
fn flush(&mut self) -> io::Result<()> { Ok(()) }
}
struct Bomb(Arc<Mutex<Vec<u8>>>, Option<Box<dyn Write+Send>>);
impl Drop for Bomb {
fn drop(&mut self) {
let mut old = self.1.take().unwrap();
let _ = old.write_all(&self.0.lock().unwrap());
io::set_panic(Some(old));
}
}
let data = Arc::new(Mutex::new(Vec::new()));

let old = io::set_panic(Some(box Sink(data.clone())));
let _bomb = Bomb(data.clone(), Some(old.unwrap_or(box io::stdout())));

enum DirState {
Temp(tempfile::TempDir),
Perm(PathBuf),
@@ -292,7 +219,7 @@ fn run_test(
}
}

let outdir = if let Some(mut path) = persist_doctests {
let outdir = if let Some(mut path) = options.persist_doctests {
path.push(format!("{}_{}",
filename
.to_string()
@@ -314,49 +241,73 @@ fn run_test(
};
let output_file = outdir.path().join("rust_out");

let config = interface::Config {
opts: sessopts,
crate_cfg: config::parse_cfgspecs(cfgs),
input,
input_path: None,
output_file: Some(output_file.clone()),
output_dir: None,
file_loader: None,
diagnostic_output: DiagnosticOutput::Raw(box Sink(data.clone())),
stderr: Some(data.clone()),
crate_name: None,
lint_caps: Default::default(),
};
let mut compiler = Command::new(std::env::current_exe().unwrap().with_file_name("rustc"));
Copy link
Member

Choose a reason for hiding this comment

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

I'm pretty sure this will fail on OpenBSD, see #61377.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

any suggestions how to solve this? I do not have access to an OpenBSD computer

Copy link
Member

Choose a reason for hiding this comment

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

IMO, the correct thing here is probably to fallback to RUSTC from the environment, and then falling back to just rustc (whichever one we find in PATH). This isn't great, but I think should work fine for most cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tried to fix it like this but I get a lot of tests that fails due to RUSTC is set stage0 while the rustdoc that is tested is stage2, but it feels more correct to check the RUSTC env var first otherwise the miri case will still not work

let mut compiler = if let Some(rustc) = env::var_os("RUSTC"){
    eprintln!("env var {:?}",&rustc);
    Command::new(&rustc)
}else {
    let rustc = std::env::current_exe().unwrap_or_default().with_file_name("rustc");
    eprintln!("exec {:?}",&rustc);
    Command::new(&rustc)
};

compiler.arg("--crate-type").arg("bin");
for cfg in &options.cfgs {
compiler.arg("--cfg").arg(&cfg);
}
if let Some(sysroot) = options.maybe_sysroot {
compiler.arg("--sysroot").arg(sysroot);
}
compiler.arg("--edition").arg(&edition.to_string());
compiler.env("UNSTABLE_RUSTDOC_TEST_PATH", path);
compiler.env("UNSTABLE_RUSTDOC_TEST_LINE",
format!("{}", line as isize - line_offset as isize));
compiler.arg("-o").arg(&output_file);
if as_test_harness {
compiler.arg("--test");
}
for lib_str in &options.lib_strs {
compiler.arg("-L").arg(&lib_str);
}
for extern_str in &options.extern_strs {
compiler.arg("--extern").arg(&extern_str);
}
for codegen_options_str in &options.codegen_options_strs {
compiler.arg("-C").arg(&codegen_options_str);
}
if let Some(linker) = options.linker {
compiler.arg(&format!("-C linker={:?}", linker));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
compiler.arg(&format!("-C linker={:?}", linker));
compiler.arg(&format!("-Clinker={}", linker.display()));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

was hopping to remove this code in a following PR rebasing the #63816 on to this PR

Copy link
Member

Choose a reason for hiding this comment

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

Ah, this should be doing .arg("-Clinker").arg(&linker) anyway

Copy link
Member

Choose a reason for hiding this comment

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

Followup PR is fine though

Copy link
Contributor Author

Choose a reason for hiding this comment

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

have rebase #63816 and in that PR this line is removed

}
if no_run {
compiler.arg("--emit=metadata");
}

let compile_result = panic::catch_unwind(AssertUnwindSafe(|| {
interface::run_compiler(config, |compiler| {
if no_run {
compiler.global_ctxt().and_then(|global_ctxt| global_ctxt.take().enter(|tcx| {
tcx.analysis(LOCAL_CRATE)
})).ok();
} else {
compiler.compile().ok();
};
compiler.session().compile_status()
})
})).map_err(|_| ()).and_then(|s| s.map_err(|_| ()));
compiler.arg("-");
compiler.stdin(Stdio::piped());
compiler.stderr(Stdio::piped());

let mut child = compiler.spawn().expect("Failed to spawn rustc process");
{
let stdin = child.stdin.as_mut().expect("Failed to open stdin");
stdin.write_all(test.as_bytes()).expect("could write out test sources");
}
let output = child.wait_with_output().expect("Failed to read stdout");

match (compile_result, compile_fail) {
(Ok(()), true) => {
struct Bomb<'a>(&'a str);
impl Drop for Bomb<'_> {
fn drop(&mut self) {
eprint!("{}",self.0);
}
}

let out = str::from_utf8(&output.stderr).unwrap();
let _bomb = Bomb(&out);
match (output.status.success(), compile_fail) {
(true, true) => {
return Err(TestFailure::UnexpectedCompilePass);
}
(Ok(()), false) => {}
(Err(_), true) => {
(true, false) => {}
(false, true) => {
if !error_codes.is_empty() {
let out = String::from_utf8(data.lock().unwrap().to_vec()).unwrap();
error_codes.retain(|err| !out.contains(err));

if !error_codes.is_empty() {
return Err(TestFailure::MissingErrorCodes(error_codes));
}
}
}
(Err(_), false) => {
(false, false) => {
return Err(TestFailure::CompileError);
}
}
@@ -652,45 +603,28 @@ pub struct Collector {
// the `names` vector of that test will be `["Title", "Subtitle"]`.
names: Vec<String>,

cfgs: Vec<String>,
libs: Vec<SearchPath>,
cg: CodegenOptions,
externs: Externs,
options: Options,
use_headers: bool,
cratename: String,
opts: TestOptions,
maybe_sysroot: Option<PathBuf>,
position: Span,
source_map: Option<Lrc<SourceMap>>,
filename: Option<PathBuf>,
linker: Option<PathBuf>,
edition: Edition,
persist_doctests: Option<PathBuf>,
}

impl Collector {
pub fn new(cratename: String, cfgs: Vec<String>, libs: Vec<SearchPath>, cg: CodegenOptions,
externs: Externs, use_headers: bool, opts: TestOptions,
maybe_sysroot: Option<PathBuf>, source_map: Option<Lrc<SourceMap>>,
filename: Option<PathBuf>, linker: Option<PathBuf>, edition: Edition,
persist_doctests: Option<PathBuf>) -> Collector {
pub fn new(cratename: String, options: Options, use_headers: bool, opts: TestOptions,
source_map: Option<Lrc<SourceMap>>, filename: Option<PathBuf>,) -> Collector {
Collector {
tests: Vec::new(),
names: Vec::new(),
cfgs,
libs,
cg,
externs,
options,
use_headers,
cratename,
opts,
maybe_sysroot,
position: DUMMY_SP,
source_map,
filename,
linker,
edition,
persist_doctests,
}
}

@@ -725,16 +659,10 @@ impl Tester for Collector {
fn add_test(&mut self, test: String, config: LangString, line: usize) {
let filename = self.get_filename();
let name = self.generate_name(line, &filename);
let cfgs = self.cfgs.clone();
let libs = self.libs.clone();
let cg = self.cg.clone();
let externs = self.externs.clone();
let cratename = self.cratename.to_string();
let opts = self.opts.clone();
let maybe_sysroot = self.maybe_sysroot.clone();
let linker = self.linker.clone();
let edition = config.edition.unwrap_or(self.edition);
let persist_doctests = self.persist_doctests.clone();
let edition = config.edition.unwrap_or(self.options.edition.clone());
let options = self.options.clone();

debug!("creating test {}: {}", name, test);
self.tests.push(testing::TestDescAndFn {
@@ -751,20 +679,14 @@ impl Tester for Collector {
&cratename,
&filename,
line,
cfgs,
libs,
cg,
externs,
options,
config.should_panic,
config.no_run,
config.test_harness,
config.compile_fail,
config.error_codes,
&opts,
maybe_sysroot,
linker,
edition,
persist_doctests
);

if let Err(err) = res {