From 5670fe06cd494aa027e0d59d1086249fa63b4526 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Sun, 13 Mar 2022 17:40:16 -0500 Subject: [PATCH 1/8] Deal with destructuring tag unions behind opaques correctly Closes #2702 --- compiler/constrain/src/pattern.rs | 22 +++++++++---- compiler/solve/tests/solve_expr.rs | 53 ++++++++++++++++++++++++++++++ compiler/unify/src/unify.rs | 12 +++++-- 3 files changed, 79 insertions(+), 8 deletions(-) diff --git a/compiler/constrain/src/pattern.rs b/compiler/constrain/src/pattern.rs index 0a543728bd..def61bc01e 100644 --- a/compiler/constrain/src/pattern.rs +++ b/compiler/constrain/src/pattern.rs @@ -504,12 +504,22 @@ pub fn constrain_pattern( ); // Link the entire wrapped opaque type (with the now-constrained argument) to the type - // variables of the opaque type - // TODO: better expectation here - let link_type_variables_con = constraints.equal_types( - (**specialized_def_type).clone(), - Expected::NoExpectation(arg_pattern_type), - Category::OpaqueWrap(*opaque), + // variables of the opaque type. + // + // For example, suppose we have `O k := [ A k, B k ]`, and the pattern `@O (A s) -> s == ""`. + // Previous constraints will have solved `typeof s ~ Str`, and we have the + // `specialized_def_type` being `[ A k1, B k1 ]`, specializing `k` as `k1` for this opaque + // usage. + // We now want to link `typeof s ~ k1`, so to capture this relationship, we link + // the type of `A s` (the arg type) to `[ A k1, B k1 ]` (the specialized opaque type). + // + // This must **always** be a presence constraint, that is enforcing + // `[ A k1, B k1 ] += typeof (A s)`, because we are in a destructure position and not + // all constructors are covered in this branch! + let link_type_variables_con = constraints.pattern_presence( + arg_pattern_type, + PExpected::NoExpectation((**specialized_def_type).clone()), + PatternCategory::Opaque(*opaque), loc_arg_pattern.region, ); diff --git a/compiler/solve/tests/solve_expr.rs b/compiler/solve/tests/solve_expr.rs index 5f3578faab..bdd91011b9 100644 --- a/compiler/solve/tests/solve_expr.rs +++ b/compiler/solve/tests/solve_expr.rs @@ -5573,4 +5573,57 @@ mod solve_expr { r#"[ A, B, C ]"#, ) } + + #[test] + // https://github.com/rtfeldman/roc/issues/2702 + fn tag_inclusion_behind_opaque() { + infer_eq_without_problem( + indoc!( + r#" + Outer k := [ Empty, Wrapped k ] + + insert : Outer k, k -> Outer k + insert = \m, var -> + when m is + $Outer Empty -> $Outer (Wrapped var) + $Outer (Wrapped _) -> $Outer (Wrapped var) + + insert + "# + ), + r#"Outer k, k -> Outer k"#, + ) + } + + #[test] + fn tag_inclusion_behind_opaque_infer() { + infer_eq_without_problem( + indoc!( + r#" + Outer k := [ Empty, Wrapped k ] + + when ($Outer Empty) is + $Outer Empty -> $Outer (Wrapped "") + $Outer (Wrapped k) -> $Outer (Wrapped k) + "# + ), + r#"Outer Str"#, + ) + } + + #[test] + fn tag_inclusion_behind_opaque_infer_single_ctor() { + infer_eq_without_problem( + indoc!( + r#" + Outer := [ A, B ] + + when ($Outer A) is + $Outer A -> $Outer A + $Outer B -> $Outer B + "# + ), + r#"Outer"#, + ) + } } diff --git a/compiler/unify/src/unify.rs b/compiler/unify/src/unify.rs index 88d0e0da98..133429b94c 100644 --- a/compiler/unify/src/unify.rs +++ b/compiler/unify/src/unify.rs @@ -180,10 +180,12 @@ fn unify_context(subs: &mut Subs, pool: &mut Pool, ctx: Context) -> Outcome { // println!("\n --------------- \n"); let content_1 = subs.get(ctx.first).content; let content_2 = subs.get(ctx.second).content; + let mode = if ctx.mode.is_eq() { "~" } else { "+=" }; println!( - "{:?} {:?} ~ {:?} {:?}", + "{:?} {:?} {} {:?} {:?}", ctx.first, roc_types::subs::SubsFmtContent(&content_1, subs), + mode, ctx.second, roc_types::subs::SubsFmtContent(&content_2, subs), ); @@ -334,7 +336,13 @@ fn unify_alias( problems.extend(merge(subs, ctx, *other_content)); } - // if problems.is_empty() { problems.extend(unify_pool(subs, pool, real_var, *other_real_var)); } + // THEORY: if two aliases or opaques have the same name and arguments, their + // real_var is the same and we don't need to check it. + // See https://github.com/rtfeldman/roc/pull/1510 + // + // if problems.is_empty() && either_is_opaque { + // problems.extend(unify_pool(subs, pool, real_var, *other_real_var, ctx.mode)); + // } problems } else { From 530fa9943ab4628cfb580cb4d2eccd63c8d51717 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Sun, 13 Mar 2022 17:55:52 -0500 Subject: [PATCH 2/8] Generate unsigned div and modulo correctly Closes #2705 --- compiler/gen_llvm/src/llvm/build.rs | 16 ++++++++++++++-- compiler/test_gen/src/gen_num.rs | 28 ++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index 56e46bbc29..11270a506d 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -6558,7 +6558,13 @@ fn build_int_binop<'a, 'ctx, 'env>( NumGte => bd.build_int_compare(SGE, lhs, rhs, "int_gte").into(), NumLt => bd.build_int_compare(SLT, lhs, rhs, "int_lt").into(), NumLte => bd.build_int_compare(SLE, lhs, rhs, "int_lte").into(), - NumRemUnchecked => bd.build_int_signed_rem(lhs, rhs, "rem_int").into(), + NumRemUnchecked => { + if int_width.is_signed() { + bd.build_int_signed_rem(lhs, rhs, "rem_int").into() + } else { + bd.build_int_unsigned_rem(lhs, rhs, "rem_uint").into() + } + } NumIsMultipleOf => { // this builds the following construct // @@ -6632,7 +6638,13 @@ fn build_int_binop<'a, 'ctx, 'env>( &[lhs.into(), rhs.into()], &bitcode::NUM_POW_INT[int_width], ), - NumDivUnchecked => bd.build_int_signed_div(lhs, rhs, "div_int").into(), + NumDivUnchecked => { + if int_width.is_signed() { + bd.build_int_signed_div(lhs, rhs, "div_int").into() + } else { + bd.build_int_unsigned_div(lhs, rhs, "div_uint").into() + } + } NumDivCeilUnchecked => call_bitcode_fn( env, &[lhs.into(), rhs.into()], diff --git a/compiler/test_gen/src/gen_num.rs b/compiler/test_gen/src/gen_num.rs index 9c1312fd18..008d152b76 100644 --- a/compiler/test_gen/src/gen_num.rs +++ b/compiler/test_gen/src/gen_num.rs @@ -2831,3 +2831,31 @@ fn upcast_of_int_checked_is_zext() { u16 ) } + +#[test] +#[cfg(any(feature = "gen-llvm"))] +fn modulo_of_unsigned() { + assert_evals_to!( + indoc!( + r#" + 0b1111_1111u8 % 64 + "# + ), + 63, + u8 + ) +} + +#[test] +#[cfg(any(feature = "gen-llvm"))] +fn div_of_unsigned() { + assert_evals_to!( + indoc!( + r#" + 0b1111_1111u8 // 2 + "# + ), + 127, + u8 + ) +} From fe1cbc22613c31baba04a205196880b4ff076cb7 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Sun, 13 Mar 2022 18:11:57 -0500 Subject: [PATCH 3/8] Print i8s correctly in repl Closes #2710 --- repl_eval/src/eval.rs | 33 ++++++++++++++++++--------------- repl_test/src/tests.rs | 14 ++++++++++++++ 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/repl_eval/src/eval.rs b/repl_eval/src/eval.rs index a57624e92e..a37ae12c9f 100644 --- a/repl_eval/src/eval.rs +++ b/repl_eval/src/eval.rs @@ -281,7 +281,7 @@ fn jit_to_ast_help<'a, A: ReplApp<'a>>( let (newtype_containers, content) = unroll_newtypes(env, content); let content = unroll_aliases(env, content); - macro_rules! helper { + macro_rules! num_helper { ($ty:ty) => { app.call_function(main_fn_name, |_, num: $ty| { num_to_ast(env, number_literal_to_ast(env.arena, num), content) @@ -297,21 +297,24 @@ fn jit_to_ast_help<'a, A: ReplApp<'a>>( Layout::Builtin(Builtin::Int(int_width)) => { use IntWidth::*; - let result = match int_width { - U8 | I8 => { - // NOTE: `helper!` does not handle 8-bit numbers yet + let result = match (content, int_width) { + (Content::Structure(FlatType::Apply(Symbol::NUM_NUM, _)), U8) => num_helper!(u8), + (_, U8) => { + // This is not a number, it's a tag union or something else app.call_function(main_fn_name, |mem: &A::Memory, num: u8| { byte_to_ast(env, mem, num, content) }) } - U16 => helper!(u16), - U32 => helper!(u32), - U64 => helper!(u64), - U128 => helper!(u128), - I16 => helper!(i16), - I32 => helper!(i32), - I64 => helper!(i64), - I128 => helper!(i128), + // The rest are numbers... for now + (_, U16) => num_helper!(u16), + (_, U32) => num_helper!(u32), + (_, U64) => num_helper!(u64), + (_, U128) => num_helper!(u128), + (_, I8) => num_helper!(i8), + (_, I16) => num_helper!(i16), + (_, I32) => num_helper!(i32), + (_, I64) => num_helper!(i64), + (_, I128) => num_helper!(i128), }; Ok(result) @@ -320,14 +323,14 @@ fn jit_to_ast_help<'a, A: ReplApp<'a>>( use FloatWidth::*; let result = match float_width { - F32 => helper!(f32), - F64 => helper!(f64), + F32 => num_helper!(f32), + F64 => num_helper!(f64), F128 => todo!("F128 not implemented"), }; Ok(result) } - Layout::Builtin(Builtin::Decimal) => Ok(helper!(RocDec)), + Layout::Builtin(Builtin::Decimal) => Ok(num_helper!(RocDec)), Layout::Builtin(Builtin::Str) => { let size = layout.stack_size(env.target_info) as usize; Ok( diff --git a/repl_test/src/tests.rs b/repl_test/src/tests.rs index 059168b610..bfb6560af6 100644 --- a/repl_test/src/tests.rs +++ b/repl_test/src/tests.rs @@ -1051,3 +1051,17 @@ fn dec_in_repl() { r#"1.23 : Dec"#, ) } + +#[test] +fn print_i8_issue_2710() { + expect_success( + indoc!( + r#" + a : I8 + a = -1 + a + "# + ), + r#"-1 : I8"#, + ) +} From ec4bb9ad8343d7a17af6bc1cfa8d4e5dad6e0bcd Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Sun, 13 Mar 2022 18:43:46 -0500 Subject: [PATCH 4/8] Support boxes in repl Closes #2712 --- repl_eval/src/eval.rs | 30 +++++++++++++++++++++++++++++- repl_test/src/tests.rs | 12 ++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/repl_eval/src/eval.rs b/repl_eval/src/eval.rs index a37ae12c9f..853f0a54a9 100644 --- a/repl_eval/src/eval.rs +++ b/repl_eval/src/eval.rs @@ -441,7 +441,16 @@ fn jit_to_ast_help<'a, A: ReplApp<'a>>( unreachable!("RecursivePointers can only be inside structures") } Layout::LambdaSet(_) => Ok(OPAQUE_FUNCTION), - Layout::Boxed(_inner) => todo!(), + Layout::Boxed(_) => { + let size = layout.stack_size(env.target_info); + Ok(app.call_function_dynamic_size( + main_fn_name, + size as usize, + |mem: &A::Memory, addr| { + addr_to_ast(env, mem, addr, layout, WhenRecursive::Unreachable, content) + }, + )) + } }; result.map(|e| apply_newtypes(env, newtype_containers, e)) } @@ -733,6 +742,25 @@ fn addr_to_ast<'a, M: ReplAppMemory>( ) } } + (Content::Structure(FlatType::Apply(Symbol::BOX_BOX_TYPE, args)), Layout::Boxed(inner_layout)) => { + debug_assert_eq!(args.len(), 1); + + let inner_var_index = args.into_iter().next().unwrap(); + let inner_var = env.subs[inner_var_index]; + let inner_content = env.subs.get_content_without_compacting(inner_var); + + let addr_of_inner = mem.deref_usize(addr); + let inner_expr = addr_to_ast(env, mem, addr_of_inner, &inner_layout, WhenRecursive::Unreachable, inner_content); + + let box_box = env.arena.alloc(Loc::at_zero(Expr::Var { + module_name: "Box", ident: "box" + })); + let box_box_arg = &*env.arena.alloc(Loc::at_zero(inner_expr)); + let box_box_args = env.arena.alloc([box_box_arg]); + + Expr::Apply(box_box, box_box_args, CalledVia::Space) + } + (_, Layout::Boxed(_)) => unreachable!("Box layouts can only be behind a `Box.Box` application"), other => { todo!( "TODO add support for rendering pointer to {:?} in the REPL", diff --git a/repl_test/src/tests.rs b/repl_test/src/tests.rs index bfb6560af6..22677336ad 100644 --- a/repl_test/src/tests.rs +++ b/repl_test/src/tests.rs @@ -1065,3 +1065,15 @@ fn print_i8_issue_2710() { r#"-1 : I8"#, ) } + +#[test] +fn box_box() { + expect_success( + indoc!( + r#" + Box.box "container store" + "# + ), + r#"Box.box "container store" : Box Str"#, + ) +} From e3d3f5195191250423df40ad838329ee86e719ca Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Sun, 13 Mar 2022 18:48:56 -0500 Subject: [PATCH 5/8] Demonstrate Box alias imported correctly Closes #2712 --- repl_test/src/tests.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/repl_test/src/tests.rs b/repl_test/src/tests.rs index 22677336ad..c64981a6a3 100644 --- a/repl_test/src/tests.rs +++ b/repl_test/src/tests.rs @@ -1077,3 +1077,18 @@ fn box_box() { r#"Box.box "container store" : Box Str"#, ) } + +#[test] +fn box_box_type_alias() { + expect_success( + indoc!( + r#" + HeapStr : Box Str + helloHeap : HeapStr + helloHeap = Box.box "bye stacks" + helloHeap + "# + ), + r#"Box.box "bye stacks" : HeapStr"#, + ) +} From 4c06b9d1a43559d7c0d45d775101bed4681fed65 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Sun, 13 Mar 2022 19:06:37 -0500 Subject: [PATCH 6/8] Redundant reference --- repl_eval/src/eval.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/repl_eval/src/eval.rs b/repl_eval/src/eval.rs index 853f0a54a9..bc718a7c6c 100644 --- a/repl_eval/src/eval.rs +++ b/repl_eval/src/eval.rs @@ -750,7 +750,7 @@ fn addr_to_ast<'a, M: ReplAppMemory>( let inner_content = env.subs.get_content_without_compacting(inner_var); let addr_of_inner = mem.deref_usize(addr); - let inner_expr = addr_to_ast(env, mem, addr_of_inner, &inner_layout, WhenRecursive::Unreachable, inner_content); + let inner_expr = addr_to_ast(env, mem, addr_of_inner, inner_layout, WhenRecursive::Unreachable, inner_content); let box_box = env.arena.alloc(Loc::at_zero(Expr::Var { module_name: "Box", ident: "box" From 01b51096726266d8dae422896b609959cd8abc56 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Sun, 13 Mar 2022 19:47:01 -0500 Subject: [PATCH 7/8] Fix reporting test --- reporting/tests/test_reporting.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/reporting/tests/test_reporting.rs b/reporting/tests/test_reporting.rs index d4feda347b..f896a9fa11 100644 --- a/reporting/tests/test_reporting.rs +++ b/reporting/tests/test_reporting.rs @@ -8369,7 +8369,7 @@ I need all branches in an `if` to have the same type! But all the previous branches match: - F [ A ] + F [ A ]a "# ), ) From ee8b2a52839ddeb0ce4d630d067eeb7e19f1aba6 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Sun, 13 Mar 2022 20:16:04 -0500 Subject: [PATCH 8/8] Disable repl box tests on wasm for now --- repl_test/src/tests.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/repl_test/src/tests.rs b/repl_test/src/tests.rs index c64981a6a3..5abff88496 100644 --- a/repl_test/src/tests.rs +++ b/repl_test/src/tests.rs @@ -1066,6 +1066,7 @@ fn print_i8_issue_2710() { ) } +#[cfg(not(feature = "wasm"))] #[test] fn box_box() { expect_success( @@ -1078,6 +1079,7 @@ fn box_box() { ) } +#[cfg(not(feature = "wasm"))] #[test] fn box_box_type_alias() { expect_success(