fix(experimental elaborator): Fix global values used in the elaborator (

#5135)

# Description

## Problem\*

Resolves an issue where globals would not have their type set correctly
in the elaborator. This is because `elaborate_let` was called which
creates a fresh DefinitionId rather than the one already defined by the
global, and pushes the type to that fresh id.

## Summary\*

Adds a `elaborate_global_let` function which copies over the new type to
the original DefinitionId defined by the global.
This PR also adds a call to `self.type_variables.clear()` after a global
is resolved so that its type is not defaulted too early.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

compiler/noirc_frontend/src/elaborator/expressions.rs

@@ -348,6 +348,8 @@ impl<'context> Elaborator<'context> {
                 let func_type =
                     self.type_check_variable(function_name, function_id, turbofish_generics);
 
+                self.interner.push_expr_type(function_id, func_type.clone());
+
                 // Type check the new call now that it has been changed from a method call
                 // to a function call. This way we avoid duplicating code.
                 let typ = self.type_check_call(&function_call, func_type, function_args, span);

compiler/noirc_frontend/src/elaborator/mod.rs

@@ -1212,8 +1212,6 @@ impl<'context> Elaborator<'context> {
 
         let global_id = global.global_id;
         self.current_item = Some(DependencyId::Global(global_id));
-
-        let definition_kind = DefinitionKind::Global(global_id);
         let let_stmt = global.stmt_def;
 
         if !self.in_contract
@@ -1228,11 +1226,12 @@ impl<'context> Elaborator<'context> {
             self.push_err(ResolverError::MutableGlobal { span });
         }
 
-        let (let_statement, _typ) = self.elaborate_let(let_stmt);
+        self.elaborate_global_let(let_stmt, global_id);
 
-        let statement_id = self.interner.get_global(global_id).let_statement;
-        self.interner.get_global_definition_mut(global_id).kind = definition_kind.clone();
-        self.interner.replace_statement(statement_id, let_statement);
+        // Avoid defaulting the types of globals here since they may be used in any function.
+        // Otherwise we may prematurely default to a Field inside the next function if this
+        // global was unused there, even if it is consistently used as a u8 everywhere else.
+        self.type_variables.clear();
     }
 
     fn define_function_metas(

compiler/noirc_frontend/src/elaborator/statements.rs

@@ -15,7 +15,7 @@ use crate::{
     macros_api::{
         ForLoopStatement, ForRange, HirStatement, LetStatement, Statement, StatementKind,
     },
-    node_interner::{DefinitionId, DefinitionKind, StmtId},
+    node_interner::{DefinitionId, DefinitionKind, GlobalId, StmtId},
     Type,
 };
 
@@ -24,7 +24,7 @@ use super::Elaborator;
 impl<'context> Elaborator<'context> {
     fn elaborate_statement_value(&mut self, statement: Statement) -> (HirStatement, Type) {
         match statement.kind {
-            StatementKind::Let(let_stmt) => self.elaborate_let(let_stmt),
+            StatementKind::Let(let_stmt) => self.elaborate_local_let(let_stmt),
             StatementKind::Constrain(constrain) => self.elaborate_constrain(constrain),
             StatementKind::Assign(assign) => self.elaborate_assign(assign),
             StatementKind::For(for_stmt) => self.elaborate_for(for_stmt),
@@ -51,7 +51,33 @@ impl<'context> Elaborator<'context> {
         (id, typ)
     }
 
-    pub(super) fn elaborate_let(&mut self, let_stmt: LetStatement) -> (HirStatement, Type) {
+    pub(super) fn elaborate_local_let(&mut self, let_stmt: LetStatement) -> (HirStatement, Type) {
+        let (statement, typ, _) = self.elaborate_let(let_stmt);
+        (statement, typ)
+    }
+
+    /// Elaborates a global let statement. Compared to the local version, this
+    /// version fixes up the result to use the given DefinitionId rather than
+    /// the fresh one defined by the let pattern.
+    pub(super) fn elaborate_global_let(&mut self, let_stmt: LetStatement, global_id: GlobalId) {
+        let (let_statement, _typ, new_ids) = self.elaborate_let(let_stmt);
+        let statement_id = self.interner.get_global(global_id).let_statement;
+
+        // To apply the changes from the fresh id created in elaborate_let to this global
+        // we need to change the definition kind and update the type.
+        let definition_id = self.interner.get_global(global_id).definition_id;
+        self.interner.definition_mut(definition_id).kind = DefinitionKind::Global(global_id);
+
+        assert_eq!(new_ids.len(), 1, "Globals should only define 1 value");
+        let definition_type = self.interner.definition_type(new_ids[0].id);
+        self.interner.push_definition_type(definition_id, definition_type);
+
+        self.interner.replace_statement(statement_id, let_statement);
+    }
+
+    /// Elaborate a local or global let statement. In addition to the HirLetStatement and unit
+    /// type, this also returns each HirIdent defined by this let statement.
+    fn elaborate_let(&mut self, let_stmt: LetStatement) -> (HirStatement, Type, Vec<HirIdent>) {
         let expr_span = let_stmt.expression.span;
         let (expression, expr_type) = self.elaborate_expression(let_stmt.expression);
         let definition = DefinitionKind::Local(Some(expression));
@@ -77,14 +103,18 @@ impl<'context> Elaborator<'context> {
             expr_type
         };
 
-        let let_ = HirLetStatement {
-            pattern: self.elaborate_pattern(let_stmt.pattern, r#type.clone(), definition),
-            r#type,
-            expression,
-            attributes: let_stmt.attributes,
-            comptime: let_stmt.comptime,
-        };
-        (HirStatement::Let(let_), Type::Unit)
+        let mut created_ids = Vec::new();
+        let pattern = self.elaborate_pattern_and_store_ids(
+            let_stmt.pattern,
+            r#type.clone(),
+            definition,
+            &mut created_ids,
+        );
+
+        let attributes = let_stmt.attributes;
+        let comptime = let_stmt.comptime;
+        let let_ = HirLetStatement { pattern, r#type, expression, attributes, comptime };
+        (HirStatement::Let(let_), Type::Unit, created_ids)
     }
 
     pub(super) fn elaborate_constrain(&mut self, stmt: ConstrainStatement) -> (HirStatement, Type) {

tooling/nargo_cli/tests/stdlib-tests.rs

@@ -10,7 +10,8 @@ use nargo::{
     parse_all, prepare_package,
 };
 
-fn run_stdlib_tests(use_elaborator: bool) {
+#[test]
+fn run_stdlib_tests() {
     let mut file_manager = file_manager_with_stdlib(&PathBuf::from("."));
     file_manager.add_file_with_source_canonical_path(&PathBuf::from("main.nr"), "".to_owned());
     let parsed_files = parse_all(&file_manager);
@@ -29,7 +30,7 @@ fn run_stdlib_tests(use_elaborator: bool) {
     let (mut context, dummy_crate_id) =
         prepare_package(&file_manager, &parsed_files, &dummy_package);
 
-    let result = check_crate(&mut context, dummy_crate_id, true, false, use_elaborator);
+    let result = check_crate(&mut context, dummy_crate_id, true, false, false);
     report_errors(result, &context.file_manager, true, false)
         .expect("Error encountered while compiling standard library");
 
@@ -59,15 +60,3 @@ fn run_stdlib_tests(use_elaborator: bool) {
     assert!(!test_report.is_empty(), "Could not find any tests within the stdlib");
     assert!(test_report.iter().all(|(_, status)| !status.failed()));
 }
-
-#[test]
-fn stdlib_noir_tests() {
-    run_stdlib_tests(false)
-}
-
-// Once this no longer panics we can use the elaborator by default and remove the old passes
-#[test]
-#[should_panic]
-fn stdlib_elaborator_tests() {
-    run_stdlib_tests(true)
-}