Fix ICE in relation to local declarations with erroneous initializers (#5631)

## Description

Fixes #5616 and #5622.

When generating the IR for constant and variable declarations the name
being declared is not added to the local environment if the initializer
cannot be compiled. This can happen when initializing a constant using a
non-constant expression.

Since the name isn't added to the environment, and since we attempt to
continue compilation after an error, any attempt to use the name later
in the code will cause the name to not be found, which in turn causes
the IR converter to throw an ICE because it assumes that all used names
are in scope.

This PR fixes the issue by not throwing the initializer error until the
name has been added to the environment. Note that
1. The initializer must be compiled before the name is added to the
environment. The name should not be in scope during its own
initialization.
2. #5602 is blocking a more elegant solution to this problem. The lack
of a `Never` type means that the call to `convert_resolved_typeid` may
fail if the initializer diverges, and since that call must be made
before the new name can be added to the environment we check for
termination first. The order of operations is therefore currently a)
compile the initializer, then b) check that the initializer did not
result in an error and did not diverge, then c) resolve the type of the
name being declared, then d) add the name to the environment, and
finally e) throw an error if the initializer resulted in an error. Once
the `Never` type is introduced we can move the terminator check to the
end of that sequence.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.

Author: jjcnn

sway-core/src/ir_generation/function.rs

@@ -2164,10 +2164,11 @@ impl<'eng> FnCompiler<'eng> {
 
         // We must compile the RHS before checking for shadowing, as it will still be in the
         // previous scope.
-        let init_val = self.compile_expression_to_value(context, md_mgr, body)?;
-        if init_val.is_terminator {
-            return Ok(Some(init_val));
-        }
+        // Corner case: If compilation of the expression fails, then this call returns an error.
+        // However, the declared name must be added to the local environment before the error is
+        // thrown - otherwise we will get an internal compiler error later on when the name is
+        // accessed and isn't present in the environment.
+        let init_val = self.compile_expression_to_value(context, md_mgr, body);
 
         let return_type = convert_resolved_typeid(
             self.engines.te(),
@@ -2184,6 +2185,13 @@ impl<'eng> FnCompiler<'eng> {
             .new_local_var(context, local_name.clone(), return_type, None, mutable)
             .map_err(|ir_error| CompileError::InternalOwned(ir_error.to_string(), Span::dummy()))?;
 
+        // The name has now been added, so we can check if the initializer threw an error
+        let val = init_val?;
+
+        if val.is_terminator {
+            return Ok(Some(val));
+        };
+
         // We can have empty aggregates, especially arrays, which shouldn't be initialized, but
         // otherwise use a store.
         let var_ty = local_var.get_type(context);
@@ -2195,7 +2203,7 @@ impl<'eng> FnCompiler<'eng> {
                 .add_metadatum(context, span_md_idx);
             self.current_block
                 .append(context)
-                .store(local_ptr, init_val.value)
+                .store(local_ptr, val.value)
                 .add_metadatum(context, span_md_idx);
         }
         Ok(None)
@@ -2207,7 +2215,7 @@ impl<'eng> FnCompiler<'eng> {
         md_mgr: &mut MetadataManager,
         ast_const_decl: &ty::TyConstantDecl,
         span_md_idx: Option<MetadataIndex>,
-        is_const_expression: bool,
+        is_expression: bool,
     ) -> Result<TerminatorValue, CompileError> {
         // This is local to the function, so we add it to the locals, rather than the module
         // globals like other const decls.
@@ -2219,6 +2227,11 @@ impl<'eng> FnCompiler<'eng> {
             ..
         } = ast_const_decl;
         if let Some(value) = value {
+            // Corner case: If compilation of the expression fails (e.g., because it is not
+            // constant), then this call returns an error.
+            // However, if is_expression = false then the declared name must be added to the local
+            // environment before the error is thrown - otherwise we will get an internal compiler
+            // error later on when the name is accessed and isn't present in the environment.
             let const_expr_val = compile_constant_expression(
                 self.engines,
                 context,
@@ -2229,11 +2242,13 @@ impl<'eng> FnCompiler<'eng> {
                 call_path,
                 value,
                 *is_configurable,
-            )?;
+            );
 
-            if is_const_expression {
-                Ok(TerminatorValue::new(const_expr_val, context))
+            if is_expression {
+                // No declaration. Throw any error, and return on success.
+                Ok(TerminatorValue::new(const_expr_val?, context))
             } else {
+                // Declaration. The name needs to be added to the local environment
                 let local_name = self
                     .lexical_map
                     .insert(call_path.suffix.as_str().to_owned());
@@ -2258,6 +2273,13 @@ impl<'eng> FnCompiler<'eng> {
                         CompileError::InternalOwned(ir_error.to_string(), Span::dummy())
                     })?;
 
+                // The name has now been added, so we can check if the initializer threw an error
+                let val = const_expr_val?;
+
+                if val.is_terminator(context) {
+                    return Ok(TerminatorValue::new(val, context));
+                };
+
                 // We can have empty aggregates, especially arrays, which shouldn't be initialised, but
                 // otherwise use a store.
                 let var_ty = local_var.get_type(context);
@@ -2270,11 +2292,11 @@ impl<'eng> FnCompiler<'eng> {
                     let val = self
                         .current_block
                         .append(context)
-                        .store(local_val, const_expr_val)
+                        .store(local_val, val)
                         .add_metadatum(context, span_md_idx);
                     TerminatorValue::new(val, context)
                 } else {
-                    TerminatorValue::new(const_expr_val, context)
+                    TerminatorValue::new(val, context)
                 })
             }
         } else {
@@ -2411,18 +2433,6 @@ impl<'eng> FnCompiler<'eng> {
         contents: &[ty::TyExpression],
         span_md_idx: Option<MetadataIndex>,
     ) -> Result<TerminatorValue, CompileError> {
-        // If the first element diverges, then the element type has not been determined,
-        // so we can't use the normal compilation scheme. Instead just generate code for
-        // the first element and return.
-        let first_elem_value = if !contents.is_empty() {
-            let first_elem_value = return_on_termination_or_extract!(
-                self.compile_expression_to_value(context, md_mgr, &contents[0])?
-            );
-            Some(first_elem_value)
-        } else {
-            None
-        };
-
         let elem_type = convert_resolved_typeid_no_span(
             self.engines.te(),
             self.engines.de(),
@@ -2444,14 +2454,6 @@ impl<'eng> FnCompiler<'eng> {
             .get_local(array_var)
             .add_metadatum(context, span_md_idx);
 
-        // Nothing more to do if the array is empty
-        if contents.is_empty() {
-            return Ok(TerminatorValue::new(array_value, context));
-        }
-
-        // The array is not empty, so it's safe to unwrap the first element
-        let first_elem_value = first_elem_value.unwrap();
-
         // If all elements are the same constant, then we can initialize the array
         // in a loop, reducing code size. But to check for that we've to compile
         // the expressions first, to compare. If it turns out that they're not all
@@ -2468,15 +2470,10 @@ impl<'eng> FnCompiler<'eng> {
             // We can compile all elements ahead of time without affecting register pressure.
             let compiled_elems = contents
                 .iter()
-                .enumerate()
-                .map(|(idx, e)| {
-                    if idx == 0 {
-                        // The first element has already been compiled
-                        Ok::<Value, CompileError>(first_elem_value)
-                    } else {
-                        let val = self.compile_expression_to_value(context, md_mgr, e)?;
-                        Ok(val.value)
-                    }
+                .map(|e| {
+                    Ok::<_, CompileError>(
+                        self.compile_expression_to_value(context, md_mgr, e)?.value,
+                    )
                 })
                 .collect::<Result<Vec<_>, _>>()?;
             let mut compiled_elems_iter = compiled_elems.iter();
@@ -2565,13 +2562,9 @@ impl<'eng> FnCompiler<'eng> {
 
         // Compile each element and insert it immediately.
         for (idx, elem_expr) in contents.iter().enumerate() {
-            let elem_value = if idx == 0 {
-                first_elem_value
-            } else {
-                return_on_termination_or_extract!(
-                    self.compile_expression_to_value(context, md_mgr, elem_expr)?
-                )
-            };
+            let elem_value = return_on_termination_or_extract!(
+                self.compile_expression_to_value(context, md_mgr, elem_expr)?
+            );
             let gep_val = self.current_block.append(context).get_elem_ptr_with_idx(
                 array_value,
                 elem_type,

sway-core/src/semantic_analysis/ast_node/expression/typed_expression.rs

@@ -364,15 +364,15 @@ impl ty::TyExpression {
             ExpressionKind::Break => {
                 let expr = ty::TyExpression {
                     expression: ty::TyExpressionVariant::Break,
-                    return_type: type_engine.insert(engines, TypeInfo::Unknown, None),
+                    return_type: type_engine.insert(engines, TypeInfo::Never, None),
                     span,
                 };
                 Ok(expr)
             }
             ExpressionKind::Continue => {
                 let expr = ty::TyExpression {
                     expression: ty::TyExpressionVariant::Continue,
-                    return_type: type_engine.insert(engines, TypeInfo::Unknown, None),
+                    return_type: type_engine.insert(engines, TypeInfo::Never, None),
                     span,
                 };
                 Ok(expr)
@@ -408,8 +408,7 @@ impl ty::TyExpression {
                     .unwrap_or_else(|err| ty::TyExpression::error(err, expr_span, engines));
                 let typed_expr = ty::TyExpression {
                     expression: ty::TyExpressionVariant::Return(Box::new(expr)),
-                    return_type: type_engine.insert(engines, TypeInfo::Unknown, None),
-                    // FIXME: This should be Yes?
+                    return_type: type_engine.insert(engines, TypeInfo::Never, None),
                     span,
                 };
                 Ok(typed_expr)
@@ -1758,20 +1757,20 @@ impl ty::TyExpression {
         let engines = ctx.engines();
 
         if contents.is_empty() {
-            let unknown_type = type_engine.insert(engines, TypeInfo::Unknown, None);
+            let never_type = type_engine.insert(engines, TypeInfo::Never, None);
             return Ok(ty::TyExpression {
                 expression: ty::TyExpressionVariant::Array {
-                    elem_type: unknown_type,
+                    elem_type: never_type,
                     contents: Vec::new(),
                 },
                 return_type: type_engine.insert(
                     engines,
                     TypeInfo::Array(
                         TypeArgument {
-                            type_id: unknown_type,
+                            type_id: never_type,
                             span: Span::dummy(),
                             call_path_tree: None,
-                            initial_type_id: unknown_type,
+                            initial_type_id: never_type,
                         },
                         Length::new(0, Span::dummy()),
                     ),
@@ -1781,7 +1780,8 @@ impl ty::TyExpression {
             });
         };
 
-        // start each element with the known array element type
+        // start each element with the known array element type, or Unknown if it is to be inferred
+        // from the elements
         let initial_type = match &*ctx.engines().te().get(ctx.type_annotation()) {
             TypeInfo::Array(element_type, _) => {
                 (*ctx.engines().te().get(element_type.type_id)).clone()

test/src/e2e_vm_tests/test_programs/should_fail/break_continue_in_strange_positions/Forc.lock

@@ -0,0 +1,13 @@
+[[package]]
+name = "const-instead-of-let"
+source = "member"
+dependencies = ["std"]
+
+[[package]]
+name = "core"
+source = "path+from-root-8953E158C7D5F73E"
+
+[[package]]
+name = "std"
+source = "path+from-root-8953E158C7D5F73E"
+dependencies = ["core"]

test/src/e2e_vm_tests/test_programs/should_fail/break_continue_in_strange_positions/Forc.toml

@@ -0,0 +1,8 @@
+[project]
+authors = ["Fuel Labs <[email protected]>"]
+entry = "main.sw"
+license = "Apache-2.0"
+name = "const-instead-of-let"
+
+[dependencies]
+std = { path = "../../../../../../sway-lib-std" }

test/src/e2e_vm_tests/test_programs/should_fail/break_continue_in_strange_positions/json_abi_oracle.json

@@ -0,0 +1,25 @@
+{
+  "configurables": [],
+  "functions": [
+    {
+      "attributes": null,
+      "inputs": [],
+      "name": "main",
+      "output": {
+        "name": "",
+        "type": 0,
+        "typeArguments": null
+      }
+    }
+  ],
+  "loggedTypes": [],
+  "messagesTypes": [],
+  "types": [
+    {
+      "components": null,
+      "type": "u64",
+      "typeId": 0,
+      "typeParameters": null
+    }
+  ]
+}