diff --git a/internal/compiler/passes/remove_return.rs b/internal/compiler/passes/remove_return.rs index 54ecba69d6b..dcb34ae667c 100644 --- a/internal/compiler/passes/remove_return.rs +++ b/internal/compiler/passes/remove_return.rs @@ -49,52 +49,7 @@ fn process_expression( process_codeblock(expr.into_iter().peekable(), toplevel, ty, ctx, symbol_counters) } Expression::Condition { condition, true_expr, false_expr } => { - let te = process_expression(*true_expr, false, ctx, ty, symbol_counters); - let fe = process_expression(*false_expr, false, ctx, ty, symbol_counters); - match (te, fe) { - (ExpressionResult::Just(te), ExpressionResult::Just(fe)) => { - Expression::Condition { condition, true_expr: te.into(), false_expr: fe.into() } - .into() - } - (ExpressionResult::Just(te), ExpressionResult::Return(fe)) => { - ExpressionResult::MaybeReturn { - pre_statements: Vec::new(), - condition: *condition, - returned_value: fe, - actual_value: cleanup_empty_block(te), - } - } - (ExpressionResult::Return(te), ExpressionResult::Just(fe)) => { - ExpressionResult::MaybeReturn { - pre_statements: Vec::new(), - condition: Expression::UnaryOp { sub: condition, op: '!' }, - returned_value: te, - actual_value: cleanup_empty_block(fe), - } - } - (ExpressionResult::Return(te), ExpressionResult::Return(fe)) => { - ExpressionResult::Return(Some(Expression::Condition { - condition, - true_expr: te.unwrap_or(Expression::CodeBlock(Vec::new())).into(), - false_expr: fe.unwrap_or(Expression::CodeBlock(Vec::new())).into(), - })) - } - (te, fe) => { - let has_value = has_value(ty) && (te.has_value() || fe.has_value()); - let ty = if has_value { ty } else { &Type::Void }; - let te = te.into_return_object(ty, &ctx.ret_ty, symbol_counters); - let fe = fe.into_return_object(ty, &ctx.ret_ty, symbol_counters); - ExpressionResult::ReturnObject { - has_value, - has_return_value: self::has_value(&ctx.ret_ty), - value: Expression::Condition { - condition, - true_expr: te.into(), - false_expr: fe.into(), - }, - } - } - } + process_condition(condition, true_expr, false_expr, ctx, ty, symbol_counters) } Expression::Cast { from, to } => { let ty = if !has_value(ty) { ty.clone() } else { from.ty() }; @@ -102,58 +57,7 @@ fn process_expression( .map_value(symbol_counters, |e| Expression::Cast { from: e.into(), to }) } Expression::StoreLocalVariable { name, value } => { - let inner_ty = value.ty(); - match process_expression(*value, false, ctx, &inner_ty, symbol_counters) { - ExpressionResult::Just(e) => { - ExpressionResult::Just(Expression::StoreLocalVariable { - name, - value: Box::new(e), - }) - } - ExpressionResult::Return(r) => ExpressionResult::Return(r), - ExpressionResult::MaybeReturn { - pre_statements, - condition, - returned_value, - actual_value, - } => ExpressionResult::MaybeReturn { - pre_statements, - condition, - returned_value, - actual_value: Some(Expression::StoreLocalVariable { - name, - value: Box::new( - actual_value.unwrap_or(Expression::default_value_for_type(&inner_ty)), - ), - }), - }, - ExpressionResult::ReturnObject { value, has_return_value, .. } => { - let tmp_name: SmolStr = symbol_counters.generate_name("return_check_store"); - let value_ty = value.ty(); - let load = |field: &str| Expression::StructFieldAccess { - base: Box::new(Expression::ReadLocalVariable { - name: tmp_name.clone(), - ty: value_ty.clone(), - }), - name: field.into(), - }; - let condition = load(FIELD_CONDITION); - let returned_value = has_return_value.then(|| load(FIELD_RETURNED)); - let actual_value = Some(Expression::StoreLocalVariable { - name, - value: Box::new(load(FIELD_ACTUAL)), - }); - ExpressionResult::MaybeReturn { - pre_statements: vec![Expression::StoreLocalVariable { - name: tmp_name, - value: Box::new(value), - }], - condition, - returned_value, - actual_value, - } - } - } + process_store_local_variable(name, value, ctx, symbol_counters) } e => { // Normally there shouldn't be any 'return' statements in there since return are not allowed in arbitrary expressions @@ -166,6 +70,127 @@ fn process_expression( } } +fn process_condition( + condition: Box, + true_expr: Box, + false_expr: Box, + ctx: &RemoveReturnContext, + ty: &Type, + symbol_counters: &SymbolCounters, +) -> ExpressionResult { + let te = process_expression(*true_expr, false, ctx, ty, symbol_counters); + let fe = process_expression(*false_expr, false, ctx, ty, symbol_counters); + merge_condition_branches(condition, te, fe, ctx, ty, symbol_counters) +} + +fn merge_condition_branches( + condition: Box, + te: ExpressionResult, + fe: ExpressionResult, + ctx: &RemoveReturnContext, + ty: &Type, + symbol_counters: &SymbolCounters, +) -> ExpressionResult { + match (te, fe) { + (ExpressionResult::Just(te), ExpressionResult::Just(fe)) => { + Expression::Condition { condition, true_expr: te.into(), false_expr: fe.into() }.into() + } + (ExpressionResult::Just(te), ExpressionResult::Return(fe)) => { + ExpressionResult::MaybeReturn { + pre_statements: Vec::new(), + condition: *condition, + returned_value: fe, + actual_value: cleanup_empty_block(te), + } + } + (ExpressionResult::Return(te), ExpressionResult::Just(fe)) => { + ExpressionResult::MaybeReturn { + pre_statements: Vec::new(), + condition: Expression::UnaryOp { sub: condition, op: '!' }, + returned_value: te, + actual_value: cleanup_empty_block(fe), + } + } + (ExpressionResult::Return(te), ExpressionResult::Return(fe)) => { + ExpressionResult::Return(Some(Expression::Condition { + condition, + true_expr: te.unwrap_or(Expression::CodeBlock(Vec::new())).into(), + false_expr: fe.unwrap_or(Expression::CodeBlock(Vec::new())).into(), + })) + } + (te, fe) => { + let has_value = has_value(ty) && (te.has_value() || fe.has_value()); + let ty = if has_value { ty } else { &Type::Void }; + let te = te.into_return_object(ty, &ctx.ret_ty, symbol_counters); + let fe = fe.into_return_object(ty, &ctx.ret_ty, symbol_counters); + ExpressionResult::ReturnObject { + has_value, + has_return_value: self::has_value(&ctx.ret_ty), + value: Expression::Condition { + condition, + true_expr: te.into(), + false_expr: fe.into(), + }, + } + } + } +} + +fn process_store_local_variable( + name: SmolStr, + value: Box, + ctx: &RemoveReturnContext, + symbol_counters: &SymbolCounters, +) -> ExpressionResult { + let inner_ty = value.ty(); + match process_expression(*value, false, ctx, &inner_ty, symbol_counters) { + ExpressionResult::Just(e) => { + ExpressionResult::Just(Expression::StoreLocalVariable { name, value: Box::new(e) }) + } + ExpressionResult::Return(r) => ExpressionResult::Return(r), + ExpressionResult::MaybeReturn { + pre_statements, + condition, + returned_value, + actual_value, + } => ExpressionResult::MaybeReturn { + pre_statements, + condition, + returned_value, + actual_value: Some(Expression::StoreLocalVariable { + name, + value: Box::new( + actual_value.unwrap_or(Expression::default_value_for_type(&inner_ty)), + ), + }), + }, + ExpressionResult::ReturnObject { value, has_return_value, .. } => { + let tmp_name: SmolStr = symbol_counters.generate_name("return_check_store"); + let value_ty = value.ty(); + let load = |field: &str| Expression::StructFieldAccess { + base: Box::new(Expression::ReadLocalVariable { + name: tmp_name.clone(), + ty: value_ty.clone(), + }), + name: field.into(), + }; + let condition = load(FIELD_CONDITION); + let returned_value = has_return_value.then(|| load(FIELD_RETURNED)); + let actual_value = + Some(Expression::StoreLocalVariable { name, value: Box::new(load(FIELD_ACTUAL)) }); + ExpressionResult::MaybeReturn { + pre_statements: vec![Expression::StoreLocalVariable { + name: tmp_name, + value: Box::new(value), + }], + condition, + returned_value, + actual_value, + } + } + } +} + /// Return the expression, unless it is an empty codeblock, then return None fn cleanup_empty_block(te: Expression) -> Option { if matches!(&te, Expression::CodeBlock(stmts) if stmts.is_empty()) { None } else { Some(te) } diff --git a/tests/Cargo.lock b/tests/Cargo.lock index cab28ad7d6a..38ac328a9ff 100644 --- a/tests/Cargo.lock +++ b/tests/Cargo.lock @@ -5456,6 +5456,7 @@ dependencies = [ "i-slint-backend-testing", "i-slint-compiler", "i-slint-core", + "rayon", "slint", "slint-interpreter", "spin_on", diff --git a/tests/Cargo.toml b/tests/Cargo.toml index 839f5e03ae9..0de90cd087a 100644 --- a/tests/Cargo.toml +++ b/tests/Cargo.toml @@ -61,6 +61,7 @@ i-slint-renderer-skia = { version = "=1.18.0", path = "../internal/renderers/ski image = { version = "0.25", default-features = false, features = ["png", "jpeg"] } itertools = { version = "0.15" } +rayon = { version = "1.10", default-features = false } spin_on = { version = "0.1" } [profile.release] diff --git a/tests/driver/rust/Cargo.toml b/tests/driver/rust/Cargo.toml index d36e63de552..1b6f6f76b83 100644 --- a/tests/driver/rust/Cargo.toml +++ b/tests/driver/rust/Cargo.toml @@ -39,5 +39,6 @@ i-slint-backend-qt = { workspace = true } [build-dependencies] i-slint-compiler = { workspace = true, features = ["default", "rust", "display-diagnostics", "bundle-translations"], optional = true } +rayon = { workspace = true } spin_on = { workspace = true, optional = true } test_driver_lib = { path = "../driverlib" } diff --git a/tests/driver/rust/build.rs b/tests/driver/rust/build.rs index d0b6cdc9c73..4ce1367c90f 100644 --- a/tests/driver/rust/build.rs +++ b/tests/driver/rust/build.rs @@ -1,6 +1,7 @@ // Copyright © SixtyFPS GmbH // SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0 +use rayon::prelude::*; use std::collections::HashMap; use std::ffi::{OsStr, OsString}; use std::fs::File; @@ -94,72 +95,28 @@ fn main() -> std::io::Result<()> { let mut generated_files = make_generator_files()?; - for testcase in test_driver_lib::collect_test_cases("cases")? { + let testcases = test_driver_lib::collect_test_cases("cases")?; + + // Generate the per-case modules on all cores: with the build-time feature, + // each case runs the Slint compiler (twice with deterministic-output), + // which dominates the build script's runtime. + let module_lines = rayon::ThreadPoolBuilder::new() + .stack_size(512 * 1024) + .build() + .expect("failed to create thread pool") + .install(|| { + testcases + .par_iter() + .map(|testcase| process_case(testcase, live_preview)) + .collect::>>() + })?; + + // Write the module declarations serially, in collection order, so the + // including files don't depend on thread scheduling. + for (testcase, module_line) in testcases.iter().zip(module_lines) { let generated_file = - generated_file_for_test(&testcase, &mut generated_files, &mut generated_file)?; - - println!("cargo:rerun-if-changed={}", testcase.absolute_path.display()); - let mut module_name = testcase.identifier(); - if module_name.starts_with(|c: char| !c.is_ascii_alphabetic()) { - module_name.insert(0, '_'); - } - writeln!(generated_file, "#[path=\"{module_name}.rs\"] pub mod r#{module_name};")?; - let source = std::fs::read_to_string(&testcase.absolute_path)?; - let ignored = if testcase.is_ignored("rust") { - "#[ignore = \"testcase ignored for rust\"]" - } else if (cfg!(not(feature = "build-time")) || live_preview) - && source.contains("//bundle-translations") - { - "#[ignore = \"translation bundle not working with the macro\"]" - } else if live_preview && testcase.is_ignored("js") { - "#[ignore = \"Ignored JS testcases ignored in live-preview mode\"]" - } else if live_preview && testcase.is_ignored("live-preview") { - "#[ignore = \"testcase ignored in live-preview mode\"]" - } else if live_preview && source.contains("#3464") { - "#[ignore = \"issue #3464 not fixed with the interpreter\"]" - } else if live_preview && module_name.contains("write_to_model") { - "#[ignore = \"Interpreted model don't forward to underlying models for anonymous structs\"]" - } else { - "" - }; - - let mut output = BufWriter::new(File::create( - Path::new(&std::env::var_os("OUT_DIR").unwrap()).join(format!("{module_name}.rs")), - )?); - - output.write_all( - b"#![deny(warnings)]\n#![deny(rust_2018_idioms)]\n#![deny(unsafe_code)]\n", - )?; - - #[cfg(not(feature = "build-time"))] - if !generate_macro(&source, &mut output, testcase)? { - continue; - } - #[cfg(feature = "build-time")] - generate_source(&source, &mut output, testcase)?; - - for (i, x) in test_driver_lib::extract_test_functions(&source) - .filter(|x| x.language_id == "rust") - .enumerate() - { - write!( - output, - r" -#[rust_analyzer::skip] -#[test] {} fn t_{}() -> ::std::result::Result<(), ::std::boxed::Box> {{ - use i_slint_backend_testing as slint_testing; - slint_testing::init_no_event_loop(); - slint_testing::configure_test_fonts(); - {} - Ok(()) -}}", - ignored, - i, - x.source.replace('\n', "\n ") - )?; - } - - output.flush()?; + generated_file_for_test(testcase, &mut generated_files, &mut generated_file)?; + writeln!(generated_file, "{module_line}")?; } generated_file.flush()?; @@ -180,11 +137,81 @@ fn main() -> std::io::Result<()> { Ok(()) } +/// Write the `{module_name}.rs` file for the test case into OUT_DIR and return +/// the module declaration that includes it. +fn process_case( + testcase: &test_driver_lib::TestCase, + live_preview: bool, +) -> std::io::Result { + println!("cargo:rerun-if-changed={}", testcase.absolute_path.display()); + let mut module_name = testcase.identifier(); + if module_name.starts_with(|c: char| !c.is_ascii_alphabetic()) { + module_name.insert(0, '_'); + } + let module_line = format!("#[path=\"{module_name}.rs\"] pub mod r#{module_name};"); + let source = std::fs::read_to_string(&testcase.absolute_path)?; + let ignored = if testcase.is_ignored("rust") { + "#[ignore = \"testcase ignored for rust\"]" + } else if (cfg!(not(feature = "build-time")) || live_preview) + && source.contains("//bundle-translations") + { + "#[ignore = \"translation bundle not working with the macro\"]" + } else if live_preview && testcase.is_ignored("js") { + "#[ignore = \"Ignored JS testcases ignored in live-preview mode\"]" + } else if live_preview && testcase.is_ignored("live-preview") { + "#[ignore = \"testcase ignored in live-preview mode\"]" + } else if live_preview && source.contains("#3464") { + "#[ignore = \"issue #3464 not fixed with the interpreter\"]" + } else if live_preview && module_name.contains("write_to_model") { + "#[ignore = \"Interpreted model don't forward to underlying models for anonymous structs\"]" + } else { + "" + }; + + let mut output = BufWriter::new(File::create( + Path::new(&std::env::var_os("OUT_DIR").unwrap()).join(format!("{module_name}.rs")), + )?); + + output.write_all(b"#![deny(warnings)]\n#![deny(rust_2018_idioms)]\n#![deny(unsafe_code)]\n")?; + + #[cfg(not(feature = "build-time"))] + if !generate_macro(&source, &mut output, testcase)? { + output.flush()?; + return Ok(module_line); + } + #[cfg(feature = "build-time")] + generate_source(&source, &mut output, testcase)?; + + for (i, x) in test_driver_lib::extract_test_functions(&source) + .filter(|x| x.language_id == "rust") + .enumerate() + { + write!( + output, + r" +#[rust_analyzer::skip] +#[test] {} fn t_{}() -> ::std::result::Result<(), ::std::boxed::Box> {{ + use i_slint_backend_testing as slint_testing; + slint_testing::init_no_event_loop(); + slint_testing::configure_test_fonts(); + {} + Ok(()) +}}", + ignored, + i, + x.source.replace('\n', "\n ") + )?; + } + + output.flush()?; + Ok(module_line) +} + #[cfg(not(feature = "build-time"))] fn generate_macro( source: &str, output: &mut dyn Write, - testcase: test_driver_lib::TestCase, + testcase: &test_driver_lib::TestCase, ) -> Result { if source.contains("\\{") { // Unfortunately, \{ is not valid in a rust string so it cannot be used in a slint! macro @@ -227,7 +254,7 @@ fn generate_macro( output.write_all(b"\"#]\n")?; } - let mut abs_path = testcase.absolute_path; + let mut abs_path = testcase.absolute_path.clone(); abs_path.pop(); output.write_all(b"#[include_path=r#\"")?; output.write_all(abs_path.to_string_lossy().as_bytes())?; @@ -241,15 +268,15 @@ fn generate_macro( fn generate_source( source: &str, output: &mut impl Write, - testcase: test_driver_lib::TestCase, + testcase: &test_driver_lib::TestCase, ) -> Result<(), std::io::Error> { println!("cargo::rerun-if-env-changed=SLINT_LIVE_PREVIEW"); - let generated = compile_and_generate(source, &testcase)?; + let generated = compile_and_generate(source, testcase)?; #[cfg(feature = "deterministic-output")] { - let second = compile_and_generate(source, &testcase)?; + let second = compile_and_generate(source, testcase)?; let expect_utf8 = |bytes| std::str::from_utf8(bytes).expect("generated Rust is valid UTF-8"); assert_eq!( @@ -268,24 +295,6 @@ fn generate_source( fn compile_and_generate( source: &str, testcase: &test_driver_lib::TestCase, -) -> Result, std::io::Error> { - // Run the compiler in a thread with a reduced stack size to catch excessive - // stack usage, which would break compilation in environments with small - // default stack sizes. - std::thread::scope(|scope| { - std::thread::Builder::new() - .stack_size(512 * 1024) - .spawn_scoped(scope, || compile_and_generate_impl(source, testcase)) - .expect("failed to spawn compiler thread") - .join() - .expect("compiler thread panicked") - }) -} - -#[cfg(feature = "build-time")] -fn compile_and_generate_impl( - source: &str, - testcase: &test_driver_lib::TestCase, ) -> Result, std::io::Error> { use i_slint_compiler::{diagnostics::BuildDiagnostics, *};