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

Add minicore test auxiliary and support //@ add-core-stubs directive in ui/assembly/codegen tests #130693

Merged
merged 10 commits into from
Oct 31, 2024
Merged
5 changes: 5 additions & 0 deletions src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1725,6 +1725,11 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
cmd.arg("--run-lib-path").arg(builder.sysroot_libdir(compiler, target));
cmd.arg("--rustc-path").arg(builder.rustc(compiler));

// Minicore auxiliary lib for `no_core` tests that need `core` stubs in cross-compilation
// scenarios.
cmd.arg("--minicore-path")
.arg(builder.src.join("tests").join("auxiliary").join("minicore.rs"));

let is_rustdoc = suite.ends_with("rustdoc-ui") || suite.ends_with("rustdoc-js");

if mode == "run-make" {
Expand Down
5 changes: 5 additions & 0 deletions src/tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,11 @@ pub struct Config {

/// Command for visual diff display, e.g. `diff-tool --color=always`.
pub diff_command: Option<String>,

/// Path to minicore aux library, used for `no_core` tests that need `core` stubs in
/// cross-compilation scenarios that do not otherwise want/need to `-Zbuild-std`. Used in e.g.
/// ABI tests.
pub minicore_path: PathBuf,
}

impl Config {
Expand Down
1 change: 1 addition & 0 deletions src/tools/compiletest/src/directive-list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
/// a best-effort approximation for diagnostics. Add new headers to this list when needed.
const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
// tidy-alphabetical-start
"add-core-stubs",
"assembly-output",
"aux-bin",
"aux-build",
Expand Down
28 changes: 28 additions & 0 deletions src/tools/compiletest/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,9 @@ pub struct TestProps {
pub no_auto_check_cfg: bool,
/// Run tests which require enzyme being build
pub has_enzyme: bool,
/// Build and use `minicore` as `core` stub for `no_core` tests in cross-compilation scenarios
/// that don't otherwise want/need `-Z build-std`.
pub add_core_stubs: bool,
}

mod directives {
Expand Down Expand Up @@ -243,6 +246,7 @@ mod directives {
pub const LLVM_COV_FLAGS: &'static str = "llvm-cov-flags";
pub const FILECHECK_FLAGS: &'static str = "filecheck-flags";
pub const NO_AUTO_CHECK_CFG: &'static str = "no-auto-check-cfg";
pub const ADD_CORE_STUBS: &'static str = "add-core-stubs";
// This isn't a real directive, just one that is probably mistyped often
pub const INCORRECT_COMPILER_FLAGS: &'static str = "compiler-flags";
}
Expand Down Expand Up @@ -300,6 +304,7 @@ impl TestProps {
filecheck_flags: vec![],
no_auto_check_cfg: false,
has_enzyme: false,
add_core_stubs: false,
}
}

Expand Down Expand Up @@ -564,6 +569,8 @@ impl TestProps {
}

config.set_name_directive(ln, NO_AUTO_CHECK_CFG, &mut self.no_auto_check_cfg);

self.update_add_core_stubs(ln, config);
},
);

Expand Down Expand Up @@ -677,6 +684,27 @@ impl TestProps {
pub fn local_pass_mode(&self) -> Option<PassMode> {
self.pass_mode
}

pub fn update_add_core_stubs(&mut self, ln: &str, config: &Config) {
let add_core_stubs = config.parse_name_directive(ln, directives::ADD_CORE_STUBS);
if add_core_stubs {
if !matches!(config.mode, Mode::Ui | Mode::Codegen | Mode::Assembly) {
panic!(
"`add-core-stubs` is currently only supported for ui, codegen and assembly test modes"
);
}

// FIXME(jieyouxu): this check is currently order-dependent, but we should probably
// collect all directives in one go then perform a validation pass after that.
if self.local_pass_mode().is_some_and(|pm| pm == PassMode::Run) {
// `minicore` can only be used with non-run modes, because it's `core` prelude stubs
// and can't run.
panic!("`add-core-stubs` cannot be used to run the test binary");
}

self.add_core_stubs = add_core_stubs;
}
}
}

/// If the given line begins with the appropriate comment prefix for a directive,
Expand Down
1 change: 1 addition & 0 deletions src/tools/compiletest/src/header/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ impl ConfigBuilder {
"--git-repository=",
"--nightly-branch=",
"--git-merge-commit-email=",
"--minicore-path=",
];
let mut args: Vec<String> = args.iter().map(ToString::to_string).collect();

Expand Down
13 changes: 12 additions & 1 deletion src/tools/compiletest/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,8 @@ pub fn parse_config(args: Vec<String>) -> Config {
"compiletest-diff-tool",
"What custom diff tool to use for displaying compiletest tests.",
"COMMAND",
);
)
.reqopt("", "minicore-path", "path to minicore aux library", "PATH");

let (argv0, args_) = args.split_first().unwrap();
if args.len() == 1 || args[1] == "-h" || args[1] == "--help" {
Expand Down Expand Up @@ -371,7 +372,10 @@ pub fn parse_config(args: Vec<String>) -> Config {
git_merge_commit_email: matches.opt_str("git-merge-commit-email").unwrap(),

profiler_runtime: matches.opt_present("profiler-runtime"),

diff_command: matches.opt_str("compiletest-diff-tool"),

minicore_path: opt_path(matches, "minicore-path"),
}
}

Expand Down Expand Up @@ -409,6 +413,7 @@ pub fn log_config(config: &Config) {
logv(c, format!("host-linker: {:?}", config.host_linker));
logv(c, format!("verbose: {}", config.verbose));
logv(c, format!("format: {:?}", config.format));
logv(c, format!("minicore_path: {:?}", config.minicore_path.display()));
logv(c, "\n".to_string());
}

Expand Down Expand Up @@ -885,6 +890,12 @@ fn files_related_to_test(
related.push(path);
}

// `minicore.rs` test auxiliary: we need to make sure tests get rerun if this changes.
//
// FIXME(jieyouxu): untangle these paths, we should provide both a path to root `tests/` or
// `tests/auxiliary/` and the test suite in question. `src_base` is also a terrible name.
related.push(config.src_base.parent().unwrap().join("auxiliary").join("minicore.rs"));

related
}

Expand Down
78 changes: 48 additions & 30 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1150,14 +1150,20 @@ impl<'test> TestCx<'test> {
}
}

/// `root_testpaths` refers to the path of the original test.
/// the auxiliary and the test with an aux-build have the same `root_testpaths`.
/// `root_testpaths` refers to the path of the original test. the auxiliary and the test with an
/// aux-build have the same `root_testpaths`.
fn compose_and_run_compiler(
&self,
mut rustc: Command,
input: Option<String>,
root_testpaths: &TestPaths,
) -> ProcRes {
if self.props.add_core_stubs {
let minicore_path = self.build_minicore();
rustc.arg("--extern");
rustc.arg(&format!("minicore={}", minicore_path.to_str().unwrap()));
}

let aux_dir = self.aux_output_dir();
self.build_all_auxiliary(root_testpaths, &aux_dir, &mut rustc);

Expand All @@ -1171,6 +1177,37 @@ impl<'test> TestCx<'test> {
)
}

/// Builds `minicore`. Returns the path to the minicore rlib within the base test output
/// directory.
fn build_minicore(&self) -> PathBuf {
Copy link
Member Author

Choose a reason for hiding this comment

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

Remark: I don't like how convoluted the top-level runtest logic is, but intended for future compiletest cleanup PRs instead of this PR.

let output_file_path = self.output_base_dir().join("libminicore.rlib");
let mut rustc = self.make_compile_args(
&self.config.minicore_path,
TargetLocation::ThisFile(output_file_path.clone()),
Emit::None,
AllowUnused::Yes,
LinkToAux::No,
vec![],
);

rustc.args(&["--crate-type", "rlib"]);
rustc.arg("-Cpanic=abort");

let res =
self.compose_and_run(rustc, self.config.compile_lib_path.to_str().unwrap(), None, None);
if !res.status.success() {
self.fatal_proc_rec(
&format!(
"auxiliary build of {:?} failed to compile: ",
self.config.minicore_path.display()
),
&res,
);
}

output_file_path
}

/// Builds an aux dependency.
fn build_auxiliary(
&self,
Expand Down Expand Up @@ -1662,6 +1699,15 @@ impl<'test> TestCx<'test> {

rustc.args(&self.props.compile_flags);

// FIXME(jieyouxu): we should report a fatal error or warning if user wrote `-Cpanic=` with
// something that's not `abort`, however, by moving this last we should override previous
// `-Cpanic=`s
//
// `minicore` requires `#![no_std]` and `#![no_core]`, which means no unwinding panics.
if self.props.add_core_stubs {
rustc.arg("-Cpanic=abort");
}

rustc
}

Expand Down Expand Up @@ -1848,34 +1894,6 @@ impl<'test> TestCx<'test> {
(proc_res, output_path)
}

fn compile_test_and_save_assembly(&self) -> (ProcRes, PathBuf) {
// This works with both `--emit asm` (as default output name for the assembly)
// and `ptx-linker` because the latter can write output at requested location.
let output_path = self.output_base_name().with_extension("s");
let input_file = &self.testpaths.file;

// Use the `//@ assembly-output:` directive to determine how to emit assembly.
let emit = match self.props.assembly_output.as_deref() {
Some("emit-asm") => Emit::Asm,
Some("bpf-linker") => Emit::LinkArgsAsm,
Some("ptx-linker") => Emit::None, // No extra flags needed.
Some(other) => self.fatal(&format!("unknown 'assembly-output' directive: {other}")),
None => self.fatal("missing 'assembly-output' directive"),
};

let rustc = self.make_compile_args(
input_file,
TargetLocation::ThisFile(output_path.clone()),
emit,
AllowUnused::No,
LinkToAux::Yes,
Vec::new(),
);

let proc_res = self.compose_and_run_compiler(rustc, None, self.testpaths);
(proc_res, output_path)
}

fn verify_with_filecheck(&self, output: &Path) -> ProcRes {
let mut filecheck = Command::new(self.config.llvm_filecheck.as_ref().unwrap());
filecheck.arg("--input-file").arg(output).arg(&self.testpaths.file);
Expand Down
32 changes: 31 additions & 1 deletion src/tools/compiletest/src/runtest/assembly.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use super::TestCx;
use std::path::PathBuf;

use super::{AllowUnused, Emit, LinkToAux, ProcRes, TargetLocation, TestCx};

impl TestCx<'_> {
pub(super) fn run_assembly_test(&self) {
Expand All @@ -16,4 +18,32 @@ impl TestCx<'_> {
self.fatal_proc_rec("verification with 'FileCheck' failed", &proc_res);
}
}

fn compile_test_and_save_assembly(&self) -> (ProcRes, PathBuf) {
// This works with both `--emit asm` (as default output name for the assembly)
// and `ptx-linker` because the latter can write output at requested location.
let output_path = self.output_base_name().with_extension("s");
let input_file = &self.testpaths.file;

// Use the `//@ assembly-output:` directive to determine how to emit assembly.
let emit = match self.props.assembly_output.as_deref() {
Some("emit-asm") => Emit::Asm,
Some("bpf-linker") => Emit::LinkArgsAsm,
Some("ptx-linker") => Emit::None, // No extra flags needed.
Some(other) => self.fatal(&format!("unknown 'assembly-output' directive: {other}")),
None => self.fatal("missing 'assembly-output' directive"),
};

let rustc = self.make_compile_args(
input_file,
TargetLocation::ThisFile(output_path.clone()),
emit,
AllowUnused::No,
LinkToAux::Yes,
Vec::new(),
);

let proc_res = self.compose_and_run_compiler(rustc, None, self.testpaths);
(proc_res, output_path)
}
}
5 changes: 5 additions & 0 deletions tests/assembly/compiletest-self-test/use-minicore-no-run.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
//! `compiletest` self-test to check that `add-core-stubs` is incompatible with run pass modes.

//@ add-core-stubs
//@ run-pass
//@ should-fail
72 changes: 72 additions & 0 deletions tests/auxiliary/minicore.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
//! Auxiliary `minicore` prelude which stubs out `core` items for `no_core` tests that need to work
//! in cross-compilation scenarios where no `core` is available (that don't want nor need to
//! `-Zbuild-std`).
//!
//! # Important notes
//!
//! - `minicore` is **only** intended for `core` items, and the stubs should match the actual `core`
//! items.
//!
//! # References
//!
//! This is partially adapted from `rustc_codegen_cranelift`:
//! <https://github.com/rust-lang/rust/blob/c0b5cc9003f6464c11ae1c0662c6a7e06f6f5cab/compiler/rustc_codegen_cranelift/example/mini_core.rs>.
// ignore-tidy-linelength

#![feature(no_core, lang_items, rustc_attrs)]
#![allow(unused, improper_ctypes_definitions, internal_features)]
#![no_std]
#![no_core]

// `core` has some exotic `marker_impls!` macro for handling the with-generics cases, but for our
// purposes, just use a simple macro_rules macro.
macro_rules! impl_marker_trait {
($Trait:ident => [$( $ty:ident ),* $(,)?] ) => {
$( impl $Trait for $ty {} )*
}
}

#[lang = "sized"]
pub trait Sized {}

#[lang = "legacy_receiver"]
pub trait LegacyReceiver {}
impl<T: ?Sized> LegacyReceiver for &T {}
impl<T: ?Sized> LegacyReceiver for &mut T {}

#[lang = "copy"]
pub trait Copy: Sized {}

impl_marker_trait!(Copy => [ bool, char, isize, usize, i8, i16, i32, i64, u8, u16, u32, u64 ]);
impl<'a, T: ?Sized> Copy for &'a T {}
impl<T: ?Sized> Copy for *const T {}
impl<T: ?Sized> Copy for *mut T {}

#[lang = "phantom_data"]
pub struct PhantomData<T: ?Sized>;
impl<T: ?Sized> Copy for PhantomData<T> {}

pub enum Option<T> {
None,
Some(T),
}
impl<T: Copy> Copy for Option<T> {}

pub enum Result<T, E> {
Ok(T),
Err(E),
}
impl<T: Copy, E: Copy> Copy for Result<T, E> {}

#[lang = "manually_drop"]
#[repr(transparent)]
pub struct ManuallyDrop<T: ?Sized> {
value: T,
}
impl<T: Copy + ?Sized> Copy for ManuallyDrop<T> {}
Copy link
Member

@bjorn3 bjorn3 Sep 22, 2024

Choose a reason for hiding this comment

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

You can derive Copy if you add

#[rustc_builtin_macro]
pub macro Copy($item:item) {
    /* compiler built-in */
}

and the same applies to Clone.

Copy link
Member Author

Choose a reason for hiding this comment

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

I did not make this change because I couldn't figure out how to make use of this, just kept manual impl Copy for ... for now.


#[lang = "unsafe_cell"]
#[repr(transparent)]
pub struct UnsafeCell<T: ?Sized> {
value: T,
}
20 changes: 20 additions & 0 deletions tests/codegen/compiletest-self-test/minicore-smoke-test.rs
jieyouxu marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
//! Basic smoke test for `minicore` test auxiliary.

//@ add-core-stubs
//@ compile-flags: --target=x86_64-unknown-linux-gnu
//@ needs-llvm-components: x86

#![crate_type = "lib"]
#![feature(no_core)]
#![no_std]
#![no_core]

extern crate minicore;
use minicore::*;

struct Meow;
impl Copy for Meow {}

// CHECK-LABEL: meow
#[no_mangle]
fn meow() {}
Loading
Loading