Merge pull request #7238 from smores56/allow-try-in-statements

Allow pure statements that contain early returns

crates/compiler/can/src/expr.rs

@@ -411,6 +411,106 @@ impl Expr {
             Self::RuntimeError(..) => Category::Unknown,
         }
     }
+
+    pub fn contains_any_early_returns(&self) -> bool {
+        match self {
+            Self::Num { .. }
+            | Self::Int { .. }
+            | Self::Float { .. }
+            | Self::Str { .. }
+            | Self::IngestedFile { .. }
+            | Self::SingleQuote { .. }
+            | Self::Var { .. }
+            | Self::AbilityMember { .. }
+            | Self::ParamsVar { .. }
+            | Self::Closure(..)
+            | Self::EmptyRecord
+            | Self::RecordAccessor(_)
+            | Self::ZeroArgumentTag { .. }
+            | Self::OpaqueWrapFunction(_)
+            | Self::RuntimeError(..) => false,
+            Self::Return { .. } => true,
+            Self::List { loc_elems, .. } => loc_elems
+                .iter()
+                .any(|elem| elem.value.contains_any_early_returns()),
+            Self::When {
+                loc_cond, branches, ..
+            } => {
+                loc_cond.value.contains_any_early_returns()
+                    || branches.iter().any(|branch| {
+                        branch
+                            .guard
+                            .as_ref()
+                            .is_some_and(|guard| guard.value.contains_any_early_returns())
+                            || branch.value.value.contains_any_early_returns()
+                    })
+            }
+            Self::If {
+                branches,
+                final_else,
+                ..
+            } => {
+                final_else.value.contains_any_early_returns()
+                    || branches.iter().any(|(cond, then)| {
+                        cond.value.contains_any_early_returns()
+                            || then.value.contains_any_early_returns()
+                    })
+            }
+            Self::LetRec(defs, expr, _cycle_mark) => {
+                expr.value.contains_any_early_returns()
+                    || defs
+                        .iter()
+                        .any(|def| def.loc_expr.value.contains_any_early_returns())
+            }
+            Self::LetNonRec(def, expr) => {
+                def.loc_expr.value.contains_any_early_returns()
+                    || expr.value.contains_any_early_returns()
+            }
+            Self::Call(_func, args, _called_via) => args
+                .iter()
+                .any(|(_var, arg_expr)| arg_expr.value.contains_any_early_returns()),
+            Self::RunLowLevel { args, .. } | Self::ForeignCall { args, .. } => args
+                .iter()
+                .any(|(_var, arg_expr)| arg_expr.contains_any_early_returns()),
+            Self::Tuple { elems, .. } => elems
+                .iter()
+                .any(|(_var, loc_elem)| loc_elem.value.contains_any_early_returns()),
+            Self::Record { fields, .. } => fields
+                .iter()
+                .any(|(_field_name, field)| field.loc_expr.value.contains_any_early_returns()),
+            Self::RecordAccess { loc_expr, .. } => loc_expr.value.contains_any_early_returns(),
+            Self::TupleAccess { loc_expr, .. } => loc_expr.value.contains_any_early_returns(),
+            Self::RecordUpdate { updates, .. } => {
+                updates.iter().any(|(_field_name, field_update)| {
+                    field_update.loc_expr.value.contains_any_early_returns()
+                })
+            }
+            Self::ImportParams(_module_id, _region, params) => params
+                .as_ref()
+                .is_some_and(|(_var, p)| p.contains_any_early_returns()),
+            Self::Tag { arguments, .. } => arguments
+                .iter()
+                .any(|(_var, arg)| arg.value.contains_any_early_returns()),
+            Self::OpaqueRef { argument, .. } => argument.1.value.contains_any_early_returns(),
+            Self::Crash { msg, .. } => msg.value.contains_any_early_returns(),
+            Self::Dbg {
+                loc_message,
+                loc_continuation,
+                ..
+            } => {
+                loc_message.value.contains_any_early_returns()
+                    || loc_continuation.value.contains_any_early_returns()
+            }
+            Self::Expect {
+                loc_condition,
+                loc_continuation,
+                ..
+            } => {
+                loc_condition.value.contains_any_early_returns()
+                    || loc_continuation.value.contains_any_early_returns()
+            }
+        }
+    }
 }
 
 /// Stores exhaustiveness-checking metadata for a closure argument that may

crates/compiler/constrain/src/expr.rs

@@ -3437,12 +3437,18 @@ fn constrain_let_def(
                         )
                     });
 
-                    // Ignored def must be effectful, otherwise it's dead code
+                    let effectful_constraint = if def.loc_expr.value.contains_any_early_returns() {
-                    let effectful_constraint = Constraint::ExpectEffectful(
+                        // If the statement has early returns, it doesn't need to be effectful to
+                        // potentially affect the output of the containing function
+                        Constraint::True
+                    } else {
+                        // If there are no early returns, it must be effectful or else it's dead code
+                        Constraint::ExpectEffectful(
                             fx_var,
                             ExpectEffectfulReason::Ignored,
                             def.loc_pattern.region,
-                    );
+                        )
+                    };
 
                     let enclosing_fx_constraint = constraints.fx_call(
                         fx_var,
@@ -3529,9 +3535,14 @@ fn constrain_stmt_def(
         generalizable,
     );
 
-    // Stmt expr must be effectful, otherwise it's dead code
+    let effectful_constraint = if def.loc_expr.value.contains_any_early_returns() {
-    let effectful_constraint =
+        // If the statement has early returns, it doesn't need to be effectful to
-        Constraint::ExpectEffectful(fx_var, ExpectEffectfulReason::Stmt, region);
+        // potentially affect the output of the containing function
+        Constraint::True
+    } else {
+        // If there are no early returns, it must be effectful or else it's dead code
+        Constraint::ExpectEffectful(fx_var, ExpectEffectfulReason::Stmt, region)
+    };
 
     let fx_call_kind = match fn_name {
         None => FxCallKind::Stmt,

crates/compiler/load/tests/test_reporting.rs

@@ -14655,6 +14655,52 @@ All branches in an `if` must have the same type!
         "###
     );
 
+    test_report!(
+        try_in_bare_statement,
+        indoc!(
+            r#"
+            app [main!] { pf: platform "../../../../../crates/cli/tests/test-projects/test-platform-effects-zig/main.roc" }
+
+            import pf.Effect
+
+            validateNum = \num ->
+                if num > 5 then
+                    Ok {}
+                else
+                    Err TooBig
+
+            main! = \{} ->
+                Effect.putLine! "hello"
+
+                # this returns {}, so it's ignored
+                try validateNum 10
+
+                # this returns a value, so we are incorrectly
+                # dropping the parsed value
+                try List.get [1, 2, 3] 5
+
+                Ok {}
+            "#
+        ),
+        @r###"
+        ── IGNORED RESULT in /code/proj/Main.roc ───────────────────────────────────────
+        
+        The result of this expression is ignored:
+        
+        19│      try List.get [1, 2, 3] 5
+                 ^^^^^^^^^^^^^^^^^^^^^^^^
+        
+        Standalone statements are required to produce an empty record, but the
+        type of this one is:
+        
+            Num *
+        
+        If you still want to ignore it, assign it to `_`, like this:
+        
+            _ = File.delete! "data.json"
+        "###
+    );
+
     test_report!(
         mismatch_only_early_returns,
         indoc!(
@@ -14690,6 +14736,123 @@ All branches in an `if` must have the same type!
         "###
     );
 
+    test_report!(
+        try_with_ignored_output,
+        indoc!(
+            r#"
+            app [main!] { pf: platform "../../../../../crates/cli/tests/test-projects/test-platform-effects-zig/main.roc" }
+
+            import pf.Effect
+
+            main! = \{} ->
+                Effect.putLine! "hello"
+
+                # not ignored, warning
+                try List.get [1, 2, 3] 5
+
+                # ignored, OK
+                _ = try List.get [1, 2, 3] 5
+                _ignored = try List.get [1, 2, 3] 5
+
+                Ok {}
+            "#
+        ),
+        @r###"
+        ── IGNORED RESULT in /code/proj/Main.roc ───────────────────────────────────────
+        
+        The result of this expression is ignored:
+        
+        9│      try List.get [1, 2, 3] 5
+                ^^^^^^^^^^^^^^^^^^^^^^^^
+        
+        Standalone statements are required to produce an empty record, but the
+        type of this one is:
+        
+            Num *
+        
+        If you still want to ignore it, assign it to `_`, like this:
+        
+            _ = File.delete! "data.json"
+        "###
+    );
+
+    test_report!(
+        no_early_return_in_bare_statement,
+        indoc!(
+            r#"
+            app [main!] { pf: platform "../../../../../crates/cli/tests/test-projects/test-platform-effects-zig/main.roc" }
+
+            import pf.Effect
+
+            main! = \{} ->
+                Effect.putLine! "hello"
+
+                Num.toStr 123
+
+                Ok {}
+            "#
+        ),
+        @r#"
+    ── IGNORED RESULT in /code/proj/Main.roc ───────────────────────────────────────
+
+    The result of this call to `Num.toStr` is ignored:
+
+    8│      Num.toStr 123
+            ^^^^^^^^^
+
+    Standalone statements are required to produce an empty record, but the
+    type of this one is:
+
+        Str
+
+    If you still want to ignore it, assign it to `_`, like this:
+
+        _ = File.delete! "data.json"
+
+    ── LEFTOVER STATEMENT in /code/proj/Main.roc ───────────────────────────────────
+
+    This statement does not produce any effects:
+
+    8│      Num.toStr 123
+            ^^^^^^^^^^^^^
+
+    Standalone statements are only useful if they call effectful
+    functions.
+
+    Did you forget to use its result? If not, feel free to remove it.
+    "#
+    );
+
+    test_report!(
+        no_early_return_in_ignored_statement,
+        indoc!(
+            r#"
+            app [main!] { pf: platform "../../../../../crates/cli/tests/test-projects/test-platform-effects-zig/main.roc" }
+
+            import pf.Effect
+
+            main! = \{} ->
+                Effect.putLine! "hello"
+
+                _ignored = Num.toStr 123
+
+                Ok {}
+            "#
+        ),
+        @r"
+    ── UNNECESSARY DEFINITION in /code/proj/Main.roc ───────────────────────────────
+
+    This assignment doesn't introduce any new variables:
+
+    8│      _ignored = Num.toStr 123
+            ^^^^^^^^
+
+    Since it doesn't call any effectful functions, this assignment cannot
+    affect the program's behavior. If you don't need to use the value on
+    the right-hand side, consider removing the assignment.
+    "
+    );
+
     test_report!(
         mismatch_early_return_annotated_function,
         indoc!(