From 4cf5683ee8f5884df42db28a7c05b14ac98fd240 Mon Sep 17 00:00:00 2001 From: Brendan Hansknecht Date: Wed, 11 Dec 2024 14:55:32 -0800 Subject: [PATCH 1/4] Always generate `dbg` expressions --- crates/compiler/gen_llvm/src/llvm/build.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/crates/compiler/gen_llvm/src/llvm/build.rs b/crates/compiler/gen_llvm/src/llvm/build.rs index 9b1e8f5e9ea..729c731b64c 100644 --- a/crates/compiler/gen_llvm/src/llvm/build.rs +++ b/crates/compiler/gen_llvm/src/llvm/build.rs @@ -3559,12 +3559,10 @@ pub(crate) fn build_exp_stmt<'a, 'ctx>( variable: _, remainder, } => { - if env.mode.runs_expects() { - let location = build_string_literal(env, source_location); - let source = build_string_literal(env, source); - let message = scope.load_symbol(symbol); - env.call_dbg(env, location, source, message); - } + let location = build_string_literal(env, source_location); + let source = build_string_literal(env, source); + let message = scope.load_symbol(symbol); + env.call_dbg(env, location, source, message); build_exp_stmt( env, From 8068fa6d1bbd0d666697271028958cfe3dc5ffaf Mon Sep 17 00:00:00 2001 From: Brendan Hansknecht Date: Wed, 11 Dec 2024 14:56:12 -0800 Subject: [PATCH 2/4] still run in BinaryDev mode with `roc main.roc` for expect messages --- crates/cli/src/lib.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/cli/src/lib.rs b/crates/cli/src/lib.rs index 3023944cea8..9522605d39c 100644 --- a/crates/cli/src/lib.rs +++ b/crates/cli/src/lib.rs @@ -875,16 +875,17 @@ pub fn build( // Note: This allows using `--dev` with `--optimize`. // This means frontend optimizations and dev backend. - let code_gen_backend = if matches.get_flag(FLAG_DEV) { + let code_gen_backend = if let OptLevel::Development = opt_level { if matches!(target.architecture(), Architecture::Wasm32) { CodeGenBackend::Wasm } else { CodeGenBackend::Assembly(AssemblyBackendMode::Binary) } } else { - let backend_mode = match opt_level { - OptLevel::Development => LlvmBackendMode::BinaryDev, - OptLevel::Normal | OptLevel::Size | OptLevel::Optimize => LlvmBackendMode::Binary, + let backend_mode = if let BuildConfig::BuildAndRunIfNoErrors = config { + LlvmBackendMode::BinaryDev + } else { + LlvmBackendMode::Binary }; CodeGenBackend::Llvm(backend_mode) From 37c6330c6f98c9dd8a692026bf195abc23c9b3d0 Mon Sep 17 00:00:00 2001 From: Brendan Hansknecht Date: Wed, 11 Dec 2024 15:15:46 -0800 Subject: [PATCH 3/4] cleanup when expects are run --- crates/cli/src/lib.rs | 49 ++++++++++---------- crates/compiler/gen_llvm/src/llvm/build.rs | 30 ++++++------ crates/compiler/gen_llvm/src/llvm/expect.rs | 2 +- crates/compiler/test_gen/src/helpers/llvm.rs | 2 +- 4 files changed, 43 insertions(+), 40 deletions(-) diff --git a/crates/cli/src/lib.rs b/crates/cli/src/lib.rs index 9522605d39c..82e06fa6eb8 100644 --- a/crates/cli/src/lib.rs +++ b/crates/cli/src/lib.rs @@ -873,17 +873,22 @@ pub fn build( let opt_level = opt_level_from_flags(matches); - // Note: This allows using `--dev` with `--optimize`. - // This means frontend optimizations and dev backend. - let code_gen_backend = if let OptLevel::Development = opt_level { + let should_run_expects = matches!(opt_level, OptLevel::Development | OptLevel::Normal) && + // TODO: once expect is decoupled from roc launching the executable, remove this part of the conditional. + matches!( + config, + BuildConfig::BuildAndRun | BuildConfig::BuildAndRunIfNoErrors + ); + + let code_gen_backend = if matches!(opt_level, OptLevel::Development) { if matches!(target.architecture(), Architecture::Wasm32) { CodeGenBackend::Wasm } else { CodeGenBackend::Assembly(AssemblyBackendMode::Binary) } } else { - let backend_mode = if let BuildConfig::BuildAndRunIfNoErrors = config { - LlvmBackendMode::BinaryDev + let backend_mode = if should_run_expects { + LlvmBackendMode::BinaryWithExpect } else { LlvmBackendMode::Binary }; @@ -1023,7 +1028,7 @@ pub fn build( roc_run( &arena, path, - opt_level, + should_run_expects, target, args, bytes, @@ -1066,7 +1071,7 @@ pub fn build( roc_run( &arena, path, - opt_level, + should_run_expects, target, args, bytes, @@ -1085,7 +1090,7 @@ pub fn build( fn roc_run<'a, I: IntoIterator>( arena: &Bump, script_path: &Path, - opt_level: OptLevel, + should_run_expects: bool, target: Target, args: I, binary_bytes: &[u8], @@ -1127,7 +1132,7 @@ fn roc_run<'a, I: IntoIterator>( _ => roc_run_native( arena, script_path, - opt_level, + should_run_expects, args, binary_bytes, expect_metadata, @@ -1195,7 +1200,7 @@ fn make_argv_envp<'a, I: IntoIterator, S: AsRef>( fn roc_run_native, S: AsRef>( arena: &Bump, script_path: &Path, - opt_level: OptLevel, + should_run_expects: bool, args: I, binary_bytes: &[u8], expect_metadata: ExpectMetadata, @@ -1217,11 +1222,10 @@ fn roc_run_native, S: AsRef>( .chain([std::ptr::null()]) .collect_in(arena); - match opt_level { - OptLevel::Development => roc_dev_native(arena, executable, argv, envp, expect_metadata), - OptLevel::Normal | OptLevel::Size | OptLevel::Optimize => unsafe { - roc_run_native_fast(executable, &argv, &envp); - }, + if should_run_expects { + roc_dev_native(arena, executable, argv, envp, expect_metadata); + } else { + unsafe { roc_run_native_fast(executable, &argv, &envp) }; } Ok(1) @@ -1459,7 +1463,7 @@ fn roc_run_executable_file_path(binary_bytes: &[u8]) -> std::io::Result, S: AsRef>( arena: &Bump, // This should be passed an owned value, not a reference, so we can usefully mem::forget it! script_path: &Path, - opt_level: OptLevel, + should_run_expects: bool, args: I, binary_bytes: &[u8], _expect_metadata: ExpectMetadata, @@ -1484,14 +1488,11 @@ fn roc_run_native, S: AsRef>( .chain([std::ptr::null()]) .collect_in(arena); - match opt_level { - OptLevel::Development => { - // roc_run_native_debug(executable, &argv, &envp, expectations, interns) - internal_error!("running `expect`s does not currently work on windows") - } - OptLevel::Normal | OptLevel::Size | OptLevel::Optimize => { - roc_run_native_fast(executable, &argv, &envp); - } + if should_run_expects { + // roc_run_native_debug(executable, &argv, &envp, expectations, interns) + internal_error!("running `expect`s does not currently work on windows"); + } else { + roc_run_native_fast(executable, &argv, &envp); } } diff --git a/crates/compiler/gen_llvm/src/llvm/build.rs b/crates/compiler/gen_llvm/src/llvm/build.rs index 729c731b64c..023ca5c13e5 100644 --- a/crates/compiler/gen_llvm/src/llvm/build.rs +++ b/crates/compiler/gen_llvm/src/llvm/build.rs @@ -685,7 +685,7 @@ macro_rules! debug_info_init { pub enum LlvmBackendMode { /// Assumes primitives (roc_alloc, roc_panic, etc) are provided by the host Binary, - BinaryDev, + BinaryWithExpect, /// Creates a test wrapper around the main roc function to catch and report panics. /// Provides a testing implementation of primitives (roc_alloc, roc_panic, etc) BinaryGlue, @@ -698,7 +698,7 @@ impl LlvmBackendMode { pub(crate) fn has_host(self) -> bool { match self { LlvmBackendMode::Binary => true, - LlvmBackendMode::BinaryDev => true, + LlvmBackendMode::BinaryWithExpect => true, LlvmBackendMode::BinaryGlue => false, LlvmBackendMode::GenTest => false, LlvmBackendMode::WasmGenTest => true, @@ -710,7 +710,7 @@ impl LlvmBackendMode { fn returns_roc_result(self) -> bool { match self { LlvmBackendMode::Binary => false, - LlvmBackendMode::BinaryDev => false, + LlvmBackendMode::BinaryWithExpect => false, LlvmBackendMode::BinaryGlue => true, LlvmBackendMode::GenTest => true, LlvmBackendMode::WasmGenTest => true, @@ -721,7 +721,7 @@ impl LlvmBackendMode { pub(crate) fn runs_expects(self) -> bool { match self { LlvmBackendMode::Binary => false, - LlvmBackendMode::BinaryDev => true, + LlvmBackendMode::BinaryWithExpect => true, LlvmBackendMode::BinaryGlue => false, LlvmBackendMode::GenTest => false, LlvmBackendMode::WasmGenTest => false, @@ -3618,7 +3618,7 @@ pub(crate) fn build_exp_stmt<'a, 'ctx>( variables, ); - if let LlvmBackendMode::BinaryDev = env.mode { + if let LlvmBackendMode::BinaryWithExpect = env.mode { crate::llvm::expect::notify_parent_expect(env, &shared_memory); } @@ -4901,7 +4901,9 @@ fn expose_function_to_host_help_c_abi<'a, 'ctx>( ) } - LlvmBackendMode::Binary | LlvmBackendMode::BinaryDev | LlvmBackendMode::BinaryGlue => {} + LlvmBackendMode::Binary + | LlvmBackendMode::BinaryWithExpect + | LlvmBackendMode::BinaryGlue => {} } // a generic version that writes the result into a passed *u8 pointer @@ -4954,13 +4956,13 @@ fn expose_function_to_host_help_c_abi<'a, 'ctx>( roc_call_result_type(env, roc_function.get_type().get_return_type().unwrap()).into() } - LlvmBackendMode::Binary | LlvmBackendMode::BinaryDev | LlvmBackendMode::BinaryGlue => { - basic_type_from_layout( - env, - layout_interner, - layout_interner.get_repr(return_layout), - ) - } + LlvmBackendMode::Binary + | LlvmBackendMode::BinaryWithExpect + | LlvmBackendMode::BinaryGlue => basic_type_from_layout( + env, + layout_interner, + layout_interner.get_repr(return_layout), + ), }; let size: BasicValueEnum = return_type.size_of().unwrap().into(); @@ -5757,7 +5759,7 @@ fn build_procedures_help<'a>( use LlvmBackendMode::*; match env.mode { GenTest | WasmGenTest | CliTest => { /* no host, or exposing types is not supported */ } - Binary | BinaryDev | BinaryGlue => { + Binary | BinaryWithExpect | BinaryGlue => { for (proc_name, alias_name, hels) in host_exposed_lambda_sets.iter() { let ident_string = proc_name.name().as_unsuffixed_str(&env.interns); let fn_name: String = format!("{}_{}", ident_string, hels.id.0); diff --git a/crates/compiler/gen_llvm/src/llvm/expect.rs b/crates/compiler/gen_llvm/src/llvm/expect.rs index a51101906c7..84b6324776b 100644 --- a/crates/compiler/gen_llvm/src/llvm/expect.rs +++ b/crates/compiler/gen_llvm/src/llvm/expect.rs @@ -29,7 +29,7 @@ pub(crate) struct SharedMemoryPointer<'ctx>(PointerValue<'ctx>); impl<'ctx> SharedMemoryPointer<'ctx> { pub(crate) fn get<'a, 'env>(env: &Env<'a, 'ctx, 'env>) -> Self { - let start_function = if let LlvmBackendMode::BinaryDev = env.mode { + let start_function = if let LlvmBackendMode::BinaryWithExpect = env.mode { bitcode::UTILS_EXPECT_FAILED_START_SHARED_FILE } else { bitcode::UTILS_EXPECT_FAILED_START_SHARED_BUFFER diff --git a/crates/compiler/test_gen/src/helpers/llvm.rs b/crates/compiler/test_gen/src/helpers/llvm.rs index 91ed28dcfc5..db1f84fad37 100644 --- a/crates/compiler/test_gen/src/helpers/llvm.rs +++ b/crates/compiler/test_gen/src/helpers/llvm.rs @@ -257,7 +257,7 @@ fn create_llvm_module<'a>( }; let (main_fn_name, main_fn) = match config.mode { LlvmBackendMode::Binary => unreachable!(), - LlvmBackendMode::BinaryDev => unreachable!(), + LlvmBackendMode::BinaryWithExpect => unreachable!(), LlvmBackendMode::BinaryGlue => unreachable!(), LlvmBackendMode::CliTest => unreachable!(), LlvmBackendMode::WasmGenTest => roc_gen_llvm::llvm::build::build_wasm_test_wrapper( From 3f2117403e44b9dc488f1d58d94bf8076934f968 Mon Sep 17 00:00:00 2001 From: Joshua Warner Date: Wed, 11 Dec 2024 18:39:15 -0800 Subject: [PATCH 4/4] Fix bug found in the wild via fuzzing, move fuzzing job to the end --- .github/workflows/ubuntu_x86_64.yml | 10 ++-- crates/compiler/fmt/src/expr.rs | 1 + .../num_bang_amp_z_dot_t.expr.formatted.roc | 2 + .../pass/num_bang_amp_z_dot_t.expr.result-ast | 49 +++++++++++++++++++ .../pass/num_bang_amp_z_dot_t.expr.roc | 3 ++ .../test_syntax/tests/test_snapshots.rs | 1 + 6 files changed, 61 insertions(+), 5 deletions(-) create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/num_bang_amp_z_dot_t.expr.formatted.roc create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/num_bang_amp_z_dot_t.expr.result-ast create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/num_bang_amp_z_dot_t.expr.roc diff --git a/.github/workflows/ubuntu_x86_64.yml b/.github/workflows/ubuntu_x86_64.yml index dd639ce840f..70199def20c 100644 --- a/.github/workflows/ubuntu_x86_64.yml +++ b/.github/workflows/ubuntu_x86_64.yml @@ -29,11 +29,6 @@ jobs: - name: roc format check on builtins run: cargo run --locked --release format --check crates/compiler/builtins/roc - - name: run fuzz tests - run: | - cargo +nightly-2024-02-03 install --locked cargo-fuzz - cd crates/compiler/test_syntax/fuzz && cargo +nightly-2024-02-03 fuzz run -j4 fuzz_expr --sanitizer=none -- -dict=dict.txt -max_total_time=60 - - name: ensure there are no unused dependencies run: cargo +nightly-2024-02-03 udeps --all-targets @@ -67,3 +62,8 @@ jobs: - name: test website build script run: bash www/build.sh + + - name: run fuzz tests - ok to merge if this one fails # for now! + run: | + cargo +nightly-2024-02-03 install --locked cargo-fuzz + cd crates/compiler/test_syntax/fuzz && cargo +nightly-2024-02-03 fuzz run -j4 fuzz_expr --sanitizer=none -- -dict=dict.txt -max_total_time=60 diff --git a/crates/compiler/fmt/src/expr.rs b/crates/compiler/fmt/src/expr.rs index 4e3ce09e8aa..ee3d924a88c 100644 --- a/crates/compiler/fmt/src/expr.rs +++ b/crates/compiler/fmt/src/expr.rs @@ -513,6 +513,7 @@ fn requires_space_after_unary(item: &Expr<'_>) -> bool { is_negative, } => *is_negative, Expr::RecordUpdater(..) => true, + Expr::RecordAccess(inner, _field) => requires_space_after_unary(inner), Expr::Apply(inner, _, _) => requires_space_after_unary(&inner.value), Expr::TrySuffix { target: _, expr } => requires_space_after_unary(expr), Expr::SpaceAfter(inner, _) | Expr::SpaceBefore(inner, _) => { diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/num_bang_amp_z_dot_t.expr.formatted.roc b/crates/compiler/test_syntax/tests/snapshots/pass/num_bang_amp_z_dot_t.expr.formatted.roc new file mode 100644 index 00000000000..3da2a32ce35 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/num_bang_amp_z_dot_t.expr.formatted.roc @@ -0,0 +1,2 @@ +4 +! &z.t \ No newline at end of file diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/num_bang_amp_z_dot_t.expr.result-ast b/crates/compiler/test_syntax/tests/snapshots/pass/num_bang_amp_z_dot_t.expr.result-ast new file mode 100644 index 00000000000..982b5110fc8 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/num_bang_amp_z_dot_t.expr.result-ast @@ -0,0 +1,49 @@ +@0-8 SpaceAfter( + Defs( + Defs { + tags: [ + EitherIndex(2147483648), + ], + regions: [ + @0-1, + ], + space_before: [ + Slice { start: 0, length: 0 }, + ], + space_after: [ + Slice { start: 0, length: 0 }, + ], + spaces: [], + type_defs: [], + value_defs: [ + Stmt( + @0-1 Num( + "4", + ), + ), + ], + }, + @2-8 SpaceBefore( + UnaryOp( + @4-6 SpaceBefore( + RecordAccess( + RecordUpdater( + "z", + ), + "t", + ), + [ + Newline, + ], + ), + @2-3 Not, + ), + [ + Newline, + ], + ), + ), + [ + Newline, + ], +) diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/num_bang_amp_z_dot_t.expr.roc b/crates/compiler/test_syntax/tests/snapshots/pass/num_bang_amp_z_dot_t.expr.roc new file mode 100644 index 00000000000..63d71bb5dc5 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/num_bang_amp_z_dot_t.expr.roc @@ -0,0 +1,3 @@ +4 +! +&z.t diff --git a/crates/compiler/test_syntax/tests/test_snapshots.rs b/crates/compiler/test_syntax/tests/test_snapshots.rs index 6e852a213fe..b1ae9285588 100644 --- a/crates/compiler/test_syntax/tests/test_snapshots.rs +++ b/crates/compiler/test_syntax/tests/test_snapshots.rs @@ -514,6 +514,7 @@ mod test_snapshots { pass/not_multiline_string.expr, pass/not_record_updater.expr, pass/not_tag.expr, + pass/num_bang_amp_z_dot_t.expr, pass/number_literal_suffixes.expr, pass/old_app_header.full, pass/old_interface_header.header,