From df0854e0c4536359ee7d4f2e6bbca51a0b04467b Mon Sep 17 00:00:00 2001 From: jfecher Date: Mon, 22 Jul 2024 12:58:15 -0500 Subject: [PATCH] chore: Remove comptime scanning pass (#5569) # Description ## Problem\* ## Summary\* This should have been part of the removing legacy code PR but was missed. We don't need to scan for comptime code anymore since the elaborator runs the interpreter when it sees a comptime node already. ## 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. --- .../src/elaborator/expressions.rs | 8 +- compiler/noirc_frontend/src/elaborator/mod.rs | 25 +- .../noirc_frontend/src/elaborator/patterns.rs | 11 +- .../src/elaborator/statements.rs | 4 +- .../src/hir/comptime/interpreter.rs | 16 +- .../noirc_frontend/src/hir/comptime/mod.rs | 1 - .../noirc_frontend/src/hir/comptime/scan.rs | 272 ------------------ .../noirc_frontend/src/hir/comptime/tests.rs | 10 +- 8 files changed, 11 insertions(+), 336 deletions(-) delete mode 100644 compiler/noirc_frontend/src/hir/comptime/scan.rs diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index 3ea9cb43ec0..987c8c3f7ee 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -709,10 +709,8 @@ impl<'context> Elaborator<'context> { let (block, _typ) = self.elaborate_block_expression(block); self.check_and_pop_function_context(); - let mut interpreter_errors = vec![]; - let mut interpreter = self.setup_interpreter(&mut interpreter_errors); + let mut interpreter = self.setup_interpreter(); let value = interpreter.evaluate_block(block); - self.include_interpreter_errors(interpreter_errors); let (id, typ) = self.inline_comptime_value(value, span); let location = self.interner.id_location(id); @@ -796,8 +794,7 @@ impl<'context> Elaborator<'context> { }; let file = self.file; - let mut interpreter_errors = vec![]; - let mut interpreter = self.setup_interpreter(&mut interpreter_errors); + let mut interpreter = self.setup_interpreter(); let mut comptime_args = Vec::new(); let mut errors = Vec::new(); @@ -813,7 +810,6 @@ impl<'context> Elaborator<'context> { let bindings = interpreter.interner.get_instantiation_bindings(func).clone(); let result = interpreter.call_function(function, comptime_args, bindings, location); - self.include_interpreter_errors(interpreter_errors); if !errors.is_empty() { self.errors.append(&mut errors); diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 810e3d43fde..5327e911c47 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -1210,13 +1210,11 @@ impl<'context> Elaborator<'context> { self.handle_varargs_attribute(function, &mut arguments, location); arguments.insert(0, (item, location)); - let mut interpreter_errors = vec![]; - let mut interpreter = self.setup_interpreter(&mut interpreter_errors); + let mut interpreter = self.setup_interpreter(); let value = interpreter .call_function(function, arguments, TypeBindings::new(), location) .map_err(|error| error.into_compilation_error_pair())?; - self.include_interpreter_errors(interpreter_errors); if value != Value::Unit { let items = value @@ -1351,8 +1349,7 @@ impl<'context> Elaborator<'context> { let global = self.interner.get_global(global_id); let definition_id = global.definition_id; let location = global.location; - let mut interpreter_errors = vec![]; - let mut interpreter = self.setup_interpreter(&mut interpreter_errors); + let mut interpreter = self.setup_interpreter(); if let Err(error) = interpreter.evaluate_let(let_statement) { self.errors.push(error.into_compilation_error_pair()); @@ -1367,7 +1364,6 @@ impl<'context> Elaborator<'context> { self.interner.get_global_mut(global_id).value = Some(value); } - self.include_interpreter_errors(interpreter_errors); } fn define_function_metas( @@ -1461,10 +1457,6 @@ impl<'context> Elaborator<'context> { } } - fn include_interpreter_errors(&mut self, errors: Vec) { - self.errors.extend(errors.into_iter().map(InterpreterError::into_compilation_error_pair)); - } - /// True if we're currently within a `comptime` block, function, or global fn in_comptime_context(&self) -> bool { // The first context is the global context, followed by the function-specific context. @@ -1634,17 +1626,8 @@ impl<'context> Elaborator<'context> { } } - fn setup_interpreter<'a>( - &'a mut self, - interpreter_errors: &'a mut Vec, - ) -> Interpreter { - Interpreter::new( - self.interner, - &mut self.comptime_scopes, - self.crate_id, - self.debug_comptime_in_file, - interpreter_errors, - ) + fn setup_interpreter(&mut self) -> Interpreter { + Interpreter::new(self.interner, &mut self.comptime_scopes, self.crate_id) } fn debug_comptime T>( diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index d5a6e402dbf..99cdc86dc96 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -460,16 +460,9 @@ impl<'context> Elaborator<'context> { // Comptime variables must be replaced with their values if let Some(definition) = self.interner.try_definition(definition_id) { if definition.comptime && !self.in_comptime_context() { - let mut interpreter_errors = vec![]; - let mut interpreter = Interpreter::new( - self.interner, - &mut self.comptime_scopes, - self.crate_id, - self.debug_comptime_in_file, - &mut interpreter_errors, - ); + let mut interpreter = + Interpreter::new(self.interner, &mut self.comptime_scopes, self.crate_id); let value = interpreter.evaluate(id); - self.include_interpreter_errors(interpreter_errors); return self.inline_comptime_value(value, span); } } diff --git a/compiler/noirc_frontend/src/elaborator/statements.rs b/compiler/noirc_frontend/src/elaborator/statements.rs index ba3dcccca99..48380383eb0 100644 --- a/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/compiler/noirc_frontend/src/elaborator/statements.rs @@ -445,11 +445,9 @@ impl<'context> Elaborator<'context> { let span = statement.span; let (hir_statement, _typ) = self.elaborate_statement(statement); self.check_and_pop_function_context(); - let mut interpreter_errors = vec![]; - let mut interpreter = self.setup_interpreter(&mut interpreter_errors); + let mut interpreter = self.setup_interpreter(); let value = interpreter.evaluate_statement(hir_statement); let (expr, typ) = self.inline_comptime_value(value, span); - self.include_interpreter_errors(interpreter_errors); let location = self.interner.id_location(hir_statement); self.debug_comptime(location, |interner| expr.to_display_ast(interner).kind); diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 1f5903df4ad..a4440e34285 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -1,7 +1,6 @@ use std::{collections::hash_map::Entry, rc::Rc}; use acvm::{acir::AcirField, FieldElement}; -use fm::FileId; use im::Vector; use iter_extended::try_vecmap; use noirc_errors::Location; @@ -52,10 +51,6 @@ pub struct Interpreter<'interner> { crate_id: CrateId, - /// The scope of --debug-comptime, or None if unset - pub(super) debug_comptime_in_file: Option, - pub(super) debug_comptime_evaluations: &'interner mut Vec, - in_loop: bool, } @@ -65,17 +60,8 @@ impl<'a> Interpreter<'a> { interner: &'a mut NodeInterner, scopes: &'a mut Vec>, crate_id: CrateId, - debug_comptime_in_file: Option, - debug_comptime_evaluations: &'a mut Vec, ) -> Self { - Self { - interner, - scopes, - crate_id, - debug_comptime_in_file, - debug_comptime_evaluations, - in_loop: false, - } + Self { interner, scopes, crate_id, in_loop: false } } pub(crate) fn call_function( diff --git a/compiler/noirc_frontend/src/hir/comptime/mod.rs b/compiler/noirc_frontend/src/hir/comptime/mod.rs index 3cc7b5f7e98..16090c64174 100644 --- a/compiler/noirc_frontend/src/hir/comptime/mod.rs +++ b/compiler/noirc_frontend/src/hir/comptime/mod.rs @@ -1,7 +1,6 @@ mod errors; mod hir_to_display_ast; mod interpreter; -mod scan; mod tests; mod value; diff --git a/compiler/noirc_frontend/src/hir/comptime/scan.rs b/compiler/noirc_frontend/src/hir/comptime/scan.rs deleted file mode 100644 index 2ce22ab51e3..00000000000 --- a/compiler/noirc_frontend/src/hir/comptime/scan.rs +++ /dev/null @@ -1,272 +0,0 @@ -//! This module is for the scanning of the Hir by the interpreter. -//! In this initial step, the Hir is scanned for `Comptime` nodes -//! without actually executing anything until such a node is found. -//! Once such a node is found, the interpreter will call the relevant -//! evaluate method on that node type, insert the result into the Ast, -//! and continue scanning the rest of the program. -//! -//! Since it mostly just needs to recur on the Hir looking for Comptime -//! nodes, this pass is fairly simple. The only thing it really needs to -//! ensure to do is to push and pop scopes on the interpreter as needed -//! so that any variables defined within e.g. an `if` statement containing -//! a `Comptime` block aren't accessible outside of the `if`. -use crate::{ - hir_def::{ - expr::{ - HirArrayLiteral, HirBlockExpression, HirCallExpression, HirConstructorExpression, - HirIdent, HirIfExpression, HirIndexExpression, HirInfixExpression, HirLambda, - HirMethodCallExpression, - }, - stmt::HirForStatement, - }, - macros_api::{HirExpression, HirLiteral, HirStatement}, - node_interner::{DefinitionKind, ExprId, FuncId, GlobalId, StmtId}, -}; - -use super::{ - errors::{IResult, InterpreterError}, - interpreter::Interpreter, - Value, -}; - -use noirc_errors::Location; - -#[allow(dead_code)] -impl<'interner> Interpreter<'interner> { - /// Scan through a function, evaluating any Comptime nodes found. - /// These nodes will be modified in place, replaced with the - /// result of their evaluation. - pub fn scan_function(&mut self, function: FuncId) -> IResult<()> { - // Don't scan through functions that are already comptime. They may use comptime-only - // features (most likely HirExpression::Quote) that we'd otherwise error for. - if self.interner.function_modifiers(&function).is_comptime { - return Ok(()); - } - - if let Some(function) = self.interner.function(&function).try_as_expr() { - let state = self.enter_function(); - self.scan_expression(function)?; - self.exit_function(state); - } - - Ok(()) - } - - /// Evaluate this global if it is a comptime global. - /// Otherwise, scan through its expression for any comptime blocks to evaluate. - pub fn scan_global(&mut self, global: GlobalId) -> IResult<()> { - if let Some(let_) = self.interner.get_global_let_statement(global) { - if let_.comptime { - self.evaluate_let(let_)?; - } else { - self.scan_expression(let_.expression)?; - } - } - Ok(()) - } - - fn scan_expression(&mut self, expr: ExprId) -> IResult<()> { - match self.interner.expression(&expr) { - HirExpression::Ident(ident, _) => self.scan_ident(ident, expr), - HirExpression::Literal(literal) => self.scan_literal(literal), - HirExpression::Block(block) => self.scan_block(block), - HirExpression::Prefix(prefix) => self.scan_expression(prefix.rhs), - HirExpression::Infix(infix) => self.scan_infix(infix), - HirExpression::Index(index) => self.scan_index(index), - HirExpression::Constructor(constructor) => self.scan_constructor(constructor), - HirExpression::MemberAccess(member_access) => self.scan_expression(member_access.lhs), - HirExpression::Call(call) => self.scan_call(call), - HirExpression::MethodCall(method_call) => self.scan_method_call(method_call), - HirExpression::Cast(cast) => self.scan_expression(cast.lhs), - HirExpression::If(if_) => self.scan_if(if_), - HirExpression::Tuple(tuple) => self.scan_tuple(tuple), - HirExpression::Lambda(lambda) => self.scan_lambda(lambda), - HirExpression::Comptime(block) => { - let location = self.interner.expr_location(&expr); - let new_expr_id = - self.evaluate_block(block)?.into_hir_expression(self.interner, location)?; - let new_expr = self.interner.expression(&new_expr_id); - self.debug_comptime(new_expr_id, location); - self.interner.replace_expr(&expr, new_expr); - Ok(()) - } - HirExpression::Quote(_) => { - // This error could be detected much earlier in the compiler pipeline but - // it just makes sense for the comptime code to handle comptime things. - let location = self.interner.expr_location(&expr); - Err(InterpreterError::QuoteInRuntimeCode { location }) - } - HirExpression::Error => Ok(()), - - // Unquote should only be inserted by the comptime interpreter while expanding macros - // and is removed by the Hir -> Ast conversion pass which converts it into a normal block. - // If we find one now during scanning it most likely means the Hir -> Ast conversion - // missed it somehow. In the future we may allow users to manually write unquote - // expressions in their code but for now this is unreachable. - HirExpression::Unquote(block) => { - unreachable!("Found unquote block while scanning: {block:?}") - } - } - } - - // Identifiers have no code to execute but we may need to inline any values - // of comptime variables into runtime code. - fn scan_ident(&mut self, ident: HirIdent, id: ExprId) -> IResult<()> { - let definition = self.interner.definition(ident.id); - - match &definition.kind { - DefinitionKind::Function(_) => Ok(()), - _ => { - // Opportunistically evaluate this identifier to see if it is compile-time known. - // If so, inline its value. - if let Ok(value) = self.evaluate_ident(ident, id) { - // TODO(#4922): Inlining closures is currently unimplemented - if !matches!(value, Value::Closure(..)) { - let new_expr = self.inline_expression(value, id)?; - let location = self.interner.id_location(id); - self.debug_comptime(new_expr, location); - } - } - Ok(()) - } - } - } - - fn scan_literal(&mut self, literal: HirLiteral) -> IResult<()> { - match literal { - HirLiteral::Array(elements) | HirLiteral::Slice(elements) => match elements { - HirArrayLiteral::Standard(elements) => { - for element in elements { - self.scan_expression(element)?; - } - Ok(()) - } - HirArrayLiteral::Repeated { repeated_element, length: _ } => { - self.scan_expression(repeated_element) - } - }, - HirLiteral::Bool(_) - | HirLiteral::Integer(_, _) - | HirLiteral::Str(_) - | HirLiteral::FmtStr(_, _) - | HirLiteral::Unit => Ok(()), - } - } - - fn scan_block(&mut self, block: HirBlockExpression) -> IResult<()> { - self.push_scope(); - for statement in &block.statements { - self.scan_statement(*statement)?; - } - self.pop_scope(); - Ok(()) - } - - fn scan_infix(&mut self, infix: HirInfixExpression) -> IResult<()> { - self.scan_expression(infix.lhs)?; - self.scan_expression(infix.rhs) - } - - fn scan_index(&mut self, index: HirIndexExpression) -> IResult<()> { - self.scan_expression(index.collection)?; - self.scan_expression(index.index) - } - - fn scan_constructor(&mut self, constructor: HirConstructorExpression) -> IResult<()> { - for (_, field) in constructor.fields { - self.scan_expression(field)?; - } - Ok(()) - } - - fn scan_call(&mut self, call: HirCallExpression) -> IResult<()> { - self.scan_expression(call.func)?; - for arg in call.arguments { - self.scan_expression(arg)?; - } - Ok(()) - } - - fn scan_method_call(&mut self, method_call: HirMethodCallExpression) -> IResult<()> { - self.scan_expression(method_call.object)?; - for arg in method_call.arguments { - self.scan_expression(arg)?; - } - Ok(()) - } - - fn scan_if(&mut self, if_: HirIfExpression) -> IResult<()> { - self.scan_expression(if_.condition)?; - - self.push_scope(); - self.scan_expression(if_.consequence)?; - self.pop_scope(); - - if let Some(alternative) = if_.alternative { - self.push_scope(); - self.scan_expression(alternative)?; - self.pop_scope(); - } - Ok(()) - } - - fn scan_tuple(&mut self, tuple: Vec) -> IResult<()> { - for field in tuple { - self.scan_expression(field)?; - } - Ok(()) - } - - fn scan_lambda(&mut self, lambda: HirLambda) -> IResult<()> { - self.scan_expression(lambda.body) - } - - fn scan_statement(&mut self, statement: StmtId) -> IResult<()> { - match self.interner.statement(&statement) { - HirStatement::Let(let_) => self.scan_expression(let_.expression), - HirStatement::Constrain(constrain) => self.scan_expression(constrain.0), - HirStatement::Assign(assign) => self.scan_expression(assign.expression), - HirStatement::For(for_) => self.scan_for(for_), - HirStatement::Break => Ok(()), - HirStatement::Continue => Ok(()), - HirStatement::Expression(expression) => self.scan_expression(expression), - HirStatement::Semi(semi) => self.scan_expression(semi), - HirStatement::Error => Ok(()), - HirStatement::Comptime(comptime) => { - let location = self.interner.statement_location(comptime); - let new_expr = self - .evaluate_comptime(comptime)? - .into_hir_expression(self.interner, location)?; - self.debug_comptime(new_expr, location); - self.interner.replace_statement(statement, HirStatement::Expression(new_expr)); - Ok(()) - } - } - } - - fn scan_for(&mut self, for_: HirForStatement) -> IResult<()> { - // We don't need to set self.in_loop since we're not actually evaluating this loop. - // We just need to push a scope so that if there's a `comptime { .. }` expr inside this - // loop, any variables it defines aren't accessible outside of it. - self.push_scope(); - self.scan_expression(for_.block)?; - self.pop_scope(); - Ok(()) - } - - fn inline_expression(&mut self, value: Value, expr: ExprId) -> IResult { - let location = self.interner.expr_location(&expr); - let new_expr_id = value.into_hir_expression(self.interner, location)?; - let new_expr = self.interner.expression(&new_expr_id); - self.interner.replace_expr(&expr, new_expr); - Ok(new_expr_id) - } - - fn debug_comptime(&mut self, expr: ExprId, location: Location) { - if Some(location.file) == self.debug_comptime_in_file { - let expr = expr.to_display_ast(self.interner); - self.debug_comptime_evaluations - .push(InterpreterError::debug_evaluate_comptime(expr, location)); - } - } -} diff --git a/compiler/noirc_frontend/src/hir/comptime/tests.rs b/compiler/noirc_frontend/src/hir/comptime/tests.rs index 84430bdfa30..0f58a2cda95 100644 --- a/compiler/noirc_frontend/src/hir/comptime/tests.rs +++ b/compiler/noirc_frontend/src/hir/comptime/tests.rs @@ -58,15 +58,7 @@ fn elaborate_src_code(src: &str) -> (NodeInterner, FuncId) { fn interpret_helper(src: &str) -> Result { let (mut interner, main_id) = elaborate_src_code(src); let mut scopes = vec![HashMap::default()]; - let no_debug_evaluate_comptime = None; - let mut interpreter_errors = vec![]; - let mut interpreter = Interpreter::new( - &mut interner, - &mut scopes, - CrateId::Root(0), - no_debug_evaluate_comptime, - &mut interpreter_errors, - ); + let mut interpreter = Interpreter::new(&mut interner, &mut scopes, CrateId::Root(0)); let no_location = Location::dummy(); interpreter.call_function(main_id, Vec::new(), HashMap::new(), no_location)