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

Fix codegen-units tests that were disabled 8 years ago #128929

Merged
merged 1 commit into from
Aug 12, 2024
Merged

Fix codegen-units tests that were disabled 8 years ago #128929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 7 additions & 1 deletion src/tools/compiletest/src/runtest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3027,11 +3027,17 @@ impl<'test> TestCx<'test> {
const PREFIX: &str = "MONO_ITEM ";
const CGU_MARKER: &str = "@@";

// Some MonoItems can contain {closure@/path/to/checkout/tests/codgen-units/test.rs}
// To prevent the current dir from leaking, we just replace the entire path to the test
// file with TEST_PATH.
let actual: Vec<MonoItem> = proc_res
.stdout
.lines()
.filter(|line| line.starts_with(PREFIX))
.map(|line| str_to_mono_item(line, true))
.map(|line| {
line.replace(&self.testpaths.file.display().to_string(), "TEST_PATH").to_string()
})
Comment on lines +3037 to +3039
Copy link
Member

@jieyouxu jieyouxu Aug 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Discussion (not blocking): I think this is fine for codegen-units tests, but ui tests normalize paths in their output with placeholders like $SRC_DIR, $DIR, $COMPILER_DIR. Should we follow that for consistency?

rust/src/tools/compiletest/src/runtest.rs

Lines 4322 to 4328 in 730d5d4

let base_dir = Path::new("/rustc/FAKE_PREFIX");
// Paths into the libstd/libcore
normalize_path(&base_dir.join("library"), "$SRC_DIR");
// `ui-fulldeps` tests can show paths to the compiler source when testing macros from
// `rustc_macros`
// eg. /home/user/rust/compiler
normalize_path(&base_dir.join("compiler"), "$COMPILER_DIR");

But AFAIK those are only applied when the diagnostic is --format json or something. We probably also have other ways to do normalization for different test suites, it's a bit of a YOLO situation in here.

Copy link
Member

@jieyouxu jieyouxu Aug 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

(unrelated remark I need to breakup / tidy up runtest.rs because the 4700 lines of run test logic is a pain every time I try to look up something)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. We should probably apply all the path normalizations to all tests to be honest. It can be hard to remember how little code sharing there is between test suites.

Copy link
Member

Choose a reason for hiding this comment

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

#128952

.map(|line| str_to_mono_item(&line, true))
.collect();

let expected: Vec<MonoItem> = errors::load_errors(&self.testpaths.file, None)
Expand Down
2 changes: 2 additions & 0 deletions tests/codegen-units/item-collection/auxiliary/cgu_extern_closures.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//@ compile-flags: -Zinline-mir=no

#![crate_type = "lib"]

#[inline]
Expand Down
21 changes: 8 additions & 13 deletions tests/codegen-units/item-collection/cross-crate-closures.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,22 @@
// In the current version of the collector that still has to support
// legacy-codegen, closures do not generate their own MonoItems, so we are
// ignoring this test until MIR codegen has taken over completely
//@ ignore-test

//@ compile-flags:-Zprint-mono-items=eager
// We need to disable MIR inlining in both this and its aux-build crate. The MIR inliner
// will just inline everything into our start function if we let it. As it should.
//@ compile-flags:-Zprint-mono-items=eager -Zinline-mir=no

#![deny(dead_code)]
#![feature(start)]

//@ aux-build:cgu_extern_closures.rs
extern crate cgu_extern_closures;

//~ MONO_ITEM fn cross_crate_closures::start[0]
//~ MONO_ITEM fn start @@ cross_crate_closures-cgu.0[Internal]
#[start]
fn start(_: isize, _: *const *const u8) -> isize {
//~ MONO_ITEM fn cgu_extern_closures::inlined_fn[0]
//~ MONO_ITEM fn cgu_extern_closures::inlined_fn[0]::{{closure}}[0]
//~ MONO_ITEM fn cgu_extern_closures::inlined_fn @@ cross_crate_closures-cgu.0[Internal]
//~ MONO_ITEM fn cgu_extern_closures::inlined_fn::{closure#0} @@ cross_crate_closures-cgu.0[Internal]
let _ = cgu_extern_closures::inlined_fn(1, 2);

//~ MONO_ITEM fn cgu_extern_closures::inlined_fn_generic[0]<i32>
//~ MONO_ITEM fn cgu_extern_closures::inlined_fn_generic[0]::{{closure}}[0]<i32>
//~ MONO_ITEM fn cgu_extern_closures::inlined_fn_generic::<i32> @@ cross_crate_closures-cgu.0[Internal]
//~ MONO_ITEM fn cgu_extern_closures::inlined_fn_generic::<i32>::{closure#0} @@ cross_crate_closures-cgu.0[Internal]
let _ = cgu_extern_closures::inlined_fn_generic(3, 4, 5i32);

// Nothing should be generated for this call, we just link to the instance
Expand All @@ -28,5 +25,3 @@ fn start(_: isize, _: *const *const u8) -> isize {

0
}

//~ MONO_ITEM drop-glue i8
30 changes: 13 additions & 17 deletions tests/codegen-units/item-collection/non-generic-closures.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,49 +1,45 @@
// In the current version of the collector that still has to support
// legacy-codegen, closures do not generate their own MonoItems, so we are
// ignoring this test until MIR codegen has taken over completely
//@ ignore-test

//
//@ compile-flags:-Zprint-mono-items=eager
//@ compile-flags:-Zprint-mono-items=eager -Zinline-mir=no

#![deny(dead_code)]
#![feature(start)]

//~ MONO_ITEM fn non_generic_closures::temporary[0]
//~ MONO_ITEM fn temporary @@ non_generic_closures-cgu.0[Internal]
fn temporary() {
//~ MONO_ITEM fn non_generic_closures::temporary[0]::{{closure}}[0]
//~ MONO_ITEM fn temporary::{closure#0} @@ non_generic_closures-cgu.0[Internal]
(|a: u32| {
let _ = a;
})(4);
}

//~ MONO_ITEM fn non_generic_closures::assigned_to_variable_but_not_executed[0]
//~ MONO_ITEM fn assigned_to_variable_but_not_executed @@ non_generic_closures-cgu.0[Internal]
fn assigned_to_variable_but_not_executed() {
//~ MONO_ITEM fn non_generic_closures::assigned_to_variable_but_not_executed[0]::{{closure}}[0]
let _x = |a: i16| {
let _ = a + 1;
};
}

//~ MONO_ITEM fn non_generic_closures::assigned_to_variable_executed_directly[0]
//~ MONO_ITEM fn assigned_to_variable_executed_indirectly @@ non_generic_closures-cgu.0[Internal]
fn assigned_to_variable_executed_indirectly() {
//~ MONO_ITEM fn non_generic_closures::assigned_to_variable_executed_directly[0]::{{closure}}[0]
//~ MONO_ITEM fn assigned_to_variable_executed_indirectly::{closure#0} @@ non_generic_closures-cgu.0[Internal]
//~ MONO_ITEM fn <{closure@TEST_PATH:27:13: 27:21} as std::ops::FnOnce<(i32,)>>::call_once - shim @@ non_generic_closures-cgu.0[Internal]
//~ MONO_ITEM fn <{closure@TEST_PATH:27:13: 27:21} as std::ops::FnOnce<(i32,)>>::call_once - shim(vtable) @@ non_generic_closures-cgu.0[Internal]
//~ MONO_ITEM fn std::ptr::drop_in_place::<{closure@TEST_PATH:27:13: 27:21}> - shim(None) @@ non_generic_closures-cgu.0[Internal]
let f = |a: i32| {
let _ = a + 2;
};
run_closure(&f);
}

//~ MONO_ITEM fn non_generic_closures::assigned_to_variable_executed_indirectly[0]
//~ MONO_ITEM fn assigned_to_variable_executed_directly @@ non_generic_closures-cgu.0[Internal]
fn assigned_to_variable_executed_directly() {
//~ MONO_ITEM fn non_generic_closures::assigned_to_variable_executed_indirectly[0]::{{closure}}[0]
//~ MONO_ITEM fn assigned_to_variable_executed_directly::{closure#0} @@ non_generic_closures-cgu.0[Internal]
let f = |a: i64| {
let _ = a + 3;
};
f(4);
}

//~ MONO_ITEM fn non_generic_closures::start[0]
//~ MONO_ITEM fn start @@ non_generic_closures-cgu.0[Internal]
#[start]
fn start(_: isize, _: *const *const u8) -> isize {
temporary();
Expand All @@ -54,7 +50,7 @@ fn start(_: isize, _: *const *const u8) -> isize {
0
}

//~ MONO_ITEM fn non_generic_closures::run_closure[0]
//~ MONO_ITEM fn run_closure @@ non_generic_closures-cgu.0[Internal]
fn run_closure(f: &Fn(i32)) {
f(3);
}
42 changes: 15 additions & 27 deletions tests/codegen-units/partitioning/methods-are-with-self-type.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,30 +1,23 @@
// Currently, all generic functions are instantiated in each codegen unit that
// uses them, even those not marked with #[inline], so this test does not make
// much sense at the moment.
//@ ignore-test

// We specify incremental here because we want to test the partitioning for incremental compilation
//@ incremental
//@ compile-flags:-Zprint-mono-items=lazy

#![allow(dead_code)]
#![feature(start)]
#![crate_type = "lib"]

struct SomeType;
pub struct SomeType;

struct SomeGenericType<T1, T2>(T1, T2);

mod mod1 {
pub mod mod1 {
use super::{SomeGenericType, SomeType};

// Even though the impl is in `mod1`, the methods should end up in the
// parent module, since that is where their self-type is.
impl SomeType {
//~ MONO_ITEM fn methods_are_with_self_type::mod1[0]::{{impl}}[0]::method[0] @@ methods_are_with_self_type[External]
fn method(&self) {}

//~ MONO_ITEM fn methods_are_with_self_type::mod1[0]::{{impl}}[0]::associated_fn[0] @@ methods_are_with_self_type[External]
fn associated_fn() {}
//~ MONO_ITEM fn mod1::<impl SomeType>::method @@ methods_are_with_self_type[External]
pub fn method(&self) {}
//~ MONO_ITEM fn mod1::<impl SomeType>::associated_fn @@ methods_are_with_self_type[External]
pub fn associated_fn() {}
}

impl<T1, T2> SomeGenericType<T1, T2> {
Expand Down Expand Up @@ -52,25 +45,20 @@ mod type2 {
pub struct Struct;
}

//~ MONO_ITEM fn methods_are_with_self_type::start[0]
#[start]
fn start(_: isize, _: *const *const u8) -> isize {
//~ MONO_ITEM fn methods_are_with_self_type::mod1[0]::{{impl}}[1]::method[0]<u32, u64> @@ methods_are_with_self_type.volatile[WeakODR]
//~ MONO_ITEM fn start @@ methods_are_with_self_type[External]
pub fn start() {
//~ MONO_ITEM fn mod1::<impl SomeGenericType<u32, u64>>::method @@ methods_are_with_self_type.volatile[External]
SomeGenericType(0u32, 0u64).method();
//~ MONO_ITEM fn methods_are_with_self_type::mod1[0]::{{impl}}[1]::associated_fn[0]<char, &str> @@ methods_are_with_self_type.volatile[WeakODR]
//~ MONO_ITEM fn mod1::<impl SomeGenericType<char, &str>>::associated_fn @@ methods_are_with_self_type.volatile[External]
SomeGenericType::associated_fn('c', "&str");

//~ MONO_ITEM fn methods_are_with_self_type::{{impl}}[0]::foo[0]<methods_are_with_self_type::type1[0]::Struct[0]> @@ methods_are_with_self_type-type1.volatile[WeakODR]
//~ MONO_ITEM fn <type1::Struct as Trait>::foo @@ methods_are_with_self_type-type1.volatile[External]
type1::Struct.foo();
//~ MONO_ITEM fn methods_are_with_self_type::{{impl}}[0]::foo[0]<methods_are_with_self_type::type2[0]::Struct[0]> @@ methods_are_with_self_type-type2.volatile[WeakODR]
//~ MONO_ITEM fn <type2::Struct as Trait>::foo @@ methods_are_with_self_type-type2.volatile[External]
type2::Struct.foo();

//~ MONO_ITEM fn methods_are_with_self_type::Trait[0]::default[0]<methods_are_with_self_type::type1[0]::Struct[0]> @@ methods_are_with_self_type-type1.volatile[WeakODR]
//~ MONO_ITEM fn <type1::Struct as Trait>::default @@ methods_are_with_self_type-type1.volatile[External]
type1::Struct.default();
//~ MONO_ITEM fn methods_are_with_self_type::Trait[0]::default[0]<methods_are_with_self_type::type2[0]::Struct[0]> @@ methods_are_with_self_type-type2.volatile[WeakODR]
//~ MONO_ITEM fn <type2::Struct as Trait>::default @@ methods_are_with_self_type-type2.volatile[External]
type2::Struct.default();

0
}

//~ MONO_ITEM drop-glue i8
Loading