Skip to content

Commit

Permalink
Rollup merge of rust-lang#130693 - jieyouxu:minicore, r=bjorn3
Browse files Browse the repository at this point in the history
Add `minicore` test auxiliary and support `//@ add-core-stubs` directive in ui/assembly/codegen tests

Context: [Real cross-compiling tests instead of `#![no_core]` silliness rust-lang#130375](rust-lang#130375)
MCP: rust-lang/compiler-team#786
Tracking issue: rust-lang#131485

This prototype PR is subject to further changes based on feedback.

### New `minicore` test auxiliary and `//@ add-core-stubs` compiletest directive

This PR introduces a prototype implementation of a `minicore` auxiliary test helper that provides `core` stubs for `#![no_core]` ui/assembly/codegen tests that need to build but not run on both the host platform and the cross-compiled target platform.

Key summary:

- `tests/auxiliary/minicore.rs` contains stub definitions of `core` items intended for consumption by `check-pass`/`build-pass` tests that want the typical prelude items like `Copy` to be stubbed out under `#![no_core]` scenarios, so that the test can be built (not run) for cross-compiled target platforms. Such tests don't want nor need full `-Z build-std` (e.g. `tests/ui/abi/compatibility.rs`).
- `minicore` is intended for `core` items **only**, not `std`- or `alloc`-exclusive items. If stubs for `alloc` or `std` are wanted, they should be provided by an additional directive and test auxiliary, and not be conflated with `minicore` or `core` stubs. This is because a wider range of tests can benefit from `core`-only stubs.

### Implementation

- The `minicore` auxiliary is a single source file `tests/auxiliary/minicore.rs`.
- The path to `minicore` is made avaiable from bootstrap to compiletest via the `--minicore-path` compiletest flag.
- `minicore` is then built on-demand via the `//@ add-core-stubs` compiletest directive, for each test revision for the given target platform (this distinction is important for when host platform != target platform in cross-compilation scenario).
- `minicore` is then made available to the test as an [extern prelude].

[extern prelude]: https://doc.rust-lang.org/reference/names/preludes.html#extern-prelude

### Example usage

```rs
// tests/ui/abi/my-abi-test.rs

//@ check-pass
//@ add-core-stubs
//@ compile-flags: --target i686-unknown-linux-gnu
//@ needs-llvm-components: x86

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

extern crate minicore;
use minicore::*;

#[lang = "clone"]
pub trait Clone: Sized {      // `Sized` is provided by `minicore`
    fn clone(&self) -> Self;
}
```

### Implementation steps

- [x] 1. Add an initial `minicore` test auxiliary.
- [x] 2. Build `minicore` in bootstrap.
- [x] 3. Setup a `--minicore-path` compiletest cli flag and pass `minicore` build artifact path from bootstrap to compiletest.
- [x] 4. Assert `add-core-stubs` is mutually incompatible with tests that require to be `run`, as the stubs are only good for tests that only need to be built (i.e. no `run-{pass,fail}`).
- [x] 5. Add some self-tests to sanity check the behavior.
- [x] 6. Ensure that `tests/auxiliary/minicore.rs` is input stamped, i.e. modifying `tests/auxiliary/minicore.rs` should invalidate test cache and force the test to be rerun.

### Known limitations

- The current `minicore` is very minimal, because this PR is intended to focus on supporting the test infrastructure first. Further stubs could be added in follow-up PRs and/or on a as-needed basis.

try-job: aarch64-apple
try-job: armhf-gnu
try-job: x86_64-msvc
try-job: test-various
try-job: dist-various-1
  • Loading branch information
matthiaskrgr authored Oct 31, 2024
2 parents 4d296ea + adb6d47 commit 788db96
Show file tree
Hide file tree
Showing 13 changed files with 262 additions and 87 deletions.
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 {
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> {}

#[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
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

0 comments on commit 788db96

Please sign in to comment.