Add `minicore` test auxiliary and support `//@ use-minicore` directive in ui/assembly/codegen tests #130693

jieyouxu · 2024-09-22T08:14:46Z

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

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

New `minicore` test auxiliary and `//@ use-minicore` compiletest directive

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

Key summary:

minicore 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. 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-only or alloc 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 //@ use-minicore compiletest directive, for each test revision for the given target (this distinction is important for when host != target in cross-compilation scenario).
minicore is then made available to the test as an extern prelude.

Example usage

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

//@ check-pass
//@ use-minicore
//@ revisions: i686
//@[i686] compile-flags: --target i686-unknown-linux-gnu
//@[i686] 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

1. Add an initial minicore test auxiliary.
2. Build minicore in bootstrap.
3. Setup a --minicore-path compiletest cli flag and pass minicore build artifact path from bootstrap to compiletest.
4. Assert use-minicore 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}).
5. Add some self-tests to sanity check the behavior.
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.

try-job: aarch64-apple
try-job: armhf-gnu
try-job: x86_64-msvc
try-job: test-various
try-job: dist-various-1

jieyouxu · 2024-09-22T08:20:10Z

@bors try

[PROTOTYPE] Add minicore test auxiliary and support `//@ use-minicore` directive in ui/assembly/codegen tests **TODO: work in progress prototype implementation, opened early for MCP to reference** Context: [Real cross-compiling tests instead of `#![no_core]` silliness rust-lang#130375](rust-lang#130375) This PR introduces a prototype implementation of `minicore` auxiliary test helper. `minicore` contains stub definitions of std/core prelude items intended for consumption by tests that want the typical prelude items like `Copy` or `Result` in cross-compilation scenarios, but don't want nor need full `-Z build-std` (e.g. `tests/ui/abi/compatibility.rs`). The `minicore` auxiliary is a single source file `tests/auxiliary/minicore.rs`. The path to this auxiliary is made avaiable from bootstrap to compiletest via the `--minicore-path` compiletest flag. The `minicore` auxiliary is then built, on demand via `//@ use-minicore` compiletest directives, for each test revision for the given target (this distinction is important for when host != target in cross-compilation scenario). ### Implementation steps - [ ] 1. File an MCP to describe `tests/auxiliary/minicore.rs`, `--minicore-path` compiletest flag, `//@ use-minicore` compiletest directive and the behavior. - [ ] 2. And some self-tests to sanity check the behavior. - [ ] 3. Update rustc-dev-guide to describe the new `use-minicore` directive and provide an example, noting that `use-minicore` both requires `no_std` + `no_core` and implies `-C panic=abort` for the test file. r? `@ghost` (not yet ready for full review, still needs some self-tests and some try-jobs) (TODO: cc interested people once this passes initial try jobs in cross-compilation scenarios) cc `@/workingjubilee` `@/RalfJung` `@/nikic` `@/chrisnc` (if this makes sense to you in terms of functionality and UX) try-job: aarch64-apple try-job: armhf-gnu try-job: x86_64-msvc try-job: test-various try-job: dist-various-1

bors · 2024-09-22T08:21:21Z

⌛ Trying commit 9d6f01a with merge 01b2fff...

bors · 2024-09-22T09:12:27Z

💔 Test failed - checks-actions

bjorn3 · 2024-09-22T10:15:02Z

tests/auxiliary/minicore.rs

+pub struct ManuallyDrop<T: ?Sized> {
+    value: T,
+}
+impl<T: Copy + ?Sized> Copy for ManuallyDrop<T> {}


You can derive Copy if you add

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

and the same applies to Clone.

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.

tests/auxiliary/minicore.rs

jieyouxu · 2024-09-28T13:50:05Z

Changes since last review:

Fixed bootstrap path of tests/auxiliary/minicore.rs to be constructed from a builder.src prefix.
Trimmed minicore.rs to only include a minimal subset of core items, excluding the ones that are easy to abuse like #[rustc_layout_scalar_valid_range_start(1)] or #[rustc_nonnull_optimization_guaranteed].
Made sure no alloc or std-specific items get included in minicore.
Removed cfg(host) and cfg(non(host)) distinction in tests/abi/compatibility.rs, and just unconditionally stub out core items. Not sure if this is desirable.
Fixed minicore doc comments to no longer mention std, instead emphasize core items only. Also added remarks about rustc-attribute-annotated core items.

src/bootstrap/src/core/build_steps/test.rs

src/tools/compiletest/src/common.rs

src/tools/compiletest/src/header.rs

jieyouxu · 2024-09-28T14:27:23Z

src/tools/compiletest/src/runtest.rs

@@ -1129,6 +1135,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 {


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

jieyouxu · 2024-09-28T14:28:00Z

src/tools/compiletest/src/runtest.rs

+        // `use-minicore` requires `#![no_std]` and `#![no_core]`, which means no unwinding panics.
+        if self.props.use_minicore {
+            rustc.arg("-Cpanic=abort");
+        }


Note: I need to document //@ use-minicore implies and mandates -C panic=abort for the test in rustc-dev-guide.

rust-lang/rustc-dev-guide#2097

jieyouxu · 2024-09-28T14:28:59Z

src/tools/compiletest/src/runtest.rs

+        // `use-minicore` requires `#![no_std]` and `#![no_core]`, which means no unwinding panics.
+        if self.props.use_minicore {


Remark: technically, this also needs to check and ensure that the test writer does not manually override -C panic=xxx where xxx != abort, but I would prefer that happen after untangling how directives are handled. The flag is now set in the final position, which will override custom user compile-flags.

RalfJung · 2024-10-12T08:29:35Z

tests/ui/abi/compatibility.rs

@@ -1,5 +1,5 @@
 //@ check-pass
-//@ revisions: host


I think we can still have the host revision, we just don't want all the cfg(host) stuff any more and instead use the minicore approach for all revisions.

RalfJung · 2024-10-12T08:30:19Z

tests/codegen/compiletest-self-test/minicore-smoke-test.rs

Why are there two of these smoke tests?

Because in compiletest, different test suites have different code paths through the complicated runtest mechanism...

RalfJung · 2024-10-12T08:30:36Z

tests/codegen/compiletest-self-test/minicore-smoke-test.rs

+//! Basic smoke test for `minicore` test auxiliary.
+
+//@ use-minicore
+//@ revisions: meow


When there's just a single revision, why use the revision system at all?

Good point, this was there during testing, I forgot I only had one revision left.

jieyouxu · 2024-10-12T08:42:37Z

Hm fair -- I had not considered that. Maybe aux-crate is indeed better then. I don't know about that mechanism, how does it differ from aux-build?

aux-crate is a bit different in that it accepts the --extern syntax like foo=path/to/file.rs, and makes foo available via extern prelude.

workingjubilee · 2024-10-12T09:09:22Z

more concretely, for either include! or aux-crate or anything else, I wouldn't want to have to rewrite the path used every time I move a test, unless it's out of a given suite entirely (at which point I probably have to rewrite the test anyways, but even if I didn't).

so we'd want something like the variable-controlled filepath Urgau mentioned.

jieyouxu · 2024-10-12T09:14:42Z

I don't think it's very easy to reuse existing aux-crate/aux-build logic.

If I use something like

//@ aux-crate: minicore={{rust-src-base}}/../auxiliary/minicore.rs

The build mechanism related to that normalization + directive spontaneously combusts and instead outputs to test sources directory tests/auxiliary/minicore.<revision_name>/.

If I use

//@ aux-crate: minicore=../../../auxiliary/minicore.rs

compiletest can find minicore.rs, however it dies at extern minicore; because something is off about the --extern configuration aux-crate setup uses. EDIT: oh it's because minicore rlib is under compatibility.x86-64-win/auxiliary but then ../../../auxiliary/minicore sends it somewhere completely unrelated. Incredible.

error: extern location for minicore does not exist: E:\Repos\rust\build\x86_64-pc-windows-msvc\test\ui\abi\compatibility.x86-64-win\auxiliary/../../../auxiliary/minicore.dll
  --> E:\Repos\rust\tests\ui\abi\compatibility.rs:71:1
   |
LL | extern crate minicore;
   | ^^^^^^^^^^^^^^^^^^^^^^

... I'll come back to this later.

jieyouxu · 2024-10-12T23:24:28Z

I might temporarily shelve this unless we just use the //@ use-minicore magic (because I find that easier implementation-wise, I still haven't figured out why aux-crate is doing what it's doing), or maybe wait until we manage to tame the compiletest internals (e.g. ideas in https://rust-lang.zulipchat.com/#narrow/stream/326414-t-infra.2Fbootstrap/topic/.28Rubberducking.29.20compiletest.20test.20discovery.20.2F.20directives).

RalfJung · 2024-10-13T07:20:21Z

Given that the alternatives have been considered, a new directive seems fine for me. But let's see what t-compiler says. :)

Urgau · 2024-10-13T11:04:32Z

Note that if we go with include!, then each test that want to use minicore.rs will have to copy-pasta the include! decl because we don't have core

It's unfortunate, but given that //@ aux-crate is unusable for our needs, I think it's an acceptable trade-off given the currently involved and complex //@ use-minicore alternative.

I would propose that we go with this:

// usage of `minicore.rs` in every test that want to use it

#[rustc_builtin_macro]
macro_rules! include { ($file:expr $(,)?) => {{ /* compiler built-in */ }}; }

include!("../../auxiliary/minicore.rs");

and we ensure -Cpanic=abort with #[cfg(not(panic = "abort"))] as proposed earlier.

workingjubilee · 2024-10-13T17:19:32Z

You forgot several lines:

#![no_core]
#![crate_type = "lib"]
#![feature(rustc_attrs)]
#![feature(no_core)]
#![feature(lang_items)]
#![allow(internal_features)]

workingjubilee · 2024-10-13T17:21:22Z

Remember: #66920

Urgau · 2024-10-14T07:25:43Z

You forgot several lines:

Sure, but they also need to be included with all the other proposal I think, so using include! doesn't change this.

RalfJung · 2024-10-14T07:35:44Z

If minicore is a separate crate, those lines will not be needed in the test.

Urgau · 2024-10-14T07:44:21Z

I still see (some) of those lines in the current minicore-smoke-test.rs file with //@ use-minicore (which compiles minicore as a separate crate):

https://github.com/jieyouxu/rust/blob/90b6aae363cd7418ead533102204814808093e22/tests/codegen/compiletest-self-test/minicore-smoke-test.rs#L8-L11

(but there would probably be fewer of them)

(and while I don't like //@ use-minicore, it's still an acceptable solution, just not my favorite)

jieyouxu · 2024-10-17T03:40:43Z

Summary for T-compiler triage

minicore is a test auxiliary containing core stubs for cross-compiling #![no_core] ui/codegen/assembly tests. This is to help #![no_core] tests avoid needing to reinvent core stubs in every test. We are asking T-compiler to decide on an approach on how to implement minicore that's the most convenient/ergonomic for test writers.

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

Candidate Approaches

Approach 1: special `//@ use-minicore` directive

Add a special compiletest directive //@ use-minicore permitted for check/build (but not run) pass mode ui/codegen/assembly tests. This will build minicore as a crate and makes it available as an extern prelude to the test. //@ use-minicore will imply -C panic=abort since the test needs to be #![no_core] anyway.

Toy example:

//@ check-pass
//@ use-minicore
//@ compile-flags: --target i686-unknown-linux-gnu
//@ needs-llvm-components: x86

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

extern crate minicore;
use minicore::*;

struct Zst;
impl Copy for Zst {}

// CHECK-LABEL: foo
#[no_mangle]
fn foo(_x: Zst) {}

Approach 2: `include!("../../../minicore.rs")`

Just use the good-ol' include! macro.

Toy example:

//@ check-pass
//@ compile-flags: -Cpanic=abort
//@ compile-flags: --target i686-unknown-linux-gnu
//@ needs-llvm-components: x86

#![crate_type = "lib"]
#![feature(no_core, rustc_attrs, lang_items)]
#![no_core]
#![allow(internal_features)]

// Needed in each test because we don't have `include!` otherwise.
#[rustc_builtin_macro]
macro_rules! include { ($file:expr $(,)?) => {{ /* compiler built-in */ }}; }

include!("../../auxiliary/minicore.rs");

struct Zst;
impl Copy for Zst {}

// CHECK-LABEL: foo
#[no_mangle]
fn foo(_x: Zst) {}

Approach 3: retrofit `aux-crate` directive

compiletest currently has aux-crate aux build directives and associated logic. However, they don't handle relative paths well nor special path remapping like //@ aux-crate: minicore={{MINICORE_PATH}} well. They also assume an additional auxiliary/ subdirectory that current tests rely on. compilest currently has some bugs around auxiliaries as well. See #130693 (comment) for experimental history.

If these bugs are fixed, then the following will probably be what it looks like:

Toy example:

//@ check-pass
//@ aux-crate: minicore={{MINICORE_PATH}}
//@ compile-flags: -Cpanic=abort
//@ compile-flags: --target i686-unknown-linux-gnu
//@ needs-llvm-components: x86

#![crate_type = "lib"]
#![feature(no_core, rustc_attrs, lang_items)]
#![no_core]
#![allow(internal_features)]

extern crate minicore;
use minicore::*;

struct Zst;
impl Copy for Zst {}

// CHECK-LABEL: foo
#[no_mangle]
fn foo(_x: Zst) {}

Remarks

If minicore is a separate crate, those lines will not be needed in the test.

@RalfJung The bare minimum crate-level attribute set of

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

is needed in the test file itself, regardless of whichever approach we take here (outside of literal copy-paste à la #include <minicore.h>) because #![no_core] is crate-local and isn't "infectious". So it's needed by the test itself in any case.

apiraino · 2024-10-17T14:34:19Z

Visited during T-compiler triage meeting on Zulip, discussion can continue (asynchronously) here.

@rustbot label -I-compiler-nominated

jieyouxu force-pushed the minicore branch from 58bef84 to 9d6f01a Compare September 22, 2024 08:16

jieyouxu mentioned this pull request Sep 22, 2024

Support tests to use minicore std/core prelude stubs rust-lang/compiler-team#786

Closed

3 tasks

jieyouxu added S-waiting-on-MCP Status: PR has a compiler MCP and is waiting for the compiler MCP to complete. and removed needs-mcp This change is large enough that it needs a major change proposal before starting work. labels Sep 22, 2024

jieyouxu mentioned this pull request Sep 22, 2024

Real cross-compiling tests instead of #![no_core] silliness #130375

Open

This comment has been minimized.

Sign in to view

bjorn3 reviewed Sep 22, 2024

View reviewed changes

tests/auxiliary/minicore.rs Outdated Show resolved Hide resolved

bjorn3 reviewed Sep 22, 2024

View reviewed changes

tests/auxiliary/minicore.rs Outdated Show resolved Hide resolved

jieyouxu mentioned this pull request Sep 24, 2024

Add a run-make test for Arm target-cpu features. #130382

Draft

jieyouxu force-pushed the minicore branch 2 times, most recently from 34e9887 to df03437 Compare September 28, 2024 13:46

jieyouxu force-pushed the minicore branch 2 times, most recently from f4a7c38 to 8bd9644 Compare September 28, 2024 14:19

jieyouxu commented Sep 28, 2024

View reviewed changes

jieyouxu changed the title ~~[PROTOTYPE] Add minicore test auxiliary and support //@ use-minicore directive in ui/assembly/codegen tests~~ Add minicore test auxiliary and support //@ use-minicore directive in ui/assembly/codegen tests Sep 28, 2024

RalfJung reviewed Oct 12, 2024

View reviewed changes

jieyouxu added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 12, 2024

jieyouxu unassigned bjorn3 and albertlarsan68 Oct 12, 2024

jieyouxu marked this pull request as draft October 12, 2024 23:24

RalfJung added the I-compiler-nominated Nominated for discussion during a compiler team meeting. label Oct 13, 2024

jfrimmel mentioned this pull request Oct 15, 2024

Regression test for AVR rjmp offset #131755

Open

rustbot removed the I-compiler-nominated Nominated for discussion during a compiler team meeting. label Oct 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `minicore` test auxiliary and support `//@ use-minicore` directive in ui/assembly/codegen tests #130693

Add `minicore` test auxiliary and support `//@ use-minicore` directive in ui/assembly/codegen tests #130693

jieyouxu commented Sep 22, 2024 •

edited

Loading

jieyouxu commented Sep 22, 2024

bors commented Sep 22, 2024

This comment has been minimized.

This comment has been minimized.

bors commented Sep 22, 2024

bjorn3 Sep 22, 2024 •

edited

Loading

jieyouxu Sep 28, 2024

jieyouxu commented Sep 28, 2024 •

edited

Loading

jieyouxu Sep 28, 2024

jieyouxu Sep 28, 2024

jieyouxu Oct 11, 2024

jieyouxu Sep 28, 2024 •

edited

Loading

RalfJung Oct 12, 2024

RalfJung Oct 12, 2024

jieyouxu Oct 12, 2024

RalfJung Oct 12, 2024

jieyouxu Oct 12, 2024

jieyouxu commented Oct 12, 2024

workingjubilee commented Oct 12, 2024

jieyouxu commented Oct 12, 2024 •

edited

Loading

jieyouxu commented Oct 12, 2024

RalfJung commented Oct 13, 2024

Urgau commented Oct 13, 2024

workingjubilee commented Oct 13, 2024

workingjubilee commented Oct 13, 2024

Urgau commented Oct 14, 2024

RalfJung commented Oct 14, 2024 via email

Urgau commented Oct 14, 2024 •

edited

Loading

jieyouxu commented Oct 17, 2024 •

edited

Loading

apiraino commented Oct 17, 2024

		// `use-minicore` requires `#![no_std]` and `#![no_core]`, which means no unwinding panics.
		if self.props.use_minicore {

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

Are you sure you want to change the base?

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

Conversation

jieyouxu commented Sep 22, 2024 • edited Loading

New minicore test auxiliary and //@ use-minicore compiletest directive

Implementation

Example usage

Implementation steps

jieyouxu commented Sep 22, 2024

bors commented Sep 22, 2024

This comment has been minimized.

This comment has been minimized.

bors commented Sep 22, 2024

bjorn3 Sep 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jieyouxu commented Sep 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jieyouxu Sep 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jieyouxu commented Oct 12, 2024

workingjubilee commented Oct 12, 2024

jieyouxu commented Oct 12, 2024 • edited Loading

jieyouxu commented Oct 12, 2024

RalfJung commented Oct 13, 2024

Urgau commented Oct 13, 2024

workingjubilee commented Oct 13, 2024

workingjubilee commented Oct 13, 2024

Urgau commented Oct 14, 2024

RalfJung commented Oct 14, 2024 via email

Urgau commented Oct 14, 2024 • edited Loading

jieyouxu commented Oct 17, 2024 • edited Loading

Summary for T-compiler triage

Candidate Approaches

Approach 1: special //@ use-minicore directive

Approach 2: include!("../../../minicore.rs")

Approach 3: retrofit aux-crate directive

Remarks

apiraino commented Oct 17, 2024

Add `minicore` test auxiliary and support `//@ use-minicore` directive in ui/assembly/codegen tests #130693

Add `minicore` test auxiliary and support `//@ use-minicore` directive in ui/assembly/codegen tests #130693

jieyouxu commented Sep 22, 2024 •

edited

Loading

New `minicore` test auxiliary and `//@ use-minicore` compiletest directive

bjorn3 Sep 22, 2024 •

edited

Loading

jieyouxu commented Sep 28, 2024 •

edited

Loading

jieyouxu Sep 28, 2024 •

edited

Loading

jieyouxu commented Oct 12, 2024 •

edited

Loading

Urgau commented Oct 14, 2024 •

edited

Loading

jieyouxu commented Oct 17, 2024 •

edited

Loading

Approach 1: special `//@ use-minicore` directive

Approach 2: `include!("../../../minicore.rs")`

Approach 3: retrofit `aux-crate` directive