Merge pull request #6458 from roc-lang/list-get-repl

List.get with negative index in repl

crates/compiler/gen_llvm/src/run_roc.rs

@@ -128,7 +128,7 @@ macro_rules! run_jit_function {
             Err((error_msg, _)) => {
                 eprintln!("This Roc code crashed with: \"{error_msg}\"");
 
-                Expr::MalformedClosure
+                Expr::REPL_RUNTIME_CRASH
             }
         }
     }};
@@ -165,10 +165,10 @@ macro_rules! run_jit_function_dynamic_type {
             let result = Result::from(call_result);
 
             match result {
-                Ok(()) => $transform(output.add(CALL_RESULT_WIDTH) as usize),
-                Err((msg, _crash_tag)) => {
-                    eprintln!("{}", msg);
-                    panic!("Roc hit an error");
+                Ok(()) => Some($transform(output.add(CALL_RESULT_WIDTH) as usize)),
+                Err((error_msg, _)) => {
+                    eprintln!("This Roc code crashed with: \"{error_msg}\"");
+                    None
                 }
             }
         }

crates/compiler/parse/src/ast.rs

@@ -1296,6 +1296,16 @@ impl<'a> Spaceable<'a> for ImplementsAbilities<'a> {
 }
 
 impl<'a> Expr<'a> {
+    pub const REPL_OPAQUE_FUNCTION: Self = Expr::Var {
+        module_name: "",
+        ident: "<function>",
+    };
+
+    pub const REPL_RUNTIME_CRASH: Self = Expr::Var {
+        module_name: "",
+        ident: "*",
+    };
+
     pub fn loc_ref(&'a self, region: Region) -> Loc<&'a Self> {
         Loc {
             region,

crates/repl_cli/src/cli_gen.rs

@@ -117,14 +117,13 @@ impl<'a> ReplApp<'a> for CliApp {
         main_fn_name: &str,
         ret_bytes: usize,
         mut transform: F,
-    ) -> T
+    ) -> Option<T>
     where
         F: FnMut(&'a Self::Memory, usize) -> T,
         Self::Memory: 'a,
     {
-        run_jit_function_dynamic_type!(self.lib, main_fn_name, ret_bytes, |v| transform(
-            &CliMemory, v
-        ))
+        let mut t = |v| transform(&CliMemory, v);
+        run_jit_function_dynamic_type!(self.lib, main_fn_name, ret_bytes, t)
     }
 }
 

crates/repl_eval/src/eval.rs

@@ -265,8 +265,6 @@ fn get_tags_vars_and_variant<'a>(
     (vars_of_tag, union_variant)
 }
 
-const FAKE_EXPR: &Loc<Expr> = &Loc::at_zero(Expr::Crash);
-
 fn expr_of_tag<'a, M: ReplAppMemory>(
     env: &mut Env<'a, '_>,
     mem: &'a M,
@@ -296,6 +294,7 @@ fn expr_of_tag<'a, M: ReplAppMemory>(
         cmp_fields(&env.layout_cache.interner, i1, *lay1, i2, *lay2)
     });
 
+    const FAKE_EXPR: &Loc<Expr> = &Loc::at_zero(Expr::Crash);
     let mut output: Vec<&Loc<Expr>> =
         Vec::from_iter_in(std::iter::repeat(FAKE_EXPR).take(layouts.len()), env.arena);
     let mut field_addr = data_addr;
@@ -359,11 +358,6 @@ fn tag_id_from_recursive_ptr<'a, M: ReplAppMemory>(
     }
 }
 
-const OPAQUE_FUNCTION: Expr = Expr::Var {
-    module_name: "",
-    ident: "<function>",
-};
-
 fn jit_to_ast_help<'a, A: ReplApp<'a>>(
     env: &mut Env<'a, '_>,
     app: &mut A,
@@ -430,7 +424,10 @@ fn jit_to_ast_help<'a, A: ReplApp<'a>>(
                 Expr::Str(StrLiteral::PlainLine(arena_str))
             };
 
-            app.call_function_returns_roc_str(env.target_info, main_fn_name, body)
+            match app.call_function_returns_roc_str(env.target_info, main_fn_name, body) {
+                Some(string) => string,
+                None => Expr::REPL_RUNTIME_CRASH,
+            }
         }
         LayoutRepr::Builtin(Builtin::List(elem_layout)) => app.call_function_returns_roc_list(
             main_fn_name,
@@ -476,7 +473,7 @@ fn jit_to_ast_help<'a, A: ReplApp<'a>>(
                 }
                 Content::Structure(FlatType::Func(_, _, _)) => {
                     // a function with a struct as the closure environment
-                    OPAQUE_FUNCTION
+                    Expr::REPL_OPAQUE_FUNCTION
                 }
                 other => {
                     unreachable!(
@@ -486,16 +483,21 @@ fn jit_to_ast_help<'a, A: ReplApp<'a>>(
                 }
             };
 
-            app.call_function_dynamic_size(
+            let opt_struct = app.call_function_dynamic_size(
                 main_fn_name,
                 result_stack_size as usize,
                 struct_addr_to_ast,
-            )
+            );
+
+            match opt_struct {
+                Some(struct_) => struct_,
+                None => Expr::REPL_RUNTIME_CRASH,
+            }
         }
         LayoutRepr::Union(UnionLayout::NonRecursive(_)) => {
             let size = env.layout_cache.interner.stack_size(layout);
 
-            app.call_function_dynamic_size(
+            let opt_union = app.call_function_dynamic_size(
                 main_fn_name,
                 size as usize,
                 |mem: &'a A::Memory, addr: usize| {
@@ -508,15 +510,20 @@ fn jit_to_ast_help<'a, A: ReplApp<'a>>(
                         env.subs.get_root_key_without_compacting(raw_var),
                     )
                 },
-            )
+            );
+
+            match opt_union {
+                Some(union_) => union_,
+                None => Expr::REPL_RUNTIME_CRASH,
+            }
         }
         LayoutRepr::Union(UnionLayout::Recursive(_))
         | LayoutRepr::Union(UnionLayout::NonNullableUnwrapped(_))
         | LayoutRepr::Union(UnionLayout::NullableUnwrapped { .. })
         | LayoutRepr::Union(UnionLayout::NullableWrapped { .. }) => {
             let size = env.layout_cache.interner.stack_size(layout);
 
-            app.call_function_dynamic_size(
+            let opt_union = app.call_function_dynamic_size(
                 main_fn_name,
                 size as usize,
                 |mem: &'a A::Memory, addr: usize| {
@@ -529,7 +536,12 @@ fn jit_to_ast_help<'a, A: ReplApp<'a>>(
                         env.subs.get_root_key_without_compacting(raw_var),
                     )
                 },
-            )
+            );
+
+            match opt_union {
+                Some(union_) => union_,
+                None => Expr::REPL_RUNTIME_CRASH,
+            }
         }
         LayoutRepr::RecursivePointer(_) => {
             unreachable!("RecursivePointers can only be inside structures")
@@ -538,7 +550,7 @@ fn jit_to_ast_help<'a, A: ReplApp<'a>>(
             unreachable!("Ptr will never be visible to users")
         }
         LayoutRepr::LambdaSet(_) | LayoutRepr::FunctionPointer(_) | LayoutRepr::Erased(_) => {
-            OPAQUE_FUNCTION
+            Expr::REPL_OPAQUE_FUNCTION
         }
     };
 
@@ -578,7 +590,7 @@ fn addr_to_ast<'a, M: ReplAppMemory>(
 
     let expr = match (raw_content, layout) {
         (Content::Structure(FlatType::Func(_, _, _)), _) | (_, LayoutRepr::LambdaSet(_) | LayoutRepr::FunctionPointer(_) | LayoutRepr::Erased(_)) => {
-            OPAQUE_FUNCTION
+            Expr::REPL_OPAQUE_FUNCTION
         }
         (_, LayoutRepr::Builtin(Builtin::Bool)) => {
             // TODO: bits are not as expected here.

crates/repl_eval/src/lib.rs

@@ -25,12 +25,13 @@ pub trait ReplApp<'a> {
         self.call_function(main_fn_name, transform)
     }
 
+    /// When the executed code calls roc_panic, this function will return None
     fn call_function_returns_roc_str<T, F>(
         &mut self,
         target_info: TargetInfo,
         main_fn_name: &str,
         transform: F,
-    ) -> T
+    ) -> Option<T>
     where
         F: Fn(&'a Self::Memory, usize) -> T,
         Self::Memory: 'a,
@@ -45,12 +46,14 @@ pub trait ReplApp<'a> {
 
     /// Run user code that returns a struct or union, whose size is provided as an argument
     /// The `transform` callback takes the app's memory and the address of the returned value
+    ///
+    /// When the executed code calls roc_panic, this function will return None
     fn call_function_dynamic_size<T, F>(
         &mut self,
         main_fn_name: &str,
         ret_bytes: usize,
         transform: F,
-    ) -> T
+    ) -> Option<T>
     where
         F: FnMut(&'a Self::Memory, usize) -> T,
         Self::Memory: 'a;

crates/repl_expect/src/app.rs

@@ -121,7 +121,7 @@ impl<'a> ReplApp<'a> for ExpectReplApp<'a> {
         _target_info: TargetInfo,
         main_fn_name: &str,
         transform: F,
-    ) -> T
+    ) -> Option<T>
     where
         F: Fn(&'a Self::Memory, usize) -> T,
         Self::Memory: 'a,
@@ -138,11 +138,11 @@ impl<'a> ReplApp<'a> for ExpectReplApp<'a> {
         _main_fn_name: &str,
         _ret_bytes: usize,
         mut transform: F,
-    ) -> T
+    ) -> Option<T>
     where
         F: FnMut(&'a Self::Memory, usize) -> T,
         Self::Memory: 'a,
     {
-        transform(self.memory, self.offset)
+        Some(transform(self.memory, self.offset))
     }
 }

crates/repl_test/src/tests.rs

@@ -755,6 +755,58 @@ fn type_problem_string_interpolation() {
     );
 }
 
+#[cfg(not(feature = "wasm"))] // TODO: mismatch is due to terminal control codes!
+#[test]
+fn list_drop_at_negative_index() {
+    expect_failure(
+        "List.dropAt [1, 2, 3] -1",
+        indoc!(
+            r#"
+            ── TYPE MISMATCH ───────────────────────────────────────────────────────────────
+
+            This 2nd argument to dropAt has an unexpected type:
+
+            4│      List.dropAt [1, 2, 3] -1
+                                          ^^
+
+            The argument is a number of type:
+
+                I8, I16, F32, I32, F64, I64, I128, or Dec
+
+            But dropAt needs its 2nd argument to be:
+
+                Nat
+            "#
+        ),
+    );
+}
+
+#[cfg(not(feature = "wasm"))] // TODO: mismatch is due to terminal control codes!
+#[test]
+fn list_get_negative_index() {
+    expect_failure(
+        "List.get [1, 2, 3] -1",
+        indoc!(
+            r#"
+            ── TYPE MISMATCH ───────────────────────────────────────────────────────────────
+
+            This 2nd argument to get has an unexpected type:
+
+            4│      List.get [1, 2, 3] -1
+                                       ^^
+
+            The argument is a number of type:
+
+                I8, I16, F32, I32, F64, I64, I128, or Dec
+
+            But get needs its 2nd argument to be:
+
+                Nat
+            "#
+        ),
+    );
+}
+
 #[test]
 fn issue_2149_i8_ok() {
     expect_success(r#"Str.toI8 "127""#, "Ok 127 : Result I8 [InvalidNumStr]");

crates/repl_wasm/src/repl.rs

@@ -160,7 +160,7 @@ impl<'a> ReplApp<'a> for WasmReplApp<'a> {
         _main_fn_name: &str,
         _ret_bytes: usize,
         mut transform: F,
-    ) -> T
+    ) -> Option<T>
     where
         F: FnMut(&'a Self::Memory, usize) -> T,
         Self::Memory: 'a,
@@ -172,7 +172,7 @@ impl<'a> ReplApp<'a> for WasmReplApp<'a> {
         let app_result_addr = js_get_result_and_memory(copied_bytes.as_mut_ptr());
         let mem = self.arena.alloc(WasmMemory { copied_bytes });
 
-        transform(mem, app_result_addr)
+        Some(transform(mem, app_result_addr))
     }
 }