From fd3377b3ae279fafc6d72a62c7661c542c713117 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 21 Jan 2025 16:00:21 -0300 Subject: [PATCH 01/11] feat: parse globals in SSA parser (#7112) Co-authored-by: Maxim Vezenov --- .../src/ssa/opt/normalize_value_ids.rs | 6 +- .../noirc_evaluator/src/ssa/parser/ast.rs | 27 ++- .../src/ssa/parser/into_ssa.rs | 113 +++++++++++- .../noirc_evaluator/src/ssa/parser/mod.rs | 172 +++++++++++++----- .../noirc_evaluator/src/ssa/parser/tests.rs | 16 ++ 5 files changed, 271 insertions(+), 63 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs b/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs index b248f6734a9..ed57118b840 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs @@ -25,7 +25,7 @@ impl Ssa { let mut context = Context::default(); context.populate_functions(&self.functions); for function in self.functions.values_mut() { - context.normalize_ids(function); + context.normalize_ids(function, &self.globals); } self.functions = context.functions.into_btree(); } @@ -65,14 +65,14 @@ impl Context { } } - fn normalize_ids(&mut self, old_function: &mut Function) { + fn normalize_ids(&mut self, old_function: &mut Function, globals: &Function) { self.new_ids.blocks.clear(); self.new_ids.values.clear(); let new_function_id = self.new_ids.function_ids[&old_function.id()]; let new_function = &mut self.functions[new_function_id]; - for (_, value) in old_function.dfg.globals.values_iter() { + for (_, value) in globals.dfg.values_iter() { new_function.dfg.make_global(value.get_type().into_owned()); } diff --git a/compiler/noirc_evaluator/src/ssa/parser/ast.rs b/compiler/noirc_evaluator/src/ssa/parser/ast.rs index 6c7608a2f16..05743ffd7ca 100644 --- a/compiler/noirc_evaluator/src/ssa/parser/ast.rs +++ b/compiler/noirc_evaluator/src/ssa/parser/ast.rs @@ -7,9 +7,28 @@ use crate::ssa::ir::{function::RuntimeType, instruction::BinaryOp, types::Type}; #[derive(Debug)] pub(crate) struct ParsedSsa { + pub(crate) globals: Vec, pub(crate) functions: Vec, } +#[derive(Debug)] +pub(crate) struct ParsedGlobal { + pub(crate) name: Identifier, + pub(crate) value: ParsedGlobalValue, +} + +#[derive(Debug)] +pub(crate) enum ParsedGlobalValue { + NumericConstant(ParsedNumericConstant), + MakeArray(ParsedMakeArray), +} + +#[derive(Debug)] +pub(crate) struct ParsedMakeArray { + pub(crate) elements: Vec, + pub(crate) typ: Type, +} + #[derive(Debug)] pub(crate) struct ParsedFunction { pub(crate) runtime_type: RuntimeType, @@ -145,6 +164,12 @@ pub(crate) enum ParsedTerminator { #[derive(Debug, Clone)] pub(crate) enum ParsedValue { - NumericConstant { constant: FieldElement, typ: Type }, + NumericConstant(ParsedNumericConstant), Variable(Identifier), } + +#[derive(Debug, Clone)] +pub(crate) struct ParsedNumericConstant { + pub(crate) value: FieldElement, + pub(crate) typ: Type, +} diff --git a/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs b/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs index e2eea234dc7..456f2f85af1 100644 --- a/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs +++ b/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::{collections::HashMap, sync::Arc}; use acvm::acir::circuit::ErrorSelector; @@ -6,15 +6,17 @@ use crate::ssa::{ function_builder::FunctionBuilder, ir::{ basic_block::BasicBlockId, + call_stack::CallStackId, + dfg::GlobalsGraph, function::{Function, FunctionId}, - instruction::ConstrainError, + instruction::{ConstrainError, Instruction}, value::ValueId, }, }; use super::{ - ast::AssertMessage, Identifier, ParsedBlock, ParsedFunction, ParsedInstruction, ParsedSsa, - ParsedTerminator, ParsedValue, RuntimeType, Ssa, SsaError, + ast::AssertMessage, Identifier, ParsedBlock, ParsedFunction, ParsedGlobal, ParsedGlobalValue, + ParsedInstruction, ParsedSsa, ParsedTerminator, ParsedValue, RuntimeType, Ssa, SsaError, Type, }; impl ParsedSsa { @@ -39,6 +41,17 @@ struct Translator { /// will recreate the SSA step by step, which can result in a new ID layout. variables: HashMap>, + /// The function that will hold the actual SSA globals. + globals_function: Function, + + /// The types of globals in the parsed SSA, in the order they were defined. + global_types: Vec, + + /// Maps names (e.g. "g0") in the parsed SSA to global IDs. + global_values: HashMap, + + globals_graph: Arc, + error_selector_counter: u64, } @@ -74,13 +87,26 @@ impl Translator { functions.insert(function.internal_name.clone(), function_id); } + // Does not matter what ID we use here. + let globals = Function::new("globals".to_owned(), main_id); + let mut translator = Self { builder, functions, variables: HashMap::new(), blocks: HashMap::new(), + globals_function: globals, + global_types: Vec::new(), + global_values: HashMap::new(), + globals_graph: Arc::new(GlobalsGraph::default()), error_selector_counter: 0, }; + + translator.translate_globals(std::mem::take(&mut parsed_ssa.globals))?; + + translator.globals_graph = + Arc::new(GlobalsGraph::from_dfg(translator.globals_function.dfg.clone())); + translator.translate_function_body(main_function)?; Ok(translator) @@ -103,6 +129,8 @@ impl Translator { } fn translate_function_body(&mut self, function: ParsedFunction) -> Result<(), SsaError> { + self.builder.set_globals(self.globals_graph.clone()); + // First define all blocks so that they are known (a block might jump to a block that comes next) for (index, block) in function.blocks.iter().enumerate() { // The first block is the entry block and it was automatically created by the builder @@ -297,8 +325,8 @@ impl Translator { fn translate_value(&mut self, value: ParsedValue) -> Result { match value { - ParsedValue::NumericConstant { constant, typ } => { - Ok(self.builder.numeric_constant(constant, typ.unwrap_numeric())) + ParsedValue::NumericConstant(constant) => { + Ok(self.builder.numeric_constant(constant.value, constant.typ.unwrap_numeric())) } ParsedValue::Variable(identifier) => self.lookup_variable(&identifier).or_else(|e| { self.lookup_function(&identifier) @@ -311,6 +339,45 @@ impl Translator { } } + fn translate_globals(&mut self, globals: Vec) -> Result<(), SsaError> { + for global in globals { + self.translate_global(global)?; + } + Ok(()) + } + + fn translate_global(&mut self, global: ParsedGlobal) -> Result<(), SsaError> { + let value_id = match global.value { + ParsedGlobalValue::NumericConstant(constant) => self + .globals_function + .dfg + .make_constant(constant.value, constant.typ.unwrap_numeric()), + ParsedGlobalValue::MakeArray(make_array) => { + let mut elements = im::Vector::new(); + for element in make_array.elements { + let element_id = match element { + ParsedValue::NumericConstant(constant) => self + .globals_function + .dfg + .make_constant(constant.value, constant.typ.unwrap_numeric()), + ParsedValue::Variable(identifier) => self.lookup_global(identifier)?, + }; + elements.push_back(element_id); + } + + let instruction = Instruction::MakeArray { elements, typ: make_array.typ.clone() }; + let block = self.globals_function.entry_block(); + let call_stack = CallStackId::root(); + self.globals_function + .dfg + .insert_instruction_and_results(instruction, block, None, call_stack) + .first() + } + }; + + self.define_global(global.name, value_id) + } + fn define_variable( &mut self, identifier: Identifier, @@ -329,13 +396,40 @@ impl Translator { } fn lookup_variable(&mut self, identifier: &Identifier) -> Result { - if let Some(value_id) = self.variables[&self.current_function_id()].get(&identifier.name) { + if let Some(value_id) = self + .variables + .get(&self.current_function_id()) + .and_then(|hash| hash.get(&identifier.name)) + { + Ok(*value_id) + } else if let Some(value_id) = self.global_values.get(&identifier.name) { Ok(*value_id) } else { Err(SsaError::UnknownVariable(identifier.clone())) } } + fn define_global(&mut self, identifier: Identifier, value_id: ValueId) -> Result<(), SsaError> { + if self.global_values.contains_key(&identifier.name) { + return Err(SsaError::GlobalAlreadyDefined(identifier)); + } + + self.global_values.insert(identifier.name, value_id); + + let typ = self.globals_function.dfg.type_of_value(value_id); + self.global_types.push(typ); + + Ok(()) + } + + fn lookup_global(&mut self, identifier: Identifier) -> Result { + if let Some(value_id) = self.global_values.get(&identifier.name) { + Ok(*value_id) + } else { + Err(SsaError::UnknownGlobal(identifier)) + } + } + fn lookup_block(&mut self, identifier: &Identifier) -> Result { if let Some(block_id) = self.blocks[&self.current_function_id()].get(&identifier.name) { Ok(*block_id) @@ -354,13 +448,14 @@ impl Translator { fn finish(self) -> Ssa { let mut ssa = self.builder.finish(); + ssa.globals = self.globals_function; + // Normalize the IDs so we have a better chance of matching the SSA we parsed // after the step-by-step reconstruction done during translation. This assumes // that the SSA we parsed was printed by the `SsaBuilder`, which normalizes // before each print. ssa.normalize_ids(); - // Does not matter what ID we use here. - ssa.globals = Function::new("globals".to_owned(), ssa.main_id); + ssa } diff --git a/compiler/noirc_evaluator/src/ssa/parser/mod.rs b/compiler/noirc_evaluator/src/ssa/parser/mod.rs index 143ba511879..cc660355bbd 100644 --- a/compiler/noirc_evaluator/src/ssa/parser/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/parser/mod.rs @@ -13,8 +13,9 @@ use super::{ use acvm::{AcirField, FieldElement}; use ast::{ - AssertMessage, Identifier, ParsedBlock, ParsedFunction, ParsedInstruction, ParsedParameter, - ParsedSsa, ParsedValue, + AssertMessage, Identifier, ParsedBlock, ParsedFunction, ParsedGlobal, ParsedGlobalValue, + ParsedInstruction, ParsedMakeArray, ParsedNumericConstant, ParsedParameter, ParsedSsa, + ParsedValue, }; use lexer::{Lexer, LexerError}; use noirc_errors::Span; @@ -99,6 +100,8 @@ pub(crate) enum SsaError { ParserError(ParserError), #[error("Unknown variable '{0}'")] UnknownVariable(Identifier), + #[error("Unknown global '{0}'")] + UnknownGlobal(Identifier), #[error("Unknown block '{0}'")] UnknownBlock(Identifier), #[error("Unknown function '{0}'")] @@ -107,6 +110,8 @@ pub(crate) enum SsaError { MismatchedReturnValues { returns: Vec, expected: usize }, #[error("Variable '{0}' already defined")] VariableAlreadyDefined(Identifier), + #[error("Global '{0}' already defined")] + GlobalAlreadyDefined(Identifier), } impl SsaError { @@ -114,8 +119,10 @@ impl SsaError { match self { SsaError::ParserError(parser_error) => parser_error.span(), SsaError::UnknownVariable(identifier) + | SsaError::UnknownGlobal(identifier) | SsaError::UnknownBlock(identifier) | SsaError::VariableAlreadyDefined(identifier) + | SsaError::GlobalAlreadyDefined(identifier) | SsaError::UnknownFunction(identifier) => identifier.span, SsaError::MismatchedReturnValues { returns, expected: _ } => returns[0].span, } @@ -138,12 +145,39 @@ impl<'a> Parser<'a> { } pub(crate) fn parse_ssa(&mut self) -> ParseResult { + let globals = self.parse_globals()?; + let mut functions = Vec::new(); while !self.at(Token::Eof) { let function = self.parse_function()?; functions.push(function); } - Ok(ParsedSsa { functions }) + Ok(ParsedSsa { globals, functions }) + } + + fn parse_globals(&mut self) -> ParseResult> { + let mut globals = Vec::new(); + + while let Some(name) = self.eat_identifier()? { + self.eat_or_error(Token::Assign)?; + + let value = self.parse_global_value()?; + globals.push(ParsedGlobal { name, value }); + } + + Ok(globals) + } + + fn parse_global_value(&mut self) -> ParseResult { + if let Some(constant) = self.parse_numeric_constant()? { + return Ok(ParsedGlobalValue::NumericConstant(constant)); + } + + if let Some(make_array) = self.parse_make_array()? { + return Ok(ParsedGlobalValue::MakeArray(make_array)); + } + + self.expected_global_value() } fn parse_function(&mut self) -> ParseResult { @@ -461,40 +495,12 @@ impl<'a> Parser<'a> { return Ok(ParsedInstruction::Load { target, value, typ }); } - if self.eat_keyword(Keyword::MakeArray)? { - if self.eat(Token::Ampersand)? { - let Some(string) = self.eat_byte_str()? else { - return self.expected_byte_string(); - }; - let u8 = Type::Numeric(NumericType::Unsigned { bit_size: 8 }); - let typ = Type::Slice(Arc::new(vec![u8.clone()])); - let elements = string - .bytes() - .map(|byte| ParsedValue::NumericConstant { - constant: FieldElement::from(byte as u128), - typ: u8.clone(), - }) - .collect(); - return Ok(ParsedInstruction::MakeArray { target, elements, typ }); - } else if let Some(string) = self.eat_byte_str()? { - let u8 = Type::Numeric(NumericType::Unsigned { bit_size: 8 }); - let typ = Type::Array(Arc::new(vec![u8.clone()]), string.len() as u32); - let elements = string - .bytes() - .map(|byte| ParsedValue::NumericConstant { - constant: FieldElement::from(byte as u128), - typ: u8.clone(), - }) - .collect(); - return Ok(ParsedInstruction::MakeArray { target, elements, typ }); - } else { - self.eat_or_error(Token::LeftBracket)?; - let elements = self.parse_comma_separated_values()?; - self.eat_or_error(Token::RightBracket)?; - self.eat_or_error(Token::Colon)?; - let typ = self.parse_type()?; - return Ok(ParsedInstruction::MakeArray { target, elements, typ }); - } + if let Some(make_array) = self.parse_make_array()? { + return Ok(ParsedInstruction::MakeArray { + target, + elements: make_array.elements, + typ: make_array.typ, + }); } if self.eat_keyword(Keyword::Not)? { @@ -524,6 +530,52 @@ impl<'a> Parser<'a> { self.expected_instruction_or_terminator() } + fn parse_make_array(&mut self) -> ParseResult> { + if !self.eat_keyword(Keyword::MakeArray)? { + return Ok(None); + } + + let make_array = if self.eat(Token::Ampersand)? { + let Some(string) = self.eat_byte_str()? else { + return self.expected_byte_string(); + }; + let u8 = Type::Numeric(NumericType::Unsigned { bit_size: 8 }); + let typ = Type::Slice(Arc::new(vec![u8.clone()])); + let elements = string + .bytes() + .map(|byte| { + ParsedValue::NumericConstant(ParsedNumericConstant { + value: FieldElement::from(byte as u128), + typ: u8.clone(), + }) + }) + .collect(); + ParsedMakeArray { elements, typ } + } else if let Some(string) = self.eat_byte_str()? { + let u8 = Type::Numeric(NumericType::Unsigned { bit_size: 8 }); + let typ = Type::Array(Arc::new(vec![u8.clone()]), string.len() as u32); + let elements = string + .bytes() + .map(|byte| { + ParsedValue::NumericConstant(ParsedNumericConstant { + value: FieldElement::from(byte as u128), + typ: u8.clone(), + }) + }) + .collect(); + ParsedMakeArray { elements, typ } + } else { + self.eat_or_error(Token::LeftBracket)?; + let elements = self.parse_comma_separated_values()?; + self.eat_or_error(Token::RightBracket)?; + self.eat_or_error(Token::Colon)?; + let typ = self.parse_type()?; + ParsedMakeArray { elements, typ } + }; + + Ok(Some(make_array)) + } + fn parse_terminator(&mut self) -> ParseResult { if let Some(terminator) = self.parse_return()? { return Ok(terminator); @@ -617,12 +669,8 @@ impl<'a> Parser<'a> { } fn parse_value(&mut self) -> ParseResult> { - if let Some(value) = self.parse_field_value()? { - return Ok(Some(value)); - } - - if let Some(value) = self.parse_int_value()? { - return Ok(Some(value)); + if let Some(constant) = self.parse_numeric_constant()? { + return Ok(Some(ParsedValue::NumericConstant(constant))); } if let Some(identifier) = self.eat_identifier()? { @@ -632,23 +680,35 @@ impl<'a> Parser<'a> { Ok(None) } - fn parse_field_value(&mut self) -> ParseResult> { + fn parse_numeric_constant(&mut self) -> ParseResult> { + if let Some(constant) = self.parse_field_value()? { + return Ok(Some(constant)); + } + + if let Some(constant) = self.parse_int_value()? { + return Ok(Some(constant)); + } + + Ok(None) + } + + fn parse_field_value(&mut self) -> ParseResult> { if self.eat_keyword(Keyword::Field)? { - let constant = self.eat_int_or_error()?; - Ok(Some(ParsedValue::NumericConstant { constant, typ: Type::field() })) + let value = self.eat_int_or_error()?; + Ok(Some(ParsedNumericConstant { value, typ: Type::field() })) } else { Ok(None) } } - fn parse_int_value(&mut self) -> ParseResult> { + fn parse_int_value(&mut self) -> ParseResult> { if let Some(int_type) = self.eat_int_type()? { - let constant = self.eat_int_or_error()?; + let value = self.eat_int_or_error()?; let typ = match int_type { IntType::Unsigned(bit_size) => Type::unsigned(bit_size), IntType::Signed(bit_size) => Type::signed(bit_size), }; - Ok(Some(ParsedValue::NumericConstant { constant, typ })) + Ok(Some(ParsedNumericConstant { value, typ })) } else { Ok(None) } @@ -932,6 +992,13 @@ impl<'a> Parser<'a> { }) } + fn expected_global_value(&mut self) -> ParseResult { + Err(ParserError::ExpectedGlobalValue { + found: self.token.token().clone(), + span: self.token.to_span(), + }) + } + fn expected_token(&mut self, token: Token) -> ParseResult { Err(ParserError::ExpectedToken { token, @@ -971,6 +1038,10 @@ pub(crate) enum ParserError { ExpectedByteString { found: Token, span: Span }, #[error("Expected a value, found '{found}'")] ExpectedValue { found: Token, span: Span }, + #[error( + "Expected a global value (Field literal, integer literal or make_array), found '{found}'" + )] + ExpectedGlobalValue { found: Token, span: Span }, #[error("Multiple return values only allowed for call")] MultipleReturnValuesOnlyAllowedForCall { second_target: Identifier }, } @@ -987,7 +1058,8 @@ impl ParserError { | ParserError::ExpectedInstructionOrTerminator { span, .. } | ParserError::ExpectedStringOrData { span, .. } | ParserError::ExpectedByteString { span, .. } - | ParserError::ExpectedValue { span, .. } => *span, + | ParserError::ExpectedValue { span, .. } + | ParserError::ExpectedGlobalValue { span, .. } => *span, ParserError::MultipleReturnValuesOnlyAllowedForCall { second_target, .. } => { second_target.span } diff --git a/compiler/noirc_evaluator/src/ssa/parser/tests.rs b/compiler/noirc_evaluator/src/ssa/parser/tests.rs index 8c24b2ec458..c803e2a94fe 100644 --- a/compiler/noirc_evaluator/src/ssa/parser/tests.rs +++ b/compiler/noirc_evaluator/src/ssa/parser/tests.rs @@ -530,3 +530,19 @@ fn test_does_not_simplify() { "; assert_ssa_roundtrip(src); } + +#[test] +fn parses_globals() { + let src = " + g0 = Field 0 + g1 = u32 1 + g2 = make_array [] : [Field; 0] + g3 = make_array [g2] : [[Field; 0]; 1] + + acir(inline) fn main f0 { + b0(): + return g3 + } + "; + assert_ssa_roundtrip(src); +} From 8804f0a8c4f75b51fad83fc1a423da2b36f184af Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 21 Jan 2025 17:00:24 -0300 Subject: [PATCH 02/11] feat: `loop` must have at least one `break` (#7126) --- compiler/noirc_frontend/src/ast/statement.rs | 4 +- compiler/noirc_frontend/src/ast/visitor.rs | 2 +- compiler/noirc_frontend/src/elaborator/mod.rs | 9 +- .../src/elaborator/statements.rs | 27 ++++-- .../src/hir/comptime/display.rs | 4 +- .../src/hir/comptime/hir_to_display_ast.rs | 2 +- .../src/hir/resolution/errors.rs | 11 ++- .../src/parser/parser/statement.rs | 14 ++-- compiler/noirc_frontend/src/tests.rs | 82 +++++++++++++++++++ docs/docs/noir/concepts/control_flow.md | 5 +- tooling/nargo_fmt/src/formatter/statement.rs | 2 +- 11 files changed, 137 insertions(+), 25 deletions(-) diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index 3a70ff33a35..7695de14d69 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -46,7 +46,7 @@ pub enum StatementKind { Expression(Expression), Assign(AssignStatement), For(ForLoopStatement), - Loop(Expression), + Loop(Expression, Span /* loop keyword span */), Break, Continue, /// This statement should be executed at compile-time @@ -972,7 +972,7 @@ impl Display for StatementKind { StatementKind::Expression(expression) => expression.fmt(f), StatementKind::Assign(assign) => assign.fmt(f), StatementKind::For(for_loop) => for_loop.fmt(f), - StatementKind::Loop(block) => write!(f, "loop {}", block), + StatementKind::Loop(block, _) => write!(f, "loop {}", block), StatementKind::Break => write!(f, "break"), StatementKind::Continue => write!(f, "continue"), StatementKind::Comptime(statement) => write!(f, "comptime {}", statement.kind), diff --git a/compiler/noirc_frontend/src/ast/visitor.rs b/compiler/noirc_frontend/src/ast/visitor.rs index 5c4781df7a5..d7fe63a6a45 100644 --- a/compiler/noirc_frontend/src/ast/visitor.rs +++ b/compiler/noirc_frontend/src/ast/visitor.rs @@ -1135,7 +1135,7 @@ impl Statement { StatementKind::For(for_loop_statement) => { for_loop_statement.accept(visitor); } - StatementKind::Loop(block) => { + StatementKind::Loop(block, _) => { if visitor.visit_loop_statement(block) { block.accept(visitor); } diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 0e8850b6543..96ff1597647 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -94,6 +94,11 @@ enum UnsafeBlockStatus { InUnsafeBlockWithConstrainedCalls, } +pub struct Loop { + pub is_for: bool, + pub has_break: bool, +} + pub struct Elaborator<'context> { scopes: ScopeForest, @@ -107,7 +112,7 @@ pub struct Elaborator<'context> { pub(crate) file: FileId, unsafe_block_status: UnsafeBlockStatus, - nested_loops: usize, + current_loop: Option, /// Contains a mapping of the current struct or functions's generics to /// unique type variables if we're resolving a struct. Empty otherwise. @@ -230,7 +235,7 @@ impl<'context> Elaborator<'context> { crate_graph, file: FileId::dummy(), unsafe_block_status: UnsafeBlockStatus::NotInUnsafeBlock, - nested_loops: 0, + current_loop: None, generics: Vec::new(), lambda_stack: Vec::new(), self_type: None, diff --git a/compiler/noirc_frontend/src/elaborator/statements.rs b/compiler/noirc_frontend/src/elaborator/statements.rs index a01b24c2f0f..150c6e7de48 100644 --- a/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/compiler/noirc_frontend/src/elaborator/statements.rs @@ -24,7 +24,7 @@ use crate::{ StructType, Type, }; -use super::{lints, Elaborator}; +use super::{lints, Elaborator, Loop}; impl<'context> Elaborator<'context> { fn elaborate_statement_value(&mut self, statement: Statement) -> (HirStatement, Type) { @@ -33,7 +33,7 @@ impl<'context> Elaborator<'context> { StatementKind::Constrain(constrain) => self.elaborate_constrain(constrain), StatementKind::Assign(assign) => self.elaborate_assign(assign), StatementKind::For(for_stmt) => self.elaborate_for(for_stmt), - StatementKind::Loop(block) => self.elaborate_loop(block, statement.span), + StatementKind::Loop(block, span) => self.elaborate_loop(block, span), StatementKind::Break => self.elaborate_jump(true, statement.span), StatementKind::Continue => self.elaborate_jump(false, statement.span), StatementKind::Comptime(statement) => self.elaborate_comptime_statement(*statement), @@ -227,7 +227,9 @@ impl<'context> Elaborator<'context> { let (end_range, end_range_type) = self.elaborate_expression(end); let (identifier, block) = (for_loop.identifier, for_loop.block); - self.nested_loops += 1; + let old_loop = std::mem::take(&mut self.current_loop); + + self.current_loop = Some(Loop { is_for: true, has_break: false }); self.push_scope(); // TODO: For loop variables are currently mutable by default since we haven't @@ -261,7 +263,7 @@ impl<'context> Elaborator<'context> { let (block, _block_type) = self.elaborate_expression(block); self.pop_scope(); - self.nested_loops -= 1; + self.current_loop = old_loop; let statement = HirStatement::For(HirForStatement { start_range, end_range, block, identifier }); @@ -279,13 +281,19 @@ impl<'context> Elaborator<'context> { self.push_err(ResolverError::LoopInConstrainedFn { span }); } - self.nested_loops += 1; + let old_loop = std::mem::take(&mut self.current_loop); + self.current_loop = Some(Loop { is_for: false, has_break: false }); self.push_scope(); let (block, _block_type) = self.elaborate_expression(block); self.pop_scope(); - self.nested_loops -= 1; + + let last_loop = + std::mem::replace(&mut self.current_loop, old_loop).expect("Expected a loop"); + if !last_loop.has_break { + self.push_err(ResolverError::LoopWithoutBreak { span }); + } let statement = HirStatement::Loop(block); @@ -298,7 +306,12 @@ impl<'context> Elaborator<'context> { if in_constrained_function { self.push_err(ResolverError::JumpInConstrainedFn { is_break, span }); } - if self.nested_loops == 0 { + + if let Some(current_loop) = &mut self.current_loop { + if is_break { + current_loop.has_break = true; + } + } else { self.push_err(ResolverError::JumpOutsideLoop { is_break, span }); } diff --git a/compiler/noirc_frontend/src/hir/comptime/display.rs b/compiler/noirc_frontend/src/hir/comptime/display.rs index ccdfdf00e72..39bcb87e145 100644 --- a/compiler/noirc_frontend/src/hir/comptime/display.rs +++ b/compiler/noirc_frontend/src/hir/comptime/display.rs @@ -732,8 +732,8 @@ fn remove_interned_in_statement_kind( block: remove_interned_in_expression(interner, for_loop.block), ..for_loop }), - StatementKind::Loop(block) => { - StatementKind::Loop(remove_interned_in_expression(interner, block)) + StatementKind::Loop(block, span) => { + StatementKind::Loop(remove_interned_in_expression(interner, block), span) } StatementKind::Comptime(statement) => { StatementKind::Comptime(Box::new(remove_interned_in_statement(interner, *statement))) diff --git a/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs b/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs index 71a462c9066..0257ff66ac6 100644 --- a/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs +++ b/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs @@ -59,7 +59,7 @@ impl HirStatement { block: for_stmt.block.to_display_ast(interner), span, }), - HirStatement::Loop(block) => StatementKind::Loop(block.to_display_ast(interner)), + HirStatement::Loop(block) => StatementKind::Loop(block.to_display_ast(interner), span), HirStatement::Break => StatementKind::Break, HirStatement::Continue => StatementKind::Continue, HirStatement::Expression(expr) => { diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs index 77ba76a0595..f8373080069 100644 --- a/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -98,8 +98,10 @@ pub enum ResolverError { DependencyCycle { span: Span, item: String, cycle: String }, #[error("break/continue are only allowed in unconstrained functions")] JumpInConstrainedFn { is_break: bool, span: Span }, - #[error("loop is only allowed in unconstrained functions")] + #[error("`loop` is only allowed in unconstrained functions")] LoopInConstrainedFn { span: Span }, + #[error("`loop` must have at least one `break` in it")] + LoopWithoutBreak { span: Span }, #[error("break/continue are only allowed within loops")] JumpOutsideLoop { is_break: bool, span: Span }, #[error("Only `comptime` globals can be mutable")] @@ -443,6 +445,13 @@ impl<'a> From<&'a ResolverError> for Diagnostic { *span, ) }, + ResolverError::LoopWithoutBreak { span } => { + Diagnostic::simple_error( + "`loop` must have at least one `break` in it".into(), + "Infinite loops are disallowed".into(), + *span, + ) + }, ResolverError::JumpOutsideLoop { is_break, span } => { let item = if *is_break { "break" } else { "continue" }; Diagnostic::simple_error( diff --git a/compiler/noirc_frontend/src/parser/parser/statement.rs b/compiler/noirc_frontend/src/parser/parser/statement.rs index 8c6d18d90ef..005216b1deb 100644 --- a/compiler/noirc_frontend/src/parser/parser/statement.rs +++ b/compiler/noirc_frontend/src/parser/parser/statement.rs @@ -157,8 +157,8 @@ impl<'a> Parser<'a> { return Some(StatementKind::For(for_loop)); } - if let Some(block) = self.parse_loop() { - return Some(StatementKind::Loop(block)); + if let Some((block, span)) = self.parse_loop() { + return Some(StatementKind::Loop(block, span)); } if let Some(kind) = self.parse_if_expr() { @@ -293,7 +293,7 @@ impl<'a> Parser<'a> { } /// LoopStatement = 'loop' Block - fn parse_loop(&mut self) -> Option { + fn parse_loop(&mut self) -> Option<(Expression, Span)> { let start_span = self.current_token_span; if !self.eat_keyword(Keyword::Loop) { return None; @@ -312,7 +312,7 @@ impl<'a> Parser<'a> { Expression { kind: ExpressionKind::Error, span: self.span_since(block_start_span) } }; - Some(block) + Some((block, start_span)) } /// ForRange @@ -824,13 +824,15 @@ mod tests { let src = "loop { }"; let mut parser = Parser::for_str(src); let statement = parser.parse_statement_or_error(); - let StatementKind::Loop(block) = statement.kind else { + let StatementKind::Loop(block, span) = statement.kind else { panic!("Expected loop"); }; let ExpressionKind::Block(block) = block.kind else { panic!("Expected block"); }; assert!(block.statements.is_empty()); + assert_eq!(span.start(), 0); + assert_eq!(span.end(), 4); } #[test] @@ -838,7 +840,7 @@ mod tests { let src = "loop { 1; 2 }"; let mut parser = Parser::for_str(src); let statement = parser.parse_statement_or_error(); - let StatementKind::Loop(block) = statement.kind else { + let StatementKind::Loop(block, _) = statement.kind else { panic!("Expected loop"); }; let ExpressionKind::Block(block) = block.kind else { diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 087e34fcc64..c2d7e5c5f49 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -4073,3 +4073,85 @@ fn regression_7088() { "#; assert_no_errors(src); } + +#[test] +fn errors_on_empty_loop_no_break() { + let src = r#" + fn main() { + /// Safety: test + unsafe { + foo() + } + } + + unconstrained fn foo() { + loop {} + } + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + assert!(matches!( + &errors[0].0, + CompilationError::ResolverError(ResolverError::LoopWithoutBreak { .. }) + )); +} + +#[test] +fn errors_on_loop_without_break() { + let src = r#" + fn main() { + /// Safety: test + unsafe { + foo() + } + } + + unconstrained fn foo() { + let mut x = 1; + loop { + x += 1; + bar(x); + } + } + + fn bar(_: Field) {} + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + assert!(matches!( + &errors[0].0, + CompilationError::ResolverError(ResolverError::LoopWithoutBreak { .. }) + )); +} + +#[test] +fn errors_on_loop_without_break_with_nested_loop() { + let src = r#" + fn main() { + /// Safety: test + unsafe { + foo() + } + } + + unconstrained fn foo() { + let mut x = 1; + loop { + x += 1; + bar(x); + loop { + x += 2; + break; + } + } + } + + fn bar(_: Field) {} + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + assert!(matches!( + &errors[0].0, + CompilationError::ResolverError(ResolverError::LoopWithoutBreak { .. }) + )); +} diff --git a/docs/docs/noir/concepts/control_flow.md b/docs/docs/noir/concepts/control_flow.md index a11db545e32..3e2d913ec96 100644 --- a/docs/docs/noir/concepts/control_flow.md +++ b/docs/docs/noir/concepts/control_flow.md @@ -79,8 +79,9 @@ The iteration variable `i` is still increased by one as normal when `continue` i ## Loops -In unconstrained code, `loop` is allowed for loops that end with a `break`. This is only allowed -in unconstrained code since normal constrained code requires that Noir knows exactly how many iterations +In unconstrained code, `loop` is allowed for loops that end with a `break`. +A `loop` must have at least one `break` in it. +This is only allowed in unconstrained code since normal constrained code requires that Noir knows exactly how many iterations a loop may have. ```rust diff --git a/tooling/nargo_fmt/src/formatter/statement.rs b/tooling/nargo_fmt/src/formatter/statement.rs index 983fb00db16..0006d24180c 100644 --- a/tooling/nargo_fmt/src/formatter/statement.rs +++ b/tooling/nargo_fmt/src/formatter/statement.rs @@ -75,7 +75,7 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { StatementKind::For(for_loop_statement) => { group.group(self.format_for_loop(for_loop_statement)); } - StatementKind::Loop(block) => { + StatementKind::Loop(block, _) => { group.group(self.format_loop(block)); } StatementKind::Break => { From a1cf830b3cdf17a9265b8bdbf366d65c253f0ca4 Mon Sep 17 00:00:00 2001 From: jfecher Date: Tue, 21 Jan 2025 14:37:44 -0600 Subject: [PATCH 03/11] feat: Resolve enums & prepare type system (#7115) Co-authored-by: Ary Borenszweig --- compiler/noirc_driver/src/abi_gen.rs | 2 +- compiler/noirc_driver/src/lib.rs | 2 +- compiler/noirc_frontend/src/ast/expression.rs | 4 +- .../noirc_frontend/src/elaborator/comptime.rs | 4 +- .../src/elaborator/expressions.rs | 12 +- compiler/noirc_frontend/src/elaborator/mod.rs | 26 +- .../src/elaborator/path_resolution.rs | 95 +++---- .../noirc_frontend/src/elaborator/patterns.rs | 16 +- .../noirc_frontend/src/elaborator/scope.rs | 20 +- .../src/elaborator/statements.rs | 6 +- .../src/elaborator/trait_impls.rs | 2 +- .../noirc_frontend/src/elaborator/types.rs | 66 ++--- .../src/hir/comptime/display.rs | 4 +- .../src/hir/comptime/hir_to_display_ast.rs | 4 +- .../src/hir/comptime/interpreter/builtin.rs | 24 +- .../interpreter/builtin/builtin_helpers.rs | 15 +- .../noirc_frontend/src/hir/comptime/value.rs | 8 +- .../src/hir/def_collector/dc_crate.rs | 8 +- .../src/hir/def_collector/dc_mod.rs | 158 +++++++++-- .../src/hir/def_collector/errors.rs | 33 +-- .../noirc_frontend/src/hir/def_map/mod.rs | 4 +- .../src/hir/def_map/module_data.rs | 6 +- .../src/hir/def_map/module_def.rs | 6 +- compiler/noirc_frontend/src/hir/mod.rs | 4 +- .../src/hir/resolution/errors.rs | 8 +- .../src/hir/resolution/visibility.rs | 4 +- .../src/hir/type_check/generics.rs | 4 +- compiler/noirc_frontend/src/hir_def/expr.rs | 4 +- .../noirc_frontend/src/hir_def/function.rs | 4 +- compiler/noirc_frontend/src/hir_def/types.rs | 248 +++++++++++------- compiler/noirc_frontend/src/locations.rs | 41 ++- .../src/monomorphization/mod.rs | 6 +- compiler/noirc_frontend/src/node_interner.rs | 86 +++--- .../noirc_frontend/src/resolve_locations.rs | 6 +- compiler/noirc_frontend/src/tests.rs | 5 +- compiler/noirc_frontend/src/usage_tracker.rs | 6 +- .../code_action/fill_struct_fields.rs | 2 +- tooling/lsp/src/requests/completion.rs | 22 +- .../requests/completion/completion_items.rs | 6 +- tooling/lsp/src/requests/hover.rs | 90 ++++++- tooling/lsp/src/requests/inlay_hint.rs | 12 +- .../src/trait_impl_method_stub_generator.rs | 2 +- 42 files changed, 657 insertions(+), 428 deletions(-) diff --git a/compiler/noirc_driver/src/abi_gen.rs b/compiler/noirc_driver/src/abi_gen.rs index 625a35c8d15..59b3faf1a4e 100644 --- a/compiler/noirc_driver/src/abi_gen.rs +++ b/compiler/noirc_driver/src/abi_gen.rs @@ -110,7 +110,7 @@ pub(super) fn abi_type_from_hir_type(context: &Context, typ: &Type) -> AbiType { AbiType::String { length: size } } - Type::Struct(def, args) => { + Type::DataType(def, args) => { let struct_type = def.borrow(); let fields = struct_type.get_fields(args); let fields = diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index a7e7e2d4e2f..be5cde1e0ea 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -549,7 +549,7 @@ fn compile_contract_inner( let structs = structs .into_iter() .map(|struct_id| { - let typ = context.def_interner.get_struct(struct_id); + let typ = context.def_interner.get_type(struct_id); let typ = typ.borrow(); let fields = vecmap(typ.get_fields(&[]), |(name, typ)| { (name, abi_type_from_hir_type(context, &typ)) diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs index 9d521545e7a..1f7a37428b2 100644 --- a/compiler/noirc_frontend/src/ast/expression.rs +++ b/compiler/noirc_frontend/src/ast/expression.rs @@ -8,7 +8,7 @@ use crate::ast::{ UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData, Visibility, }; use crate::node_interner::{ - ExprId, InternedExpressionKind, InternedStatementKind, QuotedTypeId, StructId, + ExprId, InternedExpressionKind, InternedStatementKind, QuotedTypeId, TypeId, }; use crate::token::{Attributes, FmtStrFragment, FunctionAttribute, Token, Tokens}; use crate::{Kind, Type}; @@ -559,7 +559,7 @@ pub struct ConstructorExpression { /// This may be filled out during macro expansion /// so that we can skip re-resolving the type name since it /// would be lost at that point. - pub struct_type: Option, + pub struct_type: Option, } #[derive(Debug, PartialEq, Eq, Clone)] diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index 9f5eef6e785..c13c74f44cb 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -19,7 +19,7 @@ use crate::{ resolution::errors::ResolverError, }, hir_def::expr::{HirExpression, HirIdent}, - node_interner::{DefinitionKind, DependencyId, FuncId, NodeInterner, StructId, TraitId}, + node_interner::{DefinitionKind, DependencyId, FuncId, NodeInterner, TraitId, TypeId}, parser::{Item, ItemKind}, token::{MetaAttribute, SecondaryAttribute}, Type, TypeBindings, UnificationError, @@ -512,7 +512,7 @@ impl<'context> Elaborator<'context> { pub(super) fn run_attributes( &mut self, traits: &BTreeMap, - types: &BTreeMap, + types: &BTreeMap, functions: &[UnresolvedFunctions], module_attributes: &[ModuleAttribute], ) { diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index ef2ae9c4df0..0cee880e781 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -29,7 +29,7 @@ use crate::{ }, node_interner::{DefinitionKind, ExprId, FuncId, InternedStatementKind, TraitMethodId}, token::{FmtStrFragment, Tokens}, - Kind, QuotedType, Shared, StructType, Type, + DataType, Kind, QuotedType, Shared, Type, }; use super::{Elaborator, LambdaContext, UnsafeBlockStatus}; @@ -614,12 +614,12 @@ impl<'context> Elaborator<'context> { let is_self_type = last_segment.ident.is_self_type_name(); let (r#type, struct_generics) = if let Some(struct_id) = constructor.struct_type { - let typ = self.interner.get_struct(struct_id); + let typ = self.interner.get_type(struct_id); let generics = typ.borrow().instantiate(self.interner); (typ, generics) } else { match self.lookup_type_or_error(path) { - Some(Type::Struct(r#type, struct_generics)) => (r#type, struct_generics), + Some(Type::DataType(r#type, struct_generics)) => (r#type, struct_generics), Some(typ) => { self.push_err(ResolverError::NonStructUsedInConstructor { typ: typ.to_string(), @@ -659,10 +659,10 @@ impl<'context> Elaborator<'context> { let reference_location = Location::new(last_segment.ident.span(), self.file); self.interner.add_struct_reference(struct_id, reference_location, is_self_type); - (expr, Type::Struct(struct_type, generics)) + (expr, Type::DataType(struct_type, generics)) } - pub(super) fn mark_struct_as_constructed(&mut self, struct_type: Shared) { + pub(super) fn mark_struct_as_constructed(&mut self, struct_type: Shared) { let struct_type = struct_type.borrow(); let parent_module_id = struct_type.id.parent_module_id(self.def_maps); self.usage_tracker.mark_as_used(parent_module_id, &struct_type.name); @@ -673,7 +673,7 @@ impl<'context> Elaborator<'context> { /// are part of the struct. fn resolve_constructor_expr_fields( &mut self, - struct_type: Shared, + struct_type: Shared, field_types: Vec<(String, ItemVisibility, Type)>, fields: Vec<(Ident, Expression)>, span: Span, diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 96ff1597647..1e792ada677 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -32,7 +32,7 @@ use crate::{ }, node_interner::{ DefinitionKind, DependencyId, ExprId, FuncId, FunctionModifiers, GlobalId, NodeInterner, - ReferenceId, StructId, TraitId, TraitImplId, TypeAliasId, + ReferenceId, TraitId, TraitImplId, TypeAliasId, TypeId, }, token::SecondaryAttribute, Shared, Type, TypeVariable, @@ -43,7 +43,7 @@ use crate::{ hir_def::traits::ResolvedTraitBound, node_interner::GlobalValue, usage_tracker::UsageTracker, - StructField, StructType, TypeBindings, + DataType, StructField, TypeBindings, }; mod comptime; @@ -152,7 +152,7 @@ pub struct Elaborator<'context> { /// struct Wrapped { /// } /// ``` - resolving_ids: BTreeSet, + resolving_ids: BTreeSet, /// Each constraint in the `where` clause of the function currently being resolved. trait_bounds: Vec, @@ -982,7 +982,7 @@ impl<'context> Elaborator<'context> { let statements = std::mem::take(&mut func.def.body.statements); let body = BlockExpression { statements }; - let struct_id = if let Some(Type::Struct(struct_type, _)) = &self.self_type { + let struct_id = if let Some(Type::DataType(struct_type, _)) = &self.self_type { Some(struct_type.borrow().id) } else { None @@ -1030,7 +1030,7 @@ impl<'context> Elaborator<'context> { self.mark_type_as_used(typ); } } - Type::Struct(struct_type, generics) => { + Type::DataType(struct_type, generics) => { self.mark_struct_as_constructed(struct_type.clone()); for generic in generics { self.mark_type_as_used(generic); @@ -1507,7 +1507,7 @@ impl<'context> Elaborator<'context> { let function_ids = functions.function_ids(); - if let Type::Struct(struct_type, _) = &self_type { + if let Type::DataType(struct_type, _) = &self_type { let struct_ref = struct_type.borrow(); // `impl`s are only allowed on types defined within the current crate @@ -1602,7 +1602,7 @@ impl<'context> Elaborator<'context> { } /// Find the struct in the parent module so we can know its visibility - fn find_struct_visibility(&self, struct_type: &StructType) -> Option { + fn find_struct_visibility(&self, struct_type: &DataType) -> Option { let parent_module_id = struct_type.id.parent_module_id(self.def_maps); let parent_module_data = self.get_module(parent_module_id); let per_ns = parent_module_data.find_name(&struct_type.name); @@ -1625,7 +1625,7 @@ impl<'context> Elaborator<'context> { } // Public struct functions should not expose private types. if let Some(struct_visibility) = func_meta.struct_id.and_then(|id| { - let struct_def = self.get_struct(id); + let struct_def = self.get_type(id); let struct_def = struct_def.borrow(); self.find_struct_visibility(&struct_def) }) { @@ -1644,7 +1644,7 @@ impl<'context> Elaborator<'context> { span: Span, ) { match typ { - Type::Struct(struct_type, generics) => { + Type::DataType(struct_type, generics) => { let struct_type = struct_type.borrow(); let struct_module_id = struct_type.id.module_id(); @@ -1714,7 +1714,7 @@ impl<'context> Elaborator<'context> { } } - fn collect_struct_definitions(&mut self, structs: &BTreeMap) { + fn collect_struct_definitions(&mut self, structs: &BTreeMap) { // This is necessary to avoid cloning the entire struct map // when adding checks after each struct field is resolved. let struct_ids = structs.keys().copied().collect::>(); @@ -1766,7 +1766,7 @@ impl<'context> Elaborator<'context> { // We need to check after all structs are resolved to // make sure every struct's fields is accurately set. for id in struct_ids { - let struct_type = self.interner.get_struct(id); + let struct_type = self.interner.get_type(id); // Only handle structs without generics as any generics args will be checked // after monomorphization when performing SSA codegen @@ -1786,14 +1786,14 @@ impl<'context> Elaborator<'context> { pub fn resolve_struct_fields( &mut self, unresolved: &NoirStruct, - struct_id: StructId, + struct_id: TypeId, ) -> Vec { self.recover_generics(|this| { this.current_item = Some(DependencyId::Struct(struct_id)); this.resolving_ids.insert(struct_id); - let struct_def = this.interner.get_struct(struct_id); + let struct_def = this.interner.get_type(struct_id); this.add_existing_generics(&unresolved.generics, &struct_def.borrow().generics); let fields = vecmap(&unresolved.fields, |field| { diff --git a/compiler/noirc_frontend/src/elaborator/path_resolution.rs b/compiler/noirc_frontend/src/elaborator/path_resolution.rs index 120ead52883..bae26535e01 100644 --- a/compiler/noirc_frontend/src/elaborator/path_resolution.rs +++ b/compiler/noirc_frontend/src/elaborator/path_resolution.rs @@ -9,7 +9,7 @@ use crate::hir::resolution::errors::ResolverError; use crate::hir::resolution::visibility::item_in_module_is_visible; use crate::locations::ReferencesTracker; -use crate::node_interner::{FuncId, GlobalId, StructId, TraitId, TypeAliasId}; +use crate::node_interner::{FuncId, GlobalId, TraitId, TypeAliasId, TypeId}; use crate::{Shared, Type, TypeAlias}; use super::types::SELF_TYPE_NAME; @@ -27,12 +27,12 @@ pub(crate) struct PathResolution { #[derive(Debug, Clone)] pub enum PathResolutionItem { Module(ModuleId), - Struct(StructId), + Type(TypeId), TypeAlias(TypeAliasId), Trait(TraitId), Global(GlobalId), ModuleFunction(FuncId), - StructFunction(StructId, Option, FuncId), + Method(TypeId, Option, FuncId), TypeAliasFunction(TypeAliasId, Option, FuncId), TraitFunction(TraitId, Option, FuncId), } @@ -41,7 +41,7 @@ impl PathResolutionItem { pub fn function_id(&self) -> Option { match self { PathResolutionItem::ModuleFunction(func_id) - | PathResolutionItem::StructFunction(_, _, func_id) + | PathResolutionItem::Method(_, _, func_id) | PathResolutionItem::TypeAliasFunction(_, _, func_id) | PathResolutionItem::TraitFunction(_, _, func_id) => Some(*func_id), _ => None, @@ -58,12 +58,12 @@ impl PathResolutionItem { pub fn description(&self) -> &'static str { match self { PathResolutionItem::Module(..) => "module", - PathResolutionItem::Struct(..) => "type", + PathResolutionItem::Type(..) => "type", PathResolutionItem::TypeAlias(..) => "type alias", PathResolutionItem::Trait(..) => "trait", PathResolutionItem::Global(..) => "global", PathResolutionItem::ModuleFunction(..) - | PathResolutionItem::StructFunction(..) + | PathResolutionItem::Method(..) | PathResolutionItem::TypeAliasFunction(..) | PathResolutionItem::TraitFunction(..) => "function", } @@ -80,19 +80,19 @@ pub struct Turbofish { #[derive(Debug)] enum IntermediatePathResolutionItem { Module, - Struct(StructId, Option), + Type(TypeId, Option), TypeAlias(TypeAliasId, Option), Trait(TraitId, Option), } pub(crate) type PathResolutionResult = Result; -enum StructMethodLookupResult { +enum MethodLookupResult { /// The method could not be found. There might be trait methods that could be imported, /// but none of them are. NotFound(Vec), - /// Found a struct method. - FoundStructMethod(PerNs), + /// Found a method. + FoundMethod(PerNs), /// Found a trait method and it's currently in scope. FoundTraitMethod(PerNs, TraitId), /// There's only one trait method that matches, but it's not in scope @@ -124,16 +124,16 @@ impl<'context> Elaborator<'context> { let mut module_id = self.module_id(); if path.kind == PathKind::Plain && path.first_name() == Some(SELF_TYPE_NAME) { - if let Some(Type::Struct(struct_type, _)) = &self.self_type { - let struct_type = struct_type.borrow(); + if let Some(Type::DataType(datatype, _)) = &self.self_type { + let datatype = datatype.borrow(); if path.segments.len() == 1 { return Ok(PathResolution { - item: PathResolutionItem::Struct(struct_type.id), + item: PathResolutionItem::Type(datatype.id), errors: Vec::new(), }); } - module_id = struct_type.id.module_id(); + module_id = datatype.id.module_id(); path.segments.remove(0); } } @@ -211,9 +211,9 @@ impl<'context> Elaborator<'context> { last_segment.ident.is_self_type_name(), ); - let current_module_id_is_struct; + let current_module_id_is_type; - (current_module_id, current_module_id_is_struct, intermediate_item) = match typ { + (current_module_id, current_module_id_is_type, intermediate_item) = match typ { ModuleDefId::ModuleId(id) => { if last_segment_generics.is_some() { errors.push(PathResolutionError::TurbofishNotAllowedOnItem { @@ -227,7 +227,7 @@ impl<'context> Elaborator<'context> { ModuleDefId::TypeId(id) => ( id.module_id(), true, - IntermediatePathResolutionItem::Struct(id, last_segment.turbofish()), + IntermediatePathResolutionItem::Type(id, last_segment.turbofish()), ), ModuleDefId::TypeAliasId(id) => { let type_alias = self.interner.get_type_alias(id); @@ -266,10 +266,9 @@ impl<'context> Elaborator<'context> { current_module = self.get_module(current_module_id); // Check if namespace - let found_ns = if current_module_id_is_struct { - match self.resolve_struct_function(importing_module, current_module, current_ident) - { - StructMethodLookupResult::NotFound(vec) => { + let found_ns = if current_module_id_is_type { + match self.resolve_method(importing_module, current_module, current_ident) { + MethodLookupResult::NotFound(vec) => { if vec.is_empty() { return Err(PathResolutionError::Unresolved(current_ident.clone())); } else { @@ -285,16 +284,13 @@ impl<'context> Elaborator<'context> { ); } } - StructMethodLookupResult::FoundStructMethod(per_ns) => per_ns, - StructMethodLookupResult::FoundTraitMethod(per_ns, trait_id) => { + MethodLookupResult::FoundMethod(per_ns) => per_ns, + MethodLookupResult::FoundTraitMethod(per_ns, trait_id) => { let trait_ = self.interner.get_trait(trait_id); self.usage_tracker.mark_as_used(importing_module, &trait_.name); per_ns } - StructMethodLookupResult::FoundOneTraitMethodButNotInScope( - per_ns, - trait_id, - ) => { + MethodLookupResult::FoundOneTraitMethodButNotInScope(per_ns, trait_id) => { let trait_ = self.interner.get_trait(trait_id); let trait_name = self.fully_qualified_trait_path(trait_); errors.push(PathResolutionError::TraitMethodNotInScope { @@ -303,7 +299,7 @@ impl<'context> Elaborator<'context> { }); per_ns } - StructMethodLookupResult::FoundMultipleTraitMethods(vec) => { + MethodLookupResult::FoundMultipleTraitMethods(vec) => { let traits = vecmap(vec, |trait_id| { let trait_ = self.interner.get_trait(trait_id); self.usage_tracker.mark_as_used(importing_module, &trait_.name); @@ -355,32 +351,29 @@ impl<'context> Elaborator<'context> { } fn self_type_module_id(&self) -> Option { - if let Some(Type::Struct(struct_type, _)) = &self.self_type { - Some(struct_type.borrow().id.module_id()) + if let Some(Type::DataType(datatype, _)) = &self.self_type { + Some(datatype.borrow().id.module_id()) } else { None } } - fn resolve_struct_function( + fn resolve_method( &self, importing_module_id: ModuleId, current_module: &ModuleData, ident: &Ident, - ) -> StructMethodLookupResult { - // If the current module is a struct, next we need to find a function for it. - // The function could be in the struct itself, or it could be defined in traits. + ) -> MethodLookupResult { + // If the current module is a type, next we need to find a function for it. + // The function could be in the type itself, or it could be defined in traits. let item_scope = current_module.scope(); let Some(values) = item_scope.values().get(ident) else { - return StructMethodLookupResult::NotFound(vec![]); + return MethodLookupResult::NotFound(vec![]); }; - // First search if the function is defined in the struct itself + // First search if the function is defined in the type itself if let Some(item) = values.get(&None) { - return StructMethodLookupResult::FoundStructMethod(PerNs { - types: None, - values: Some(*item), - }); + return MethodLookupResult::FoundMethod(PerNs { types: None, values: Some(*item) }); } // Otherwise, the function could be defined in zero, one or more traits. @@ -409,25 +402,23 @@ impl<'context> Elaborator<'context> { let (trait_id, item) = values.iter().next().expect("Expected an item"); let trait_id = trait_id.expect("The None option was already considered before"); let per_ns = PerNs { types: None, values: Some(*item) }; - return StructMethodLookupResult::FoundOneTraitMethodButNotInScope( - per_ns, trait_id, - ); + return MethodLookupResult::FoundOneTraitMethodButNotInScope(per_ns, trait_id); } else { let trait_ids = vecmap(values, |(trait_id, _)| { trait_id.expect("The none option was already considered before") }); - return StructMethodLookupResult::NotFound(trait_ids); + return MethodLookupResult::NotFound(trait_ids); } } if results.len() > 1 { let trait_ids = vecmap(results, |(trait_id, _)| trait_id); - return StructMethodLookupResult::FoundMultipleTraitMethods(trait_ids); + return MethodLookupResult::FoundMultipleTraitMethods(trait_ids); } let (trait_id, item) = results.remove(0); let per_ns = PerNs { types: None, values: Some(*item) }; - StructMethodLookupResult::FoundTraitMethod(per_ns, trait_id) + MethodLookupResult::FoundTraitMethod(per_ns, trait_id) } } @@ -437,14 +428,14 @@ fn merge_intermediate_path_resolution_item_with_module_def_id( ) -> PathResolutionItem { match module_def_id { ModuleDefId::ModuleId(module_id) => PathResolutionItem::Module(module_id), - ModuleDefId::TypeId(struct_id) => PathResolutionItem::Struct(struct_id), + ModuleDefId::TypeId(type_id) => PathResolutionItem::Type(type_id), ModuleDefId::TypeAliasId(type_alias_id) => PathResolutionItem::TypeAlias(type_alias_id), ModuleDefId::TraitId(trait_id) => PathResolutionItem::Trait(trait_id), ModuleDefId::GlobalId(global_id) => PathResolutionItem::Global(global_id), ModuleDefId::FunctionId(func_id) => match intermediate_item { IntermediatePathResolutionItem::Module => PathResolutionItem::ModuleFunction(func_id), - IntermediatePathResolutionItem::Struct(struct_id, generics) => { - PathResolutionItem::StructFunction(struct_id, generics, func_id) + IntermediatePathResolutionItem::Type(type_id, generics) => { + PathResolutionItem::Method(type_id, generics, func_id) } IntermediatePathResolutionItem::TypeAlias(alias_id, generics) => { PathResolutionItem::TypeAliasFunction(alias_id, generics, func_id) @@ -460,13 +451,13 @@ fn get_type_alias_module_def_id(type_alias: &Shared) -> Option Some(struct_id.borrow().id.module_id()), + Type::DataType(type_id, _generics) => Some(type_id.borrow().id.module_id()), Type::Alias(type_alias, _generics) => get_type_alias_module_def_id(type_alias), Type::Error => None, _ => { - // For now we only allow type aliases that point to structs. + // For now we only allow type aliases that point to data types. // The more general case is captured here: https://github.com/noir-lang/noir/issues/6398 - panic!("Type alias in path not pointing to struct not yet supported") + panic!("Type alias in path not pointing to a data type is not yet supported") } } } diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 6a672866d7e..6ab12d1e537 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -17,7 +17,7 @@ use crate::{ stmt::HirPattern, }, node_interner::{DefinitionId, DefinitionKind, ExprId, FuncId, GlobalId, TraitImplKind}, - Kind, Shared, StructType, Type, TypeAlias, TypeBindings, + DataType, Kind, Shared, Type, TypeAlias, TypeBindings, }; use super::{path_resolution::PathResolutionItem, Elaborator, ResolverMeta}; @@ -192,7 +192,7 @@ impl<'context> Elaborator<'context> { }; let (struct_type, generics) = match self.lookup_type_or_error(name) { - Some(Type::Struct(struct_type, struct_generics)) => (struct_type, struct_generics), + Some(Type::DataType(struct_type, struct_generics)) => (struct_type, struct_generics), None => return error_identifier(self), Some(typ) => { let typ = typ.to_string(); @@ -210,7 +210,7 @@ impl<'context> Elaborator<'context> { turbofish_span, ); - let actual_type = Type::Struct(struct_type.clone(), generics); + let actual_type = Type::DataType(struct_type.clone(), generics); let location = Location::new(span, self.file); self.unify(&actual_type, &expected_type, || TypeCheckError::TypeMismatchWithSource { @@ -250,7 +250,7 @@ impl<'context> Elaborator<'context> { #[allow(clippy::too_many_arguments)] fn resolve_constructor_pattern_fields( &mut self, - struct_type: Shared, + struct_type: Shared, fields: Vec<(Ident, Pattern)>, span: Span, expected_type: Type, @@ -434,7 +434,7 @@ impl<'context> Elaborator<'context> { pub(super) fn resolve_struct_turbofish_generics( &mut self, - struct_type: &StructType, + struct_type: &DataType, generics: Vec, unresolved_turbofish: Option>, span: Span, @@ -574,8 +574,8 @@ impl<'context> Elaborator<'context> { /// solve these fn resolve_item_turbofish(&mut self, item: PathResolutionItem) -> Vec { match item { - PathResolutionItem::StructFunction(struct_id, Some(generics), _func_id) => { - let struct_type = self.interner.get_struct(struct_id); + PathResolutionItem::Method(struct_id, Some(generics), _func_id) => { + let struct_type = self.interner.get_type(struct_id); let struct_type = struct_type.borrow(); let struct_generics = struct_type.instantiate(self.interner); self.resolve_struct_turbofish_generics( @@ -886,7 +886,7 @@ impl<'context> Elaborator<'context> { fn get_type_alias_generics(type_alias: &TypeAlias, generics: &[Type]) -> Vec { let typ = type_alias.get_type(generics); match typ { - Type::Struct(_, generics) => generics, + Type::DataType(_, generics) => generics, Type::Alias(type_alias, generics) => { get_type_alias_generics(&type_alias.borrow(), &generics) } diff --git a/compiler/noirc_frontend/src/elaborator/scope.rs b/compiler/noirc_frontend/src/elaborator/scope.rs index fe01e3cb7f3..327ae02b204 100644 --- a/compiler/noirc_frontend/src/elaborator/scope.rs +++ b/compiler/noirc_frontend/src/elaborator/scope.rs @@ -10,8 +10,8 @@ use crate::{ expr::{HirCapturedVar, HirIdent}, traits::Trait, }, - node_interner::{DefinitionId, StructId, TraitId}, - Shared, StructType, + node_interner::{DefinitionId, TraitId, TypeId}, + DataType, Shared, }; use crate::{Type, TypeAlias}; @@ -37,8 +37,8 @@ impl<'context> Elaborator<'context> { current_module } - pub(super) fn get_struct(&self, type_id: StructId) -> Shared { - self.interner.get_struct(type_id) + pub(super) fn get_type(&self, type_id: TypeId) -> Shared { + self.interner.get_type(type_id) } pub(super) fn get_trait_mut(&mut self, trait_id: TraitId) -> &mut Trait { @@ -160,12 +160,12 @@ impl<'context> Elaborator<'context> { } /// Lookup a given struct type by name. - pub fn lookup_struct_or_error(&mut self, path: Path) -> Option> { + pub fn lookup_datatype_or_error(&mut self, path: Path) -> Option> { let span = path.span(); match self.resolve_path_or_error(path) { Ok(item) => { - if let PathResolutionItem::Struct(struct_id) = item { - Some(self.get_struct(struct_id)) + if let PathResolutionItem::Type(struct_id) = item { + Some(self.get_type(struct_id)) } else { self.push_err(ResolverError::Expected { expected: "type", @@ -194,10 +194,10 @@ impl<'context> Elaborator<'context> { let span = path.span; match self.resolve_path_or_error(path) { - Ok(PathResolutionItem::Struct(struct_id)) => { - let struct_type = self.get_struct(struct_id); + Ok(PathResolutionItem::Type(struct_id)) => { + let struct_type = self.get_type(struct_id); let generics = struct_type.borrow().instantiate(self.interner); - Some(Type::Struct(struct_type, generics)) + Some(Type::DataType(struct_type, generics)) } Ok(PathResolutionItem::TypeAlias(alias_id)) => { let alias = self.interner.get_type_alias(alias_id); diff --git a/compiler/noirc_frontend/src/elaborator/statements.rs b/compiler/noirc_frontend/src/elaborator/statements.rs index 150c6e7de48..a95e260b6a5 100644 --- a/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/compiler/noirc_frontend/src/elaborator/statements.rs @@ -21,7 +21,7 @@ use crate::{ }, }, node_interner::{DefinitionId, DefinitionKind, GlobalId, StmtId}, - StructType, Type, + DataType, Type, }; use super::{lints, Elaborator, Loop}; @@ -491,7 +491,7 @@ impl<'context> Elaborator<'context> { let lhs_type = lhs_type.follow_bindings(); match &lhs_type { - Type::Struct(s, args) => { + Type::DataType(s, args) => { let s = s.borrow(); if let Some((field, visibility, index)) = s.get_field(field_name, args) { let reference_location = Location::new(span, self.file); @@ -555,7 +555,7 @@ impl<'context> Elaborator<'context> { pub(super) fn check_struct_field_visibility( &mut self, - struct_type: &StructType, + struct_type: &DataType, field_name: &str, visibility: ItemVisibility, span: Span, diff --git a/compiler/noirc_frontend/src/elaborator/trait_impls.rs b/compiler/noirc_frontend/src/elaborator/trait_impls.rs index 20f048bed05..aa27ac29fa6 100644 --- a/compiler/noirc_frontend/src/elaborator/trait_impls.rs +++ b/compiler/noirc_frontend/src/elaborator/trait_impls.rs @@ -217,7 +217,7 @@ impl<'context> Elaborator<'context> { self.file = trait_impl.file_id; let object_crate = match &trait_impl.resolved_object_type { - Some(Type::Struct(struct_type, _)) => struct_type.borrow().id.krate(), + Some(Type::DataType(struct_type, _)) => struct_type.borrow().id.krate(), _ => CrateId::Dummy, }; diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 36427997843..7fc403ebf23 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -154,12 +154,12 @@ impl<'context> Elaborator<'context> { let location = Location::new(named_path_span.unwrap_or(typ.span), self.file); match resolved_type { - Type::Struct(ref struct_type, _) => { + Type::DataType(ref data_type, _) => { // Record the location of the type reference self.interner.push_type_ref_location(resolved_type.clone(), location); if !is_synthetic { self.interner.add_struct_reference( - struct_type.borrow().id, + data_type.borrow().id, location, is_self_type_name, ); @@ -259,11 +259,11 @@ impl<'context> Elaborator<'context> { return Type::Alias(type_alias, args); } - match self.lookup_struct_or_error(path) { - Some(struct_type) => { - if self.resolving_ids.contains(&struct_type.borrow().id) { - self.push_err(ResolverError::SelfReferentialStruct { - span: struct_type.borrow().name.span(), + match self.lookup_datatype_or_error(path) { + Some(data_type) => { + if self.resolving_ids.contains(&data_type.borrow().id) { + self.push_err(ResolverError::SelfReferentialType { + span: data_type.borrow().name.span(), }); return Type::Error; @@ -272,23 +272,23 @@ impl<'context> Elaborator<'context> { if !self.in_contract() && self .interner - .struct_attributes(&struct_type.borrow().id) + .struct_attributes(&data_type.borrow().id) .iter() .any(|attr| matches!(attr, SecondaryAttribute::Abi(_))) { self.push_err(ResolverError::AbiAttributeOutsideContract { - span: struct_type.borrow().name.span(), + span: data_type.borrow().name.span(), }); } - let (args, _) = self.resolve_type_args(args, struct_type.borrow(), span); + let (args, _) = self.resolve_type_args(args, data_type.borrow(), span); if let Some(current_item) = self.current_item { - let dependency_id = struct_type.borrow().id; + let dependency_id = data_type.borrow().id; self.interner.add_type_dependency(current_item, dependency_id); } - Type::Struct(struct_type, args) + Type::DataType(data_type, args) } None => Type::Error, } @@ -684,8 +684,8 @@ impl<'context> Elaborator<'context> { None } - /// This resolves a method in the form `Struct::method` where `method` is a trait method - fn resolve_struct_trait_method(&mut self, path: &Path) -> Option { + /// This resolves a method in the form `Type::method` where `method` is a trait method + fn resolve_type_trait_method(&mut self, path: &Path) -> Option { if path.segments.len() < 2 { return None; } @@ -696,16 +696,16 @@ impl<'context> Elaborator<'context> { let before_last_segment = path.last_segment(); let path_resolution = self.resolve_path(path).ok()?; - let PathResolutionItem::Struct(struct_id) = path_resolution.item else { + let PathResolutionItem::Type(type_id) = path_resolution.item else { return None; }; - let struct_type = self.get_struct(struct_id); - let generics = struct_type.borrow().instantiate(self.interner); - let typ = Type::Struct(struct_type, generics); + let datatype = self.get_type(type_id); + let generics = datatype.borrow().instantiate(self.interner); + let typ = Type::DataType(datatype, generics); let method_name = &last_segment.ident.0.contents; - // If we can find a method on the struct, this is definitely not a trait method + // If we can find a method on the type, this is definitely not a trait method if self.interner.lookup_direct_method(&typ, method_name, false).is_some() { return None; } @@ -749,7 +749,7 @@ impl<'context> Elaborator<'context> { self.resolve_trait_static_method_by_self(path) .or_else(|| self.resolve_trait_static_method(path)) .or_else(|| self.resolve_trait_method_by_named_generic(path)) - .or_else(|| self.resolve_struct_trait_method(path)) + .or_else(|| self.resolve_type_trait_method(path)) } pub(super) fn unify( @@ -1423,12 +1423,12 @@ impl<'context> Elaborator<'context> { self.lookup_method_in_trait_constraints(object_type, method_name, span) } // Mutable references to another type should resolve to methods of their element type. - // This may be a struct or a primitive type. + // This may be a data type or a primitive type. Type::MutableReference(element) => { self.lookup_method(&element, method_name, span, has_self_arg) } - // If we fail to resolve the object to a struct type, we have no way of type + // If we fail to resolve the object to a data type, we have no way of type // checking its arguments as we can't even resolve the name of the function Type::Error => None, @@ -1438,13 +1438,11 @@ impl<'context> Elaborator<'context> { None } - other => { - self.lookup_struct_or_primitive_method(&other, method_name, span, has_self_arg) - } + other => self.lookup_type_or_primitive_method(&other, method_name, span, has_self_arg), } } - fn lookup_struct_or_primitive_method( + fn lookup_type_or_primitive_method( &mut self, object_type: &Type, method_name: &str, @@ -1475,12 +1473,16 @@ impl<'context> Elaborator<'context> { return self.return_trait_method_in_scope(&generic_methods, method_name, span); } - if let Type::Struct(struct_type, _) = object_type { - let has_field_with_function_type = struct_type - .borrow() - .get_fields_as_written() - .into_iter() - .any(|field| field.name.0.contents == method_name && field.typ.is_function()); + if let Type::DataType(datatype, _) = object_type { + let datatype = datatype.borrow(); + let mut has_field_with_function_type = false; + + if let Some(fields) = datatype.try_fields_raw() { + has_field_with_function_type = fields + .iter() + .any(|field| field.name.0.contents == method_name && field.typ.is_function()); + } + if has_field_with_function_type { self.push_err(TypeCheckError::CannotInvokeStructFieldFunctionType { method_name: method_name.to_string(), diff --git a/compiler/noirc_frontend/src/hir/comptime/display.rs b/compiler/noirc_frontend/src/hir/comptime/display.rs index 39bcb87e145..cbcf8b02d03 100644 --- a/compiler/noirc_frontend/src/hir/comptime/display.rs +++ b/compiler/noirc_frontend/src/hir/comptime/display.rs @@ -357,7 +357,7 @@ impl<'value, 'interner> Display for ValuePrinter<'value, 'interner> { } Value::Struct(fields, typ) => { let typename = match typ.follow_bindings() { - Type::Struct(def, _) => def.borrow().name.to_string(), + Type::DataType(def, _) => def.borrow().name.to_string(), other => other.to_string(), }; let fields = vecmap(fields, |(name, value)| { @@ -376,7 +376,7 @@ impl<'value, 'interner> Display for ValuePrinter<'value, 'interner> { } Value::Quoted(tokens) => display_quoted(tokens, 0, self.interner, f), Value::StructDefinition(id) => { - let def = self.interner.get_struct(*id); + let def = self.interner.get_type(*id); let def = def.borrow(); write!(f, "{}", def.name) } diff --git a/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs b/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs index 0257ff66ac6..806360c3683 100644 --- a/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs +++ b/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs @@ -246,7 +246,7 @@ impl HirPattern { (name.clone(), pattern.to_display_ast(interner)) }); let name = match typ.follow_bindings() { - Type::Struct(struct_def, _) => { + Type::DataType(struct_def, _) => { let struct_def = struct_def.borrow(); struct_def.name.0.contents.clone() } @@ -301,7 +301,7 @@ impl Type { let fields = vecmap(fields, |field| field.to_display_ast()); UnresolvedTypeData::Tuple(fields) } - Type::Struct(def, generics) => { + Type::DataType(def, generics) => { let struct_def = def.borrow(); let ordered_args = vecmap(generics, |generic| generic.to_display_ast()); let generics = diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index 3506b63919c..3f6b10a0176 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -403,7 +403,7 @@ fn struct_def_add_generic( }; let struct_id = get_struct(self_argument)?; - let the_struct = interner.get_struct(struct_id); + let the_struct = interner.get_type(struct_id); let mut the_struct = the_struct.borrow_mut(); let name = Rc::new(generic_name); @@ -436,7 +436,7 @@ fn struct_def_as_type( ) -> IResult { let argument = check_one_argument(arguments, location)?; let struct_id = get_struct(argument)?; - let struct_def_rc = interner.get_struct(struct_id); + let struct_def_rc = interner.get_type(struct_id); let struct_def = struct_def_rc.borrow(); let generics = vecmap(&struct_def.generics, |generic| { @@ -444,7 +444,7 @@ fn struct_def_as_type( }); drop(struct_def); - Ok(Value::Type(Type::Struct(struct_def_rc, generics))) + Ok(Value::Type(Type::DataType(struct_def_rc, generics))) } /// fn generics(self) -> [(Type, Option)] @@ -456,7 +456,7 @@ fn struct_def_generics( ) -> IResult { let argument = check_one_argument(arguments, location)?; let struct_id = get_struct(argument)?; - let struct_def = interner.get_struct(struct_id); + let struct_def = interner.get_type(struct_id); let struct_def = struct_def.borrow(); let expected = Type::Slice(Box::new(Type::Tuple(vec![ @@ -526,7 +526,7 @@ fn struct_def_fields( ) -> IResult { let (typ, generic_args) = check_two_arguments(arguments, location)?; let struct_id = get_struct(typ)?; - let struct_def = interner.get_struct(struct_id); + let struct_def = interner.get_type(struct_id); let struct_def = struct_def.borrow(); let args_location = generic_args.1; @@ -569,7 +569,7 @@ fn struct_def_fields_as_written( ) -> IResult { let argument = check_one_argument(arguments, location)?; let struct_id = get_struct(argument)?; - let struct_def = interner.get_struct(struct_id); + let struct_def = interner.get_type(struct_id); let struct_def = struct_def.borrow(); let mut fields = im::Vector::new(); @@ -607,7 +607,7 @@ fn struct_def_name( ) -> IResult { let self_argument = check_one_argument(arguments, location)?; let struct_id = get_struct(self_argument)?; - let the_struct = interner.get_struct(struct_id); + let the_struct = interner.get_type(struct_id); let name = Token::Ident(the_struct.borrow().name.to_string()); Ok(Value::Quoted(Rc::new(vec![name]))) @@ -623,7 +623,7 @@ fn struct_def_set_fields( let (the_struct, fields) = check_two_arguments(arguments, location)?; let struct_id = get_struct(the_struct)?; - let struct_def = interner.get_struct(struct_id); + let struct_def = interner.get_type(struct_id); let mut struct_def = struct_def.borrow_mut(); let field_location = fields.1; @@ -1057,7 +1057,7 @@ fn type_as_struct( location: Location, ) -> IResult { type_as(arguments, return_type, location, |typ| { - if let Type::Struct(struct_type, generics) = typ { + if let Type::DataType(struct_type, generics) = typ { Some(Value::Tuple(vec![ Value::StructDefinition(struct_type.borrow().id), Value::Slice( @@ -1432,7 +1432,7 @@ fn zeroed(return_type: Type, span: Span) -> IResult { } Type::Unit => Ok(Value::Unit), Type::Tuple(fields) => Ok(Value::Tuple(try_vecmap(fields, |field| zeroed(field, span))?)), - Type::Struct(struct_type, generics) => { + Type::DataType(struct_type, generics) => { let fields = struct_type.borrow().get_fields(&generics); let mut values = HashMap::default(); @@ -1441,7 +1441,7 @@ fn zeroed(return_type: Type, span: Span) -> IResult { values.insert(Rc::new(field_name), field_value); } - let typ = Type::Struct(struct_type, generics); + let typ = Type::DataType(struct_type, generics); Ok(Value::Struct(values, typ)) } Type::Alias(alias, generics) => zeroed(alias.borrow().get_type(&generics), span), @@ -2890,7 +2890,7 @@ pub(crate) fn option(option_type: Type, value: Option, span: Span) -> IRe /// Given a type, assert that it's an Option and return the Type for T pub(crate) fn extract_option_generic_type(typ: Type) -> Type { - let Type::Struct(struct_type, mut generics) = typ else { + let Type::DataType(struct_type, mut generics) = typ else { panic!("Expected type to be a struct"); }; diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs index a3f84a00bfb..342f494023d 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs @@ -28,11 +28,11 @@ use crate::{ function::{FuncMeta, FunctionBody}, stmt::HirPattern, }, - node_interner::{FuncId, NodeInterner, StructId, TraitId, TraitImplId}, + node_interner::{FuncId, NodeInterner, TraitId, TraitImplId, TypeId}, token::{SecondaryAttribute, Token, Tokens}, QuotedType, Type, }; -use crate::{Kind, Shared, StructType}; +use crate::{DataType, Kind, Shared}; use rustc_hash::FxHashMap as HashMap; pub(crate) fn check_argument_count( @@ -108,14 +108,13 @@ pub(crate) fn get_struct_fields( match value { Value::Struct(fields, typ) => Ok((fields, typ)), _ => { - let expected = StructType::new( - StructId::dummy_id(), + let expected = DataType::new( + TypeId::dummy_id(), Ident::new(name.to_string(), location.span), location, Vec::new(), - Vec::new(), ); - let expected = Type::Struct(Shared::new(expected), Vec::new()); + let expected = Type::DataType(Shared::new(expected), Vec::new()); type_mismatch(value, expected, location) } } @@ -327,7 +326,7 @@ pub(crate) fn get_module((value, location): (Value, Location)) -> IResult IResult { +pub(crate) fn get_struct((value, location): (Value, Location)) -> IResult { match value { Value::StructDefinition(id) => Ok(id), _ => type_mismatch(value, Type::Quoted(QuotedType::StructDefinition), location), @@ -434,7 +433,7 @@ fn gather_hir_pattern_tokens( tokens.push(Token::RightParen); } HirPattern::Struct(typ, fields, _) => { - let Type::Struct(struct_type, _) = typ.follow_bindings() else { + let Type::DataType(struct_type, _) = typ.follow_bindings() else { panic!("Expected type to be a struct"); }; diff --git a/compiler/noirc_frontend/src/hir/comptime/value.rs b/compiler/noirc_frontend/src/hir/comptime/value.rs index 77933ba9361..c5ec7d861cd 100644 --- a/compiler/noirc_frontend/src/hir/comptime/value.rs +++ b/compiler/noirc_frontend/src/hir/comptime/value.rs @@ -18,7 +18,7 @@ use crate::{ HirArrayLiteral, HirConstructorExpression, HirExpression, HirIdent, HirLambda, HirLiteral, ImplKind, }, - node_interner::{ExprId, FuncId, NodeInterner, StmtId, StructId, TraitId, TraitImplId}, + node_interner::{ExprId, FuncId, NodeInterner, StmtId, TraitId, TraitImplId, TypeId}, parser::{Item, Parser}, token::{SpannedToken, Token, Tokens}, Kind, QuotedType, Shared, Type, TypeBindings, @@ -62,7 +62,7 @@ pub enum Value { /// tokens can cause larger spans to be before lesser spans, causing an assert. They may also /// be inserted into separate files entirely. Quoted(Rc>), - StructDefinition(StructId), + StructDefinition(TypeId), TraitConstraint(TraitId, TraitGenerics), TraitDefinition(TraitId), TraitImpl(TraitImplId), @@ -234,7 +234,7 @@ impl Value { })?; let struct_type = match typ.follow_bindings() { - Type::Struct(def, _) => Some(def.borrow().id), + Type::DataType(def, _) => Some(def.borrow().id), _ => return Err(InterpreterError::NonStructInConstructor { typ, location }), }; @@ -388,7 +388,7 @@ impl Value { })?; let (r#type, struct_generics) = match typ.follow_bindings() { - Type::Struct(def, generics) => (def, generics), + Type::DataType(def, generics) => (def, generics), _ => return Err(InterpreterError::NonStructInConstructor { typ, location }), }; diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 10866f4b309..9aad806bb3c 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -16,8 +16,8 @@ use crate::hir::Context; use crate::ast::{Expression, NoirEnumeration}; use crate::node_interner::{ - FuncId, GlobalId, ModuleAttributes, NodeInterner, ReferenceId, StructId, TraitId, TraitImplId, - TypeAliasId, + FuncId, GlobalId, ModuleAttributes, NodeInterner, ReferenceId, TraitId, TraitImplId, + TypeAliasId, TypeId, }; use crate::ast::{ @@ -147,8 +147,8 @@ pub struct DefCollector { #[derive(Default)] pub struct CollectedItems { pub functions: Vec, - pub(crate) structs: BTreeMap, - pub(crate) enums: BTreeMap, + pub(crate) structs: BTreeMap, + pub(crate) enums: BTreeMap, pub(crate) type_aliases: BTreeMap, pub(crate) traits: BTreeMap, pub globals: Vec, diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 41234980942..b3047d66989 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -17,7 +17,7 @@ use crate::ast::{ UnresolvedTypeData, }; use crate::hir::resolution::errors::ResolverError; -use crate::node_interner::{ModuleAttributes, NodeInterner, ReferenceId, StructId}; +use crate::node_interner::{ModuleAttributes, NodeInterner, ReferenceId, TypeId}; use crate::token::SecondaryAttribute; use crate::usage_tracker::{UnusedItem, UsageTracker}; use crate::{ @@ -819,7 +819,7 @@ impl<'a> ModCollector<'a> { inner_attributes: Vec, add_to_parent_scope: bool, is_contract: bool, - is_struct: bool, + is_type: bool, ) -> Result { push_child_module( &mut context.def_interner, @@ -832,7 +832,7 @@ impl<'a> ModCollector<'a> { inner_attributes, add_to_parent_scope, is_contract, - is_struct, + is_type, ) } @@ -868,7 +868,7 @@ fn push_child_module( inner_attributes: Vec, add_to_parent_scope: bool, is_contract: bool, - is_struct: bool, + is_type: bool, ) -> Result { // Note: the difference between `location` and `mod_location` is: // - `mod_location` will point to either the token "foo" in `mod foo { ... }` @@ -884,7 +884,7 @@ fn push_child_module( outer_attributes, inner_attributes, is_contract, - is_struct, + is_type, ); let module_id = def_map.modules.insert(new_module); @@ -998,7 +998,7 @@ pub fn collect_struct( module_id: LocalModuleId, krate: CrateId, definition_errors: &mut Vec<(CompilationError, FileId)>, -) -> Option<(StructId, UnresolvedStruct)> { +) -> Option<(TypeId, UnresolvedStruct)> { let doc_comments = struct_definition.doc_comments; let struct_definition = struct_definition.item; @@ -1031,7 +1031,11 @@ pub fn collect_struct( true, // is struct ) { Ok(module_id) => { - interner.new_struct(&unresolved, resolved_generics, krate, module_id.local_id, file_id) + let name = unresolved.struct_def.name.clone(); + let span = unresolved.struct_def.span; + let attributes = unresolved.struct_def.attributes.clone(); + let local_id = module_id.local_id; + interner.new_type(name, span, attributes, resolved_generics, krate, local_id, file_id) } Err(error) => { definition_errors.push((error.into(), file_id)); @@ -1050,7 +1054,7 @@ pub fn collect_struct( // Add the struct to scope so its path can be looked up later let visibility = unresolved.struct_def.visibility; - let result = def_map.modules[module_id.0].declare_struct(name.clone(), visibility, id); + let result = def_map.modules[module_id.0].declare_type(name.clone(), visibility, id); let parent_module_id = ModuleId { krate, local_id: module_id }; @@ -1081,16 +1085,97 @@ pub fn collect_struct( #[allow(clippy::too_many_arguments)] pub fn collect_enum( - _interner: &mut NodeInterner, - _def_map: &mut CrateDefMap, - _usage_tracker: &mut UsageTracker, - _enum_definition: Documented, - _file_id: FileId, - _module_id: LocalModuleId, - _krate: CrateId, - _definition_errors: &mut [(CompilationError, FileId)], -) -> Option<(StructId, UnresolvedEnum)> { - todo!("Implement collect_enum") + interner: &mut NodeInterner, + def_map: &mut CrateDefMap, + usage_tracker: &mut UsageTracker, + enum_def: Documented, + file_id: FileId, + module_id: LocalModuleId, + krate: CrateId, + definition_errors: &mut Vec<(CompilationError, FileId)>, +) -> Option<(TypeId, UnresolvedEnum)> { + let doc_comments = enum_def.doc_comments; + let enum_def = enum_def.item; + + check_duplicate_variant_names(&enum_def, file_id, definition_errors); + + let name = enum_def.name.clone(); + + let unresolved = UnresolvedEnum { file_id, module_id, enum_def }; + + let resolved_generics = Context::resolve_generics( + interner, + &unresolved.enum_def.generics, + definition_errors, + file_id, + ); + + // Create the corresponding module for the enum namespace + let location = Location::new(name.span(), file_id); + let id = match push_child_module( + interner, + def_map, + module_id, + &name, + ItemVisibility::Public, + location, + Vec::new(), + Vec::new(), + false, // add to parent scope + false, // is contract + true, // is type + ) { + Ok(module_id) => { + let name = unresolved.enum_def.name.clone(); + let span = unresolved.enum_def.span; + let attributes = unresolved.enum_def.attributes.clone(); + let local_id = module_id.local_id; + interner.new_type(name, span, attributes, resolved_generics, krate, local_id, file_id) + } + Err(error) => { + definition_errors.push((error.into(), file_id)); + return None; + } + }; + + interner.set_doc_comments(ReferenceId::Enum(id), doc_comments); + + for (index, variant) in unresolved.enum_def.variants.iter().enumerate() { + if !variant.doc_comments.is_empty() { + let id = ReferenceId::EnumVariant(id, index); + interner.set_doc_comments(id, variant.doc_comments.clone()); + } + } + + // Add the enum to scope so its path can be looked up later + let visibility = unresolved.enum_def.visibility; + let result = def_map.modules[module_id.0].declare_type(name.clone(), visibility, id); + + let parent_module_id = ModuleId { krate, local_id: module_id }; + + if !unresolved.enum_def.is_abi() { + usage_tracker.add_unused_item( + parent_module_id, + name.clone(), + UnusedItem::Enum(id), + visibility, + ); + } + + if let Err((first_def, second_def)) = result { + let error = DefCollectorErrorKind::Duplicate { + typ: DuplicateType::TypeDefinition, + first_def, + second_def, + }; + definition_errors.push((error.into(), file_id)); + } + + if interner.is_in_lsp_mode() { + interner.register_enum(id, name.to_string(), visibility, parent_module_id); + } + + Some((id, unresolved)) } pub fn collect_impl( @@ -1333,14 +1418,35 @@ fn check_duplicate_field_names( } let previous_field_name = *seen_field_names.get(field_name).unwrap(); - definition_errors.push(( - DefCollectorErrorKind::DuplicateField { - first_def: previous_field_name.clone(), - second_def: field_name.clone(), - } - .into(), - file, - )); + let error = DefCollectorErrorKind::Duplicate { + typ: DuplicateType::StructField, + first_def: previous_field_name.clone(), + second_def: field_name.clone(), + }; + definition_errors.push((error.into(), file)); + } +} + +fn check_duplicate_variant_names( + enum_def: &NoirEnumeration, + file: FileId, + definition_errors: &mut Vec<(CompilationError, FileId)>, +) { + let mut seen_variant_names = std::collections::HashSet::new(); + for variant in &enum_def.variants { + let variant_name = &variant.item.name; + + if seen_variant_names.insert(variant_name) { + continue; + } + + let previous_variant_name = *seen_variant_names.get(variant_name).unwrap(); + let error = DefCollectorErrorKind::Duplicate { + typ: DuplicateType::EnumVariant, + first_def: previous_variant_name.clone(), + second_def: variant_name.clone(), + }; + definition_errors.push((error.into(), file)); } } diff --git a/compiler/noirc_frontend/src/hir/def_collector/errors.rs b/compiler/noirc_frontend/src/hir/def_collector/errors.rs index 1582e297144..1ca62acd29b 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/errors.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/errors.rs @@ -21,21 +21,21 @@ pub enum DuplicateType { TraitAssociatedType, TraitAssociatedConst, TraitAssociatedFunction, + StructField, + EnumVariant, } #[derive(Error, Debug, Clone)] pub enum DefCollectorErrorKind { - #[error("duplicate {typ} found in namespace")] + #[error("Duplicate {typ}")] Duplicate { typ: DuplicateType, first_def: Ident, second_def: Ident }, - #[error("duplicate struct field {first_def}")] - DuplicateField { first_def: Ident, second_def: Ident }, - #[error("unresolved import")] + #[error("Unresolved import")] UnresolvedModuleDecl { mod_name: Ident, expected_path: String, alternative_path: String }, - #[error("overlapping imports")] + #[error("Overlapping imports")] OverlappingModuleDecls { mod_name: Ident, expected_path: String, alternative_path: String }, - #[error("path resolution error")] + #[error("Path resolution error")] PathResolutionError(PathResolutionError), - #[error("cannot re-export {item_name} because it has less visibility than this use statement")] + #[error("Cannot re-export {item_name} because it has less visibility than this use statement")] CannotReexportItemWithLessVisibility { item_name: Ident, desired_visibility: ItemVisibility }, #[error("Non-struct type used in impl")] NonStructTypeInImpl { span: Span }, @@ -120,6 +120,8 @@ impl fmt::Display for DuplicateType { DuplicateType::TraitAssociatedType => write!(f, "trait associated type"), DuplicateType::TraitAssociatedConst => write!(f, "trait associated constant"), DuplicateType::TraitAssociatedFunction => write!(f, "trait associated function"), + DuplicateType::StructField => write!(f, "struct field"), + DuplicateType::EnumVariant => write!(f, "enum variant"), } } } @@ -144,23 +146,6 @@ impl<'a> From<&'a DefCollectorErrorKind> for Diagnostic { diag } } - DefCollectorErrorKind::DuplicateField { first_def, second_def } => { - let primary_message = format!( - "Duplicate definitions of struct field with name {} found", - &first_def.0.contents - ); - { - let first_span = first_def.0.span(); - let second_span = second_def.0.span(); - let mut diag = Diagnostic::simple_error( - primary_message, - "First definition found here".to_string(), - first_span, - ); - diag.add_secondary("Second definition found here".to_string(), second_span); - diag - } - } DefCollectorErrorKind::UnresolvedModuleDecl { mod_name, expected_path, alternative_path } => { let span = mod_name.0.span(); let mod_name = &mod_name.0.contents; diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index f7fc6ca08ea..3d4049a1738 100644 --- a/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -1,7 +1,7 @@ use crate::graph::{CrateGraph, CrateId}; use crate::hir::def_collector::dc_crate::{CompilationError, DefCollector}; use crate::hir::Context; -use crate::node_interner::{FuncId, GlobalId, NodeInterner, StructId}; +use crate::node_interner::{FuncId, GlobalId, NodeInterner, TypeId}; use crate::parse_program; use crate::parser::{ParsedModule, ParserError}; use crate::token::{FunctionAttribute, SecondaryAttribute, TestScope}; @@ -356,7 +356,7 @@ pub struct ContractFunctionMeta { } pub struct ContractOutputs { - pub structs: HashMap>, + pub structs: HashMap>, pub globals: HashMap>, } diff --git a/compiler/noirc_frontend/src/hir/def_map/module_data.rs b/compiler/noirc_frontend/src/hir/def_map/module_data.rs index 06188f3920b..b048aed214a 100644 --- a/compiler/noirc_frontend/src/hir/def_map/module_data.rs +++ b/compiler/noirc_frontend/src/hir/def_map/module_data.rs @@ -4,7 +4,7 @@ use noirc_errors::Location; use super::{ItemScope, LocalModuleId, ModuleDefId, ModuleId, PerNs}; use crate::ast::{Ident, ItemVisibility}; -use crate::node_interner::{FuncId, GlobalId, StructId, TraitId, TypeAliasId}; +use crate::node_interner::{FuncId, GlobalId, TraitId, TypeAliasId, TypeId}; use crate::token::SecondaryAttribute; /// Contains the actual contents of a module: its parent (if one exists), @@ -120,11 +120,11 @@ impl ModuleData { self.declare(name, visibility, id.into(), None) } - pub fn declare_struct( + pub fn declare_type( &mut self, name: Ident, visibility: ItemVisibility, - id: StructId, + id: TypeId, ) -> Result<(), (Ident, Ident)> { self.declare(name, visibility, ModuleDefId::TypeId(id), None) } diff --git a/compiler/noirc_frontend/src/hir/def_map/module_def.rs b/compiler/noirc_frontend/src/hir/def_map/module_def.rs index a751eacd2dd..40d57ae2e23 100644 --- a/compiler/noirc_frontend/src/hir/def_map/module_def.rs +++ b/compiler/noirc_frontend/src/hir/def_map/module_def.rs @@ -1,4 +1,4 @@ -use crate::node_interner::{FuncId, GlobalId, StructId, TraitId, TypeAliasId}; +use crate::node_interner::{FuncId, GlobalId, TraitId, TypeAliasId, TypeId}; use super::ModuleId; @@ -7,7 +7,7 @@ use super::ModuleId; pub enum ModuleDefId { ModuleId(ModuleId), FunctionId(FuncId), - TypeId(StructId), + TypeId(TypeId), TypeAliasId(TypeAliasId), TraitId(TraitId), GlobalId(GlobalId), @@ -21,7 +21,7 @@ impl ModuleDefId { } } - pub fn as_type(&self) -> Option { + pub fn as_type(&self) -> Option { match self { ModuleDefId::TypeId(type_id) => Some(*type_id), _ => None, diff --git a/compiler/noirc_frontend/src/hir/mod.rs b/compiler/noirc_frontend/src/hir/mod.rs index b231f8c9698..fea52be88bc 100644 --- a/compiler/noirc_frontend/src/hir/mod.rs +++ b/compiler/noirc_frontend/src/hir/mod.rs @@ -9,7 +9,7 @@ use crate::ast::UnresolvedGenerics; use crate::debug::DebugInstrumenter; use crate::graph::{CrateGraph, CrateId}; use crate::hir_def::function::FuncMeta; -use crate::node_interner::{FuncId, NodeInterner, StructId}; +use crate::node_interner::{FuncId, NodeInterner, TypeId}; use crate::parser::ParserError; use crate::usage_tracker::UsageTracker; use crate::{Generics, Kind, ParsedModule, ResolvedGeneric, TypeVariable}; @@ -151,7 +151,7 @@ impl Context<'_, '_> { /// /// For example, if you project contains a `main.nr` and `foo.nr` and you provide the `main_crate_id` and the /// `bar_struct_id` where the `Bar` struct is inside `foo.nr`, this function would return `foo::Bar` as a [String]. - pub fn fully_qualified_struct_path(&self, crate_id: &CrateId, id: StructId) -> String { + pub fn fully_qualified_struct_path(&self, crate_id: &CrateId, id: TypeId) -> String { fully_qualified_module_path(&self.def_maps, &self.crate_graph, crate_id, id.module_id()) } diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs index f8373080069..6298ef796b4 100644 --- a/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -116,8 +116,8 @@ pub enum ResolverError { NonIntegralGlobalType { span: Span, global_value: Value }, #[error("Global value `{global_value}` is larger than its kind's maximum value")] GlobalLargerThanKind { span: Span, global_value: FieldElement, kind: Kind }, - #[error("Self-referential structs are not supported")] - SelfReferentialStruct { span: Span }, + #[error("Self-referential types are not supported")] + SelfReferentialType { span: Span }, #[error("#[no_predicates] attribute is only allowed on constrained functions")] NoPredicatesAttributeOnUnconstrained { ident: Ident }, #[error("#[fold] attribute is only allowed on constrained functions")] @@ -502,9 +502,9 @@ impl<'a> From<&'a ResolverError> for Diagnostic { *span, ) } - ResolverError::SelfReferentialStruct { span } => { + ResolverError::SelfReferentialType { span } => { Diagnostic::simple_error( - "Self-referential structs are not supported".into(), + "Self-referential types are not supported".into(), "".into(), *span, ) diff --git a/compiler/noirc_frontend/src/hir/resolution/visibility.rs b/compiler/noirc_frontend/src/hir/resolution/visibility.rs index 557f799df89..25806ed299d 100644 --- a/compiler/noirc_frontend/src/hir/resolution/visibility.rs +++ b/compiler/noirc_frontend/src/hir/resolution/visibility.rs @@ -1,5 +1,5 @@ use crate::graph::CrateId; -use crate::node_interner::{FuncId, NodeInterner, StructId, TraitId}; +use crate::node_interner::{FuncId, NodeInterner, TraitId, TypeId}; use crate::Type; use std::collections::BTreeMap; @@ -75,7 +75,7 @@ fn module_is_parent_of_struct_module( } pub fn struct_member_is_visible( - struct_id: StructId, + struct_id: TypeId, visibility: ItemVisibility, current_module_id: ModuleId, def_maps: &BTreeMap, diff --git a/compiler/noirc_frontend/src/hir/type_check/generics.rs b/compiler/noirc_frontend/src/hir/type_check/generics.rs index 370223f1f11..f823b495040 100644 --- a/compiler/noirc_frontend/src/hir/type_check/generics.rs +++ b/compiler/noirc_frontend/src/hir/type_check/generics.rs @@ -5,7 +5,7 @@ use iter_extended::vecmap; use crate::{ hir_def::traits::NamedType, node_interner::{FuncId, NodeInterner, TraitId, TypeAliasId}, - ResolvedGeneric, StructType, Type, + DataType, ResolvedGeneric, Type, }; /// Represents something that can be generic over type variables @@ -74,7 +74,7 @@ impl Generic for TypeAliasId { } } -impl Generic for Ref<'_, StructType> { +impl Generic for Ref<'_, DataType> { fn item_kind(&self) -> &'static str { "struct" } diff --git a/compiler/noirc_frontend/src/hir_def/expr.rs b/compiler/noirc_frontend/src/hir_def/expr.rs index 5ac228a56d6..14a3c6fc5e9 100644 --- a/compiler/noirc_frontend/src/hir_def/expr.rs +++ b/compiler/noirc_frontend/src/hir_def/expr.rs @@ -12,7 +12,7 @@ use crate::Shared; use super::stmt::HirPattern; use super::traits::{ResolvedTraitBound, TraitConstraint}; -use super::types::{StructType, Type}; +use super::types::{DataType, Type}; /// A HirExpression is the result of an Expression in the AST undergoing /// name resolution. It is almost identical to the Expression AST node, but @@ -273,7 +273,7 @@ impl HirMethodCallExpression { #[derive(Debug, Clone)] pub struct HirConstructorExpression { - pub r#type: Shared, + pub r#type: Shared, pub struct_generics: Vec, // NOTE: It is tempting to make this a BTreeSet to force ordering of field diff --git a/compiler/noirc_frontend/src/hir_def/function.rs b/compiler/noirc_frontend/src/hir_def/function.rs index aa04738733f..33515d4888e 100644 --- a/compiler/noirc_frontend/src/hir_def/function.rs +++ b/compiler/noirc_frontend/src/hir_def/function.rs @@ -8,7 +8,7 @@ use super::traits::TraitConstraint; use crate::ast::{BlockExpression, FunctionKind, FunctionReturnType, Visibility}; use crate::graph::CrateId; use crate::hir::def_map::LocalModuleId; -use crate::node_interner::{ExprId, NodeInterner, StructId, TraitId, TraitImplId}; +use crate::node_interner::{ExprId, NodeInterner, TraitId, TraitImplId, TypeId}; use crate::{ResolvedGeneric, Type}; @@ -133,7 +133,7 @@ pub struct FuncMeta { pub trait_constraints: Vec, /// The struct this function belongs to, if any - pub struct_id: Option, + pub struct_id: Option, // The trait this function belongs to, if any pub trait_id: Option, diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 4eeec314917..2e665fde3f3 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -21,7 +21,7 @@ use noirc_printable_type::PrintableType; use crate::{ ast::{Ident, Signedness}, - node_interner::StructId, + node_interner::TypeId, }; use super::{ @@ -67,7 +67,7 @@ pub enum Type { /// A user-defined struct type. The `Shared` field here refers to /// the shared definition for each instance of this struct type. The `Vec` /// represents the generic arguments (if any) to this struct type. - Struct(Shared, Vec), + DataType(Shared, Vec), /// A user-defined alias to another type. Similar to a Struct, this carries a shared /// reference to the definition of the alias along with any generics that may have @@ -330,31 +330,46 @@ pub enum QuotedType { /// the binding to later be undone if needed. pub type TypeBindings = HashMap; -/// Represents a struct type in the type system. Each instance of this -/// rust struct will be shared across all Type::Struct variants that represent -/// the same struct type. -pub struct StructType { - /// A unique id representing this struct type. Used to check if two - /// struct types are equal. - pub id: StructId, +/// Represents a struct or enum type in the type system. Each instance of this +/// rust struct will be shared across all Type::DataType variants that represent +/// the same struct or enum type. +pub struct DataType { + /// A unique id representing this type. Used to check if two types are equal. + pub id: TypeId, pub name: Ident, - /// Fields are ordered and private, they should only - /// be accessed through get_field(), get_fields(), or instantiate() + /// A type's body is private to force struct fields or enum variants to only be + /// accessed through get_field(), get_fields(), instantiate(), or similar functions /// since these will handle applying generic arguments to fields as well. - fields: Vec, + body: TypeBody, pub generics: Generics, pub location: Location, } +enum TypeBody { + /// A type with no body is still in the process of being created + None, + Struct(Vec), + + #[allow(unused)] + Enum(Vec), +} + +#[derive(Clone)] pub struct StructField { pub visibility: ItemVisibility, pub name: Ident, pub typ: Type, } +#[derive(Clone)] +pub struct EnumVariant { + pub name: Ident, + pub params: Vec, +} + /// Corresponds to generic lists such as `` in the source program. /// Used mainly for resolved types which no longer need information such /// as names or kinds @@ -388,42 +403,35 @@ enum FunctionCoercionResult { UnconstrainedMismatch(Type), } -impl std::hash::Hash for StructType { +impl std::hash::Hash for DataType { fn hash(&self, state: &mut H) { self.id.hash(state); } } -impl Eq for StructType {} +impl Eq for DataType {} -impl PartialEq for StructType { +impl PartialEq for DataType { fn eq(&self, other: &Self) -> bool { self.id == other.id } } -impl PartialOrd for StructType { +impl PartialOrd for DataType { fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) } } -impl Ord for StructType { +impl Ord for DataType { fn cmp(&self, other: &Self) -> std::cmp::Ordering { self.id.cmp(&other.id) } } -impl StructType { - pub fn new( - id: StructId, - name: Ident, - - location: Location, - fields: Vec, - generics: Generics, - ) -> StructType { - StructType { id, fields, name, location, generics } +impl DataType { + pub fn new(id: TypeId, name: Ident, location: Location, generics: Generics) -> DataType { + DataType { id, name, location, generics, body: TypeBody::None } } /// To account for cyclic references between structs, a struct's @@ -431,14 +439,46 @@ impl StructType { /// created. Therefore, this method is used to set the fields once they /// become known. pub fn set_fields(&mut self, fields: Vec) { - self.fields = fields; + self.body = TypeBody::Struct(fields); } - pub fn num_fields(&self) -> usize { - self.fields.len() + pub fn is_struct(&self) -> bool { + matches!(&self.body, TypeBody::Struct(_)) + } + + /// Retrieve the fields of this type with no modifications. + /// Returns None if this is not a struct type. + pub fn try_fields_raw(&self) -> Option<&[StructField]> { + match &self.body { + TypeBody::Struct(fields) => Some(fields), + _ => None, + } + } + + /// Retrieve the fields of this type with no modifications. + /// Panics if this is not a struct type. + fn fields_raw(&self) -> &[StructField] { + match &self.body { + TypeBody::Struct(fields) => fields, + // Turns out we call `fields_raw` in a few places before a type may be fully finished. + // One of these is when checking for nested slices, so that check will have false + // negatives. + TypeBody::None => &[], + _ => panic!("Called DataType::fields_raw on a non-struct type: {}", self.name), + } + } + + /// Retrieve the variants of this type with no modifications. + /// Panics if this is not an enum type. + fn variants_raw(&self) -> &[EnumVariant] { + match &self.body { + TypeBody::Enum(variants) => variants, + _ => panic!("Called DataType::variants_raw on a non-enum type: {}", self.name), + } } /// Returns the field matching the given field name, as well as its visibility and field index. + /// Panics if this is not a struct type. pub fn get_field( &self, field_name: &str, @@ -446,42 +486,38 @@ impl StructType { ) -> Option<(Type, ItemVisibility, usize)> { assert_eq!(self.generics.len(), generic_args.len()); - self.fields.iter().enumerate().find(|(_, field)| field.name.0.contents == field_name).map( - |(i, field)| { - let substitutions = self - .generics - .iter() - .zip(generic_args) - .map(|(old, new)| { - ( - old.type_var.id(), - (old.type_var.clone(), old.type_var.kind(), new.clone()), - ) - }) - .collect(); + let mut fields = self.fields_raw().iter().enumerate(); + fields.find(|(_, field)| field.name.0.contents == field_name).map(|(i, field)| { + let generics = self.generics.iter().zip(generic_args); + let substitutions = generics + .map(|(old, new)| { + (old.type_var.id(), (old.type_var.clone(), old.type_var.kind(), new.clone())) + }) + .collect(); - (field.typ.substitute(&substitutions), field.visibility, i) - }, - ) + (field.typ.substitute(&substitutions), field.visibility, i) + }) } /// Returns all the fields of this type, after being applied to the given generic arguments. + /// Panics if this is not a struct type. pub fn get_fields_with_visibility( &self, generic_args: &[Type], ) -> Vec<(String, ItemVisibility, Type)> { let substitutions = self.get_fields_substitutions(generic_args); - vecmap(&self.fields, |field| { + vecmap(self.fields_raw(), |field| { let name = field.name.0.contents.clone(); (name, field.visibility, field.typ.substitute(&substitutions)) }) } + /// Retrieve the fields of this type. Panics if this is not a struct type pub fn get_fields(&self, generic_args: &[Type]) -> Vec<(String, Type)> { let substitutions = self.get_fields_substitutions(generic_args); - vecmap(&self.fields, |field| { + vecmap(self.fields_raw(), |field| { let name = field.name.0.contents.clone(); (name, field.typ.substitute(&substitutions)) }) @@ -508,21 +544,35 @@ impl StructType { /// /// This method is almost never what is wanted for type checking or monomorphization, /// prefer to use `get_fields` whenever possible. + /// + /// Panics if this is not a struct type. pub fn get_fields_as_written(&self) -> Vec { - vecmap(&self.fields, |field| StructField { - visibility: field.visibility, - name: field.name.clone(), - typ: field.typ.clone(), - }) + self.fields_raw().to_vec() } - /// Returns the field at the given index. Panics if no field exists at the given index. + /// Returns the name and raw parameters of each variant of this type. + /// This will not substitute any generic arguments so a generic variant like `X` + /// in `enum Foo { X(T) }` will return a `("X", Vec)` pair. + /// + /// Panics if this is not an enum type. + pub fn get_variants_as_written(&self) -> Vec { + self.variants_raw().to_vec() + } + + /// Returns the field at the given index. Panics if no field exists at the given index or this + /// is not a struct type. pub fn field_at(&self, index: usize) -> &StructField { - &self.fields[index] + &self.fields_raw()[index] + } + + /// Returns the enum variant at the given index. Panics if no field exists at the given index + /// or this is not an enum type. + pub fn variant_at(&self, index: usize) -> &EnumVariant { + &self.variants_raw()[index] } pub fn field_names(&self) -> BTreeSet { - self.fields.iter().map(|field| field.name.clone()).collect() + self.fields_raw().iter().map(|field| field.name.clone()).collect() } /// Instantiate this struct type, returning a Vec of the new generic args (in @@ -530,9 +580,27 @@ impl StructType { pub fn instantiate(&self, interner: &mut NodeInterner) -> Vec { vecmap(&self.generics, |generic| interner.next_type_variable_with_kind(generic.kind())) } + + /// Returns the function type of the variant at the given index of this enum. + /// Requires the `Shared` handle of self to create the given function type. + /// Panics if this is not an enum. + /// + /// The function type uses the variant "as written" ie. no generic substitutions. + /// Although the returned function is technically generic, Type::Function is returned + /// instead of Type::Forall. + pub fn variant_function_type(&self, variant_index: usize, this: Shared) -> Type { + let variant = self.variant_at(variant_index); + let args = variant.params.clone(); + assert_eq!(this.borrow().id, self.id); + let generics = vecmap(&self.generics, |generic| { + Type::NamedGeneric(generic.type_var.clone(), generic.name.clone()) + }); + let ret = Box::new(Type::DataType(this, generics)); + Type::Function(args, ret, Box::new(Type::Unit), false) + } } -impl std::fmt::Display for StructType { +impl std::fmt::Display for DataType { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "{}", self.name) } @@ -850,7 +918,7 @@ impl std::fmt::Display for Type { } } } - Type::Struct(s, args) => { + Type::DataType(s, args) => { let args = vecmap(args, |arg| arg.to_string()); if args.is_empty() { write!(f, "{}", s.borrow()) @@ -1084,7 +1152,7 @@ impl Type { alias_type.borrow().get_type(&generics).is_primitive() } Type::MutableReference(typ) => typ.is_primitive(), - Type::Struct(..) + Type::DataType(..) | Type::TypeVariable(..) | Type::TraitAsType(..) | Type::NamedGeneric(..) @@ -1145,7 +1213,7 @@ impl Type { } Type::String(length) => length.is_valid_for_program_input(), Type::Tuple(elements) => elements.iter().all(|elem| elem.is_valid_for_program_input()), - Type::Struct(definition, generics) => definition + Type::DataType(definition, generics) => definition .borrow() .get_fields(generics) .into_iter() @@ -1200,7 +1268,7 @@ impl Type { } Type::String(length) => length.is_valid_non_inlined_function_input(), Type::Tuple(elements) => elements.iter().all(|elem| elem.is_valid_non_inlined_function_input()), - Type::Struct(definition, generics) => definition + Type::DataType(definition, generics) => definition .borrow() .get_fields(generics) .into_iter() @@ -1252,7 +1320,7 @@ impl Type { Type::Tuple(elements) => { elements.iter().all(|elem| elem.is_valid_for_unconstrained_boundary()) } - Type::Struct(definition, generics) => definition + Type::DataType(definition, generics) => definition .borrow() .get_fields(generics) .into_iter() @@ -1324,7 +1392,7 @@ impl Type { | Type::FmtString(..) | Type::Unit | Type::Tuple(..) - | Type::Struct(..) + | Type::DataType(..) | Type::TraitAsType(..) | Type::Function(..) | Type::MutableReference(..) @@ -1398,7 +1466,7 @@ impl Type { let typ = typ.as_ref(); length * typ.field_count(location) } - Type::Struct(def, args) => { + Type::DataType(def, args) => { let struct_type = def.borrow(); let fields = struct_type.get_fields(args); fields.iter().fold(0, |acc, (_, field_type)| acc + field_type.field_count(location)) @@ -1439,7 +1507,7 @@ impl Type { pub(crate) fn contains_slice(&self) -> bool { match self { Type::Slice(_) => true, - Type::Struct(struct_typ, generics) => { + Type::DataType(struct_typ, generics) => { let fields = struct_typ.borrow().get_fields(generics); for field in fields.iter() { if field.1.contains_slice() { @@ -1687,7 +1755,7 @@ impl Type { // No recursive try_unify call for struct fields. Don't want // to mutate shared type variables within struct definitions. // This isn't possible currently but will be once noir gets generic types - (Struct(id_a, args_a), Struct(id_b, args_b)) => { + (DataType(id_a, args_a), DataType(id_b, args_b)) => { if id_a == id_b && args_a.len() == args_b.len() { for (a, b) in args_a.iter().zip(args_b) { a.try_unify(b, bindings)?; @@ -2037,28 +2105,6 @@ impl Type { } } - /// Iterate over the fields of this type. - /// Panics if the type is not a struct or tuple. - pub fn iter_fields(&self) -> impl Iterator { - let fields: Vec<_> = match self { - // Unfortunately the .borrow() here forces us to collect into a Vec - // only to have to call .into_iter again afterward. Trying to elide - // collecting to a Vec leads to us dropping the temporary Ref before - // the iterator is returned - Type::Struct(def, args) => vecmap(&def.borrow().fields, |field| { - let name = &field.name.0.contents; - let typ = def.borrow().get_field(name, args).unwrap().0; - (name.clone(), typ) - }), - Type::Tuple(fields) => { - let fields = fields.iter().enumerate(); - vecmap(fields, |(i, field)| (i.to_string(), field.clone())) - } - other => panic!("Tried to iterate over the fields of '{other}', which has none"), - }; - fields.into_iter() - } - /// Retrieves the type of the given field name /// Panics if the type is not a struct or tuple. pub fn get_field_type_and_visibility( @@ -2066,7 +2112,7 @@ impl Type { field_name: &str, ) -> Option<(Type, ItemVisibility)> { match self.follow_bindings() { - Type::Struct(def, args) => def + Type::DataType(def, args) => def .borrow() .get_field(field_name, &args) .map(|(typ, visibility, _)| (typ, visibility)), @@ -2266,11 +2312,11 @@ impl Type { // Do not substitute_helper fields, it can lead to infinite recursion // and we should not match fields when type checking anyway. - Type::Struct(fields, args) => { + Type::DataType(fields, args) => { let args = vecmap(args, |arg| { arg.substitute_helper(type_bindings, substitute_bound_typevars) }); - Type::Struct(fields.clone(), args) + Type::DataType(fields.clone(), args) } Type::Alias(alias, args) => { let args = vecmap(args, |arg| { @@ -2342,7 +2388,7 @@ impl Type { let field_occurs = fields.occurs(target_id); len_occurs || field_occurs } - Type::Struct(_, generic_args) | Type::Alias(_, generic_args) => { + Type::DataType(_, generic_args) | Type::Alias(_, generic_args) => { generic_args.iter().any(|arg| arg.occurs(target_id)) } Type::TraitAsType(_, _, args) => { @@ -2399,9 +2445,9 @@ impl Type { let args = Box::new(args.follow_bindings()); FmtString(size, args) } - Struct(def, args) => { + DataType(def, args) => { let args = vecmap(args, |arg| arg.follow_bindings()); - Struct(def.clone(), args) + DataType(def.clone(), args) } Alias(def, args) => { // We don't need to vecmap(args, follow_bindings) since we're recursively @@ -2499,7 +2545,7 @@ impl Type { field.replace_named_generics_with_type_variables(); } } - Type::Struct(_, generics) => { + Type::DataType(_, generics) => { for generic in generics { generic.replace_named_generics_with_type_variables(); } @@ -2601,7 +2647,7 @@ impl Type { | Type::FmtString(..) | Type::Unit | Type::Tuple(..) - | Type::Struct(..) + | Type::DataType(..) | Type::TraitAsType(..) | Type::Function(..) | Type::Forall(..) @@ -2759,7 +2805,7 @@ impl From<&Type> for PrintableType { Type::Error => unreachable!(), Type::Unit => PrintableType::Unit, Type::Constant(_, _) => unreachable!(), - Type::Struct(def, ref args) => { + Type::DataType(def, ref args) => { let struct_type = def.borrow(); let fields = struct_type.get_fields(args); let fields = vecmap(fields, |(name, typ)| (name, typ.into())); @@ -2815,7 +2861,7 @@ impl std::fmt::Debug for Type { write!(f, "{}", binding.borrow()) } } - Type::Struct(s, args) => { + Type::DataType(s, args) => { let args = vecmap(args, |arg| format!("{:?}", arg)); if args.is_empty() { write!(f, "{}", s.borrow()) @@ -2897,7 +2943,7 @@ impl std::fmt::Debug for TypeVariable { } } -impl std::fmt::Debug for StructType { +impl std::fmt::Debug for DataType { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "{}", self.name) } @@ -2934,7 +2980,7 @@ impl std::hash::Hash for Type { env.hash(state); } Type::Tuple(elems) => elems.hash(state), - Type::Struct(def, args) => { + Type::DataType(def, args) => { def.hash(state); args.hash(state); } @@ -3005,7 +3051,7 @@ impl PartialEq for Type { lhs_len == rhs_len && lhs_env == rhs_env } (Tuple(lhs_types), Tuple(rhs_types)) => lhs_types == rhs_types, - (Struct(lhs_struct, lhs_generics), Struct(rhs_struct, rhs_generics)) => { + (DataType(lhs_struct, lhs_generics), DataType(rhs_struct, rhs_generics)) => { lhs_struct == rhs_struct && lhs_generics == rhs_generics } (Alias(lhs_alias, lhs_generics), Alias(rhs_alias, rhs_generics)) => { diff --git a/compiler/noirc_frontend/src/locations.rs b/compiler/noirc_frontend/src/locations.rs index ecae5b19a95..33c37172b50 100644 --- a/compiler/noirc_frontend/src/locations.rs +++ b/compiler/noirc_frontend/src/locations.rs @@ -7,7 +7,7 @@ use crate::{ ast::{FunctionDefinition, ItemVisibility}, hir::def_map::{ModuleDefId, ModuleId}, node_interner::{ - DefinitionId, FuncId, GlobalId, NodeInterner, ReferenceId, StructId, TraitId, TypeAliasId, + DefinitionId, FuncId, GlobalId, NodeInterner, ReferenceId, TraitId, TypeAliasId, TypeId, }, }; use petgraph::prelude::NodeIndex as PetGraphIndex; @@ -60,18 +60,22 @@ impl NodeInterner { match reference { ReferenceId::Module(id) => self.module_attributes(&id).location, ReferenceId::Function(id) => self.function_modifiers(&id).name_location, - ReferenceId::Struct(id) => { - let struct_type = self.get_struct(id); - let struct_type = struct_type.borrow(); - Location::new(struct_type.name.span(), struct_type.location.file) + ReferenceId::Struct(id) | ReferenceId::Enum(id) => { + let typ = self.get_type(id); + let typ = typ.borrow(); + Location::new(typ.name.span(), typ.location.file) } ReferenceId::StructMember(id, field_index) => { - let struct_type = self.get_struct(id); + let struct_type = self.get_type(id); let struct_type = struct_type.borrow(); - Location::new( - struct_type.field_at(field_index).name.span(), - struct_type.location.file, - ) + let file = struct_type.location.file; + Location::new(struct_type.field_at(field_index).name.span(), file) + } + ReferenceId::EnumVariant(id, variant_index) => { + let typ = self.get_type(id); + let typ = typ.borrow(); + let file = typ.location.file; + Location::new(typ.variant_at(variant_index).name.span(), file) } ReferenceId::Trait(id) => { let trait_type = self.get_trait(id); @@ -126,7 +130,7 @@ impl NodeInterner { pub(crate) fn add_struct_reference( &mut self, - id: StructId, + id: TypeId, location: Location, is_self_type: bool, ) { @@ -135,7 +139,7 @@ impl NodeInterner { pub(crate) fn add_struct_member_reference( &mut self, - id: StructId, + id: TypeId, member_index: usize, location: Location, ) { @@ -324,7 +328,7 @@ impl NodeInterner { pub(crate) fn register_struct( &mut self, - id: StructId, + id: TypeId, name: String, visibility: ItemVisibility, parent_module_id: ModuleId, @@ -333,6 +337,17 @@ impl NodeInterner { self.register_name_for_auto_import(name, ModuleDefId::TypeId(id), visibility, None); } + pub(crate) fn register_enum( + &mut self, + id: TypeId, + name: String, + visibility: ItemVisibility, + parent_module_id: ModuleId, + ) { + self.add_definition_location(ReferenceId::Enum(id), Some(parent_module_id)); + self.register_name_for_auto_import(name, ModuleDefId::TypeId(id), visibility, None); + } + pub(crate) fn register_trait( &mut self, id: TraitId, diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 191c3937e4b..3e4b478c23a 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -1159,7 +1159,7 @@ impl<'interner> Monomorphizer<'interner> { monomorphized_default } - HirType::Struct(def, args) => { + HirType::DataType(def, args) => { // Not all generic arguments may be used in a struct's fields so we have to check // the arguments as well as the fields in case any need to be defaulted or are unbound. for arg in args { @@ -1279,7 +1279,7 @@ impl<'interner> Monomorphizer<'interner> { Self::check_type(&default, location) } - HirType::Struct(_def, args) => { + HirType::DataType(_def, args) => { for arg in args { Self::check_type(arg, location)?; } @@ -2133,7 +2133,7 @@ fn unwrap_struct_type( location: Location, ) -> Result, MonomorphizationError> { match typ.follow_bindings() { - HirType::Struct(def, args) => { + HirType::DataType(def, args) => { // Some of args might not be mentioned in fields, so we need to check that they aren't unbound. for arg in &args { Monomorphizer::check_type(arg, location)?; diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 431bed3b604..fbf933f59dd 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -18,7 +18,7 @@ use crate::ast::{ use crate::graph::CrateId; use crate::hir::comptime; use crate::hir::def_collector::dc_crate::CompilationError; -use crate::hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait, UnresolvedTypeAlias}; +use crate::hir::def_collector::dc_crate::{UnresolvedTrait, UnresolvedTypeAlias}; use crate::hir::def_map::DefMaps; use crate::hir::def_map::{LocalModuleId, ModuleDefId, ModuleId}; use crate::hir::type_check::generics::TraitGenerics; @@ -32,7 +32,7 @@ use crate::hir_def::expr::HirIdent; use crate::hir_def::stmt::HirLetStatement; use crate::hir_def::traits::TraitImpl; use crate::hir_def::traits::{Trait, TraitConstraint}; -use crate::hir_def::types::{Kind, StructType, Type}; +use crate::hir_def::types::{DataType, Kind, Type}; use crate::hir_def::{ expr::HirExpression, function::{FuncMeta, HirFunction}, @@ -106,14 +106,14 @@ pub struct NodeInterner { // Similar to `id_to_type` but maps definitions to their type definition_to_type: HashMap, - // Struct map. + // Struct and Enum map. // - // Each struct definition is possibly shared across multiple type nodes. + // Each type definition is possibly shared across multiple type nodes. // It is also mutated through the RefCell during name resolution to append // methods from impls to the type. - structs: HashMap>, + data_types: HashMap>, - struct_attributes: HashMap, + type_attributes: HashMap, // Maps TypeAliasId -> Shared // @@ -286,7 +286,7 @@ pub struct NodeInterner { /// ``` #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub enum DependencyId { - Struct(StructId), + Struct(TypeId), Global(GlobalId), Function(FuncId), Alias(TypeAliasId), @@ -299,8 +299,10 @@ pub enum DependencyId { #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub enum ReferenceId { Module(ModuleId), - Struct(StructId), - StructMember(StructId, usize), + Struct(TypeId), + StructMember(TypeId, usize), + Enum(TypeId), + EnumVariant(TypeId, usize), Trait(TraitId), Global(GlobalId), Function(FuncId), @@ -465,14 +467,14 @@ impl fmt::Display for FuncId { } #[derive(Debug, Eq, PartialEq, Hash, Copy, Clone, PartialOrd, Ord)] -pub struct StructId(ModuleId); +pub struct TypeId(ModuleId); -impl StructId { +impl TypeId { //dummy id for error reporting // This can be anything, as the program will ultimately fail // after resolution - pub fn dummy_id() -> StructId { - StructId(ModuleId { krate: CrateId::dummy_id(), local_id: LocalModuleId::dummy_id() }) + pub fn dummy_id() -> TypeId { + TypeId(ModuleId { krate: CrateId::dummy_id(), local_id: LocalModuleId::dummy_id() }) } pub fn module_id(self) -> ModuleId { @@ -652,8 +654,8 @@ impl Default for NodeInterner { definitions: vec![], id_to_type: HashMap::default(), definition_to_type: HashMap::default(), - structs: HashMap::default(), - struct_attributes: HashMap::default(), + data_types: HashMap::default(), + type_attributes: HashMap::default(), type_aliases: Vec::new(), traits: HashMap::default(), trait_implementations: HashMap::default(), @@ -747,25 +749,25 @@ impl NodeInterner { self.traits.insert(type_id, new_trait); } - pub fn new_struct( + /// Creates a new struct or enum type with no fields or variants. + #[allow(clippy::too_many_arguments)] + pub fn new_type( &mut self, - typ: &UnresolvedStruct, + name: Ident, + span: Span, + attributes: Vec, generics: Generics, krate: CrateId, local_id: LocalModuleId, file_id: FileId, - ) -> StructId { - let struct_id = StructId(ModuleId { krate, local_id }); - let name = typ.struct_def.name.clone(); + ) -> TypeId { + let type_id = TypeId(ModuleId { krate, local_id }); - // Fields will be filled in later - let no_fields = Vec::new(); - - let location = Location::new(typ.struct_def.span, file_id); - let new_struct = StructType::new(struct_id, name, location, no_fields, generics); - self.structs.insert(struct_id, Shared::new(new_struct)); - self.struct_attributes.insert(struct_id, typ.struct_def.attributes.clone()); - struct_id + let location = Location::new(span, file_id); + let new_type = DataType::new(type_id, name, location, generics); + self.data_types.insert(type_id, Shared::new(new_type)); + self.type_attributes.insert(type_id, attributes); + type_id } pub fn push_type_alias( @@ -791,8 +793,8 @@ impl NodeInterner { pub fn add_type_alias_ref(&mut self, type_id: TypeAliasId, location: Location) { self.type_alias_ref.push((type_id, location)); } - pub fn update_struct(&mut self, type_id: StructId, f: impl FnOnce(&mut StructType)) { - let mut value = self.structs.get_mut(&type_id).unwrap().borrow_mut(); + pub fn update_struct(&mut self, type_id: TypeId, f: impl FnOnce(&mut DataType)) { + let mut value = self.data_types.get_mut(&type_id).unwrap().borrow_mut(); f(&mut value); } @@ -803,10 +805,10 @@ impl NodeInterner { pub fn update_struct_attributes( &mut self, - type_id: StructId, + type_id: TypeId, f: impl FnOnce(&mut StructAttributes), ) { - let value = self.struct_attributes.get_mut(&type_id).unwrap(); + let value = self.type_attributes.get_mut(&type_id).unwrap(); f(value); } @@ -1096,8 +1098,8 @@ impl NodeInterner { &self.function_modifiers[func_id].attributes } - pub fn struct_attributes(&self, struct_id: &StructId) -> &StructAttributes { - &self.struct_attributes[struct_id] + pub fn struct_attributes(&self, struct_id: &TypeId) -> &StructAttributes { + &self.type_attributes[struct_id] } pub fn add_module_attributes(&mut self, module_id: ModuleId, attributes: ModuleAttributes) { @@ -1213,8 +1215,8 @@ impl NodeInterner { self.id_to_location.insert(id.into(), Location::new(span, file)); } - pub fn get_struct(&self, id: StructId) -> Shared { - self.structs[&id].clone() + pub fn get_type(&self, id: TypeId) -> Shared { + self.data_types[&id].clone() } pub fn get_type_methods(&self, typ: &Type) -> Option<&HashMap> { @@ -1387,7 +1389,7 @@ impl NodeInterner { unreachable!("Cannot add a method to the unsupported type '{}'", self_type) }); - if trait_id.is_none() && matches!(self_type, Type::Struct(..)) { + if trait_id.is_none() && matches!(self_type, Type::DataType(..)) { if let Some(existing) = self.lookup_direct_method(self_type, &method_name, true) { return Some(existing); @@ -1980,7 +1982,7 @@ impl NodeInterner { /// Register that `dependent` depends on `dependency`. /// This is usually because `dependent` refers to `dependency` in one of its struct fields. - pub fn add_type_dependency(&mut self, dependent: DependencyId, dependency: StructId) { + pub fn add_type_dependency(&mut self, dependent: DependencyId, dependency: TypeId) { self.add_dependency(dependent, DependencyId::Struct(dependency)); } @@ -2033,7 +2035,7 @@ impl NodeInterner { for (i, index) in scc.iter().enumerate() { match self.dependency_graph[*index] { DependencyId::Struct(struct_id) => { - let struct_type = self.get_struct(struct_id); + let struct_type = self.get_type(struct_id); let struct_type = struct_type.borrow(); push_error(struct_type.name.to_string(), &scc, i, struct_type.location); break; @@ -2080,7 +2082,7 @@ impl NodeInterner { /// element at the given start index. fn get_cycle_error_string(&self, scc: &[PetGraphIndex], start_index: usize) -> String { let index_to_string = |index: PetGraphIndex| match self.dependency_graph[index] { - DependencyId::Struct(id) => Cow::Owned(self.get_struct(id).borrow().name.to_string()), + DependencyId::Struct(id) => Cow::Owned(self.get_type(id).borrow().name.to_string()), DependencyId::Function(id) => Cow::Borrowed(self.function_name(&id)), DependencyId::Alias(id) => { Cow::Owned(self.get_type_alias(id).borrow().name.to_string()) @@ -2422,7 +2424,7 @@ enum TypeMethodKey { Function, Generic, Quoted(QuotedType), - Struct(StructId), + Struct(TypeId), } fn get_type_method_key(typ: &Type) -> Option { @@ -2450,7 +2452,7 @@ fn get_type_method_key(typ: &Type) -> Option { Type::Quoted(quoted) => Some(Quoted(*quoted)), Type::MutableReference(element) => get_type_method_key(element), Type::Alias(alias, _) => get_type_method_key(&alias.borrow().typ), - Type::Struct(struct_type, _) => Some(Struct(struct_type.borrow().id)), + Type::DataType(struct_type, _) => Some(Struct(struct_type.borrow().id)), // We do not support adding methods to these types Type::Forall(_, _) diff --git a/compiler/noirc_frontend/src/resolve_locations.rs b/compiler/noirc_frontend/src/resolve_locations.rs index b9e86bf0ef7..1b904f653bd 100644 --- a/compiler/noirc_frontend/src/resolve_locations.rs +++ b/compiler/noirc_frontend/src/resolve_locations.rs @@ -93,7 +93,7 @@ impl NodeInterner { fn get_type_location_from_index(&self, index: impl Into) -> Option { match self.id_type(index.into()) { - Type::Struct(struct_type, _) => Some(struct_type.borrow().location), + Type::DataType(struct_type, _) => Some(struct_type.borrow().location), _ => None, } } @@ -150,7 +150,7 @@ impl NodeInterner { let expr_rhs = &expr_member_access.rhs; let lhs_self_struct = match self.id_type(expr_lhs) { - Type::Struct(struct_type, _) => struct_type, + Type::DataType(struct_type, _) => struct_type, _ => return None, }; @@ -217,7 +217,7 @@ impl NodeInterner { .iter() .find(|(_typ, type_ref_location)| type_ref_location.contains(&location)) .and_then(|(typ, _)| match typ { - Type::Struct(struct_typ, _) => Some(struct_typ.borrow().location), + Type::DataType(struct_typ, _) => Some(struct_typ.borrow().location), _ => None, }) } diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index c2d7e5c5f49..064068dba7d 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -2818,12 +2818,13 @@ fn duplicate_struct_field() { let errors = get_program_errors(src); assert_eq!(errors.len(), 1); - let CompilationError::DefinitionError(DefCollectorErrorKind::DuplicateField { + let CompilationError::DefinitionError(DefCollectorErrorKind::Duplicate { + typ: _, first_def, second_def, }) = &errors[0].0 else { - panic!("Expected a duplicate field error, got {:?}", errors[0].0); + panic!("Expected a 'duplicate' error, got {:?}", errors[0].0); }; assert_eq!(first_def.to_string(), "x"); diff --git a/compiler/noirc_frontend/src/usage_tracker.rs b/compiler/noirc_frontend/src/usage_tracker.rs index 6987358ddb7..ea4919096c0 100644 --- a/compiler/noirc_frontend/src/usage_tracker.rs +++ b/compiler/noirc_frontend/src/usage_tracker.rs @@ -3,14 +3,15 @@ use std::collections::HashMap; use crate::{ ast::{Ident, ItemVisibility}, hir::def_map::ModuleId, - node_interner::{FuncId, GlobalId, StructId, TraitId, TypeAliasId}, + node_interner::{FuncId, GlobalId, TraitId, TypeAliasId, TypeId}, }; #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum UnusedItem { Import, Function(FuncId), - Struct(StructId), + Struct(TypeId), + Enum(TypeId), Trait(TraitId), TypeAlias(TypeAliasId), Global(GlobalId), @@ -22,6 +23,7 @@ impl UnusedItem { UnusedItem::Import => "import", UnusedItem::Function(_) => "function", UnusedItem::Struct(_) => "struct", + UnusedItem::Enum(_) => "enum", UnusedItem::Trait(_) => "trait", UnusedItem::TypeAlias(_) => "type alias", UnusedItem::Global(_) => "global", diff --git a/tooling/lsp/src/requests/code_action/fill_struct_fields.rs b/tooling/lsp/src/requests/code_action/fill_struct_fields.rs index 739f0bf4a21..7a4d562e402 100644 --- a/tooling/lsp/src/requests/code_action/fill_struct_fields.rs +++ b/tooling/lsp/src/requests/code_action/fill_struct_fields.rs @@ -24,7 +24,7 @@ impl<'a> CodeActionFinder<'a> { return; }; - let struct_type = self.interner.get_struct(struct_id); + let struct_type = self.interner.get_type(struct_id); let struct_type = struct_type.borrow(); // First get all of the struct's fields diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index a845fd4496f..597b778260a 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -31,10 +31,10 @@ use noirc_frontend::{ }, }, hir_def::traits::Trait, - node_interner::{FuncId, NodeInterner, ReferenceId, StructId}, + node_interner::{FuncId, NodeInterner, ReferenceId, TypeId}, parser::{Item, ItemKind, ParsedSubModule}, token::{MetaAttribute, Token, Tokens}, - Kind, ParsedModule, StructType, Type, TypeBinding, + DataType, Kind, ParsedModule, Type, TypeBinding, }; use sort_text::underscore_sort_text; @@ -203,7 +203,7 @@ impl<'a> NodeFinder<'a> { return; }; - let struct_type = self.interner.get_struct(struct_id); + let struct_type = self.interner.get_type(struct_id); let struct_type = struct_type.borrow(); // First get all of the struct's fields @@ -318,9 +318,9 @@ impl<'a> NodeFinder<'a> { match module_def_id { ModuleDefId::ModuleId(id) => module_id = id, ModuleDefId::TypeId(struct_id) => { - let struct_type = self.interner.get_struct(struct_id); + let struct_type = self.interner.get_type(struct_id); self.complete_type_methods( - &Type::Struct(struct_type, vec![]), + &Type::DataType(struct_type, vec![]), &prefix, FunctionKind::Any, function_completion_kind, @@ -568,7 +568,7 @@ impl<'a> NodeFinder<'a> { ) { let typ = &typ; match typ { - Type::Struct(struct_type, generics) => { + Type::DataType(struct_type, generics) => { self.complete_struct_fields(&struct_type.borrow(), generics, prefix, self_prefix); } Type::MutableReference(typ) => { @@ -800,7 +800,7 @@ impl<'a> NodeFinder<'a> { fn complete_struct_fields( &mut self, - struct_type: &StructType, + struct_type: &DataType, generics: &[Type], prefix: &str, self_prefix: bool, @@ -1799,7 +1799,7 @@ impl<'a> Visitor for NodeFinder<'a> { fn get_field_type(typ: &Type, name: &str) -> Option { match typ { - Type::Struct(struct_type, generics) => { + Type::DataType(struct_type, generics) => { Some(struct_type.borrow().get_field(name, generics)?.0) } Type::Tuple(types) => { @@ -1839,9 +1839,9 @@ fn get_array_element_type(typ: Type) -> Option { } } -fn get_type_struct_id(typ: &Type) -> Option { +fn get_type_struct_id(typ: &Type) -> Option { match typ { - Type::Struct(struct_type, _) => Some(struct_type.borrow().id), + Type::DataType(struct_type, _) => Some(struct_type.borrow().id), Type::Alias(type_alias, generics) => { let type_alias = type_alias.borrow(); let typ = type_alias.get_type(generics); @@ -1898,10 +1898,12 @@ fn module_def_id_from_reference_id(reference_id: ReferenceId) -> Option Some(ModuleDefId::ModuleId(module_id)), ReferenceId::Struct(struct_id) => Some(ModuleDefId::TypeId(struct_id)), + ReferenceId::Enum(enum_id) => Some(ModuleDefId::TypeId(enum_id)), ReferenceId::Trait(trait_id) => Some(ModuleDefId::TraitId(trait_id)), ReferenceId::Function(func_id) => Some(ModuleDefId::FunctionId(func_id)), ReferenceId::Alias(type_alias_id) => Some(ModuleDefId::TypeAliasId(type_alias_id)), ReferenceId::StructMember(_, _) + | ReferenceId::EnumVariant(_, _) | ReferenceId::Global(_) | ReferenceId::Local(_) | ReferenceId::Reference(_, _) => None, diff --git a/tooling/lsp/src/requests/completion/completion_items.rs b/tooling/lsp/src/requests/completion/completion_items.rs index db31683d51a..c8ae16bf1f4 100644 --- a/tooling/lsp/src/requests/completion/completion_items.rs +++ b/tooling/lsp/src/requests/completion/completion_items.rs @@ -6,7 +6,7 @@ use noirc_frontend::{ ast::AttributeTarget, hir::def_map::{ModuleDefId, ModuleId}, hir_def::{function::FuncMeta, stmt::HirPattern}, - node_interner::{FuncId, GlobalId, ReferenceId, StructId, TraitId, TypeAliasId}, + node_interner::{FuncId, GlobalId, ReferenceId, TraitId, TypeAliasId, TypeId}, QuotedType, Type, }; @@ -110,7 +110,7 @@ impl<'a> NodeFinder<'a> { self.completion_item_with_doc_comments(ReferenceId::Module(id), completion_item) } - fn struct_completion_item(&self, name: String, struct_id: StructId) -> CompletionItem { + fn struct_completion_item(&self, name: String, struct_id: TypeId) -> CompletionItem { let completion_item = simple_completion_item(name.clone(), CompletionItemKind::STRUCT, Some(name)); self.completion_item_with_doc_comments(ReferenceId::Struct(struct_id), completion_item) @@ -120,7 +120,7 @@ impl<'a> NodeFinder<'a> { &self, field: &str, typ: &Type, - struct_id: StructId, + struct_id: TypeId, field_index: usize, self_type: bool, ) -> CompletionItem { diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index 5d8c50fa47b..bfe2a2a2448 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -13,10 +13,10 @@ use noirc_frontend::{ traits::Trait, }, node_interner::{ - DefinitionId, DefinitionKind, ExprId, FuncId, GlobalId, NodeInterner, ReferenceId, - StructId, TraitId, TraitImplKind, TypeAliasId, + DefinitionId, DefinitionKind, ExprId, FuncId, GlobalId, NodeInterner, ReferenceId, TraitId, + TraitImplKind, TypeAliasId, TypeId, }, - Generics, Shared, StructType, Type, TypeAlias, TypeBinding, TypeVariable, + DataType, Generics, Shared, Type, TypeAlias, TypeBinding, TypeVariable, }; use crate::{ @@ -77,6 +77,10 @@ fn format_reference(reference: ReferenceId, args: &ProcessRequestCallbackArgs) - ReferenceId::StructMember(id, field_index) => { Some(format_struct_member(id, field_index, args)) } + ReferenceId::Enum(id) => Some(format_enum(id, args)), + ReferenceId::EnumVariant(id, variant_index) => { + Some(format_enum_variant(id, variant_index, args)) + } ReferenceId::Trait(id) => Some(format_trait(id, args)), ReferenceId::Global(id) => Some(format_global(id, args)), ReferenceId::Function(id) => Some(format_function(id, args)), @@ -122,8 +126,8 @@ fn format_module(id: ModuleId, args: &ProcessRequestCallbackArgs) -> Option String { - let struct_type = args.interner.get_struct(id); +fn format_struct(id: TypeId, args: &ProcessRequestCallbackArgs) -> String { + let struct_type = args.interner.get_type(id); let struct_type = struct_type.borrow(); let mut string = String::new(); @@ -149,12 +153,45 @@ fn format_struct(id: StructId, args: &ProcessRequestCallbackArgs) -> String { string } +fn format_enum(id: TypeId, args: &ProcessRequestCallbackArgs) -> String { + let typ = args.interner.get_type(id); + let typ = typ.borrow(); + + let mut string = String::new(); + if format_parent_module(ReferenceId::Enum(id), args, &mut string) { + string.push('\n'); + } + string.push_str(" "); + string.push_str("enum "); + string.push_str(&typ.name.0.contents); + format_generics(&typ.generics, &mut string); + string.push_str(" {\n"); + for field in typ.get_variants_as_written() { + string.push_str(" "); + string.push_str(&field.name.0.contents); + + if !field.params.is_empty() { + let types = field.params.iter().map(ToString::to_string).collect::>(); + string.push('('); + string.push_str(&types.join(", ")); + string.push(')'); + } + + string.push_str(",\n"); + } + string.push_str(" }"); + + append_doc_comments(args.interner, ReferenceId::Enum(id), &mut string); + + string +} + fn format_struct_member( - id: StructId, + id: TypeId, field_index: usize, args: &ProcessRequestCallbackArgs, ) -> String { - let struct_type = args.interner.get_struct(id); + let struct_type = args.interner.get_type(id); let struct_type = struct_type.borrow(); let field = struct_type.field_at(field_index); @@ -175,6 +212,39 @@ fn format_struct_member( string } +fn format_enum_variant( + id: TypeId, + field_index: usize, + args: &ProcessRequestCallbackArgs, +) -> String { + let enum_type = args.interner.get_type(id); + let enum_type = enum_type.borrow(); + let variant = enum_type.variant_at(field_index); + + let mut string = String::new(); + if format_parent_module(ReferenceId::Enum(id), args, &mut string) { + string.push_str("::"); + } + string.push_str(&enum_type.name.0.contents); + string.push('\n'); + string.push_str(" "); + string.push_str(&variant.name.0.contents); + if !variant.params.is_empty() { + let types = variant.params.iter().map(ToString::to_string).collect::>(); + string.push('('); + string.push_str(&types.join(", ")); + string.push(')'); + } + + for typ in variant.params.iter() { + string.push_str(&go_to_type_links(typ, args.interner, args.files)); + } + + append_doc_comments(args.interner, ReferenceId::EnumVariant(id, field_index), &mut string); + + string +} + fn format_trait(id: TraitId, args: &ProcessRequestCallbackArgs) -> String { let a_trait = args.interner.get_trait(id); @@ -368,7 +438,7 @@ fn format_function(id: FuncId, args: &ProcessRequestCallbackArgs) -> String { true } else if let Some(struct_id) = func_meta.struct_id { - let struct_type = args.interner.get_struct(struct_id); + let struct_type = args.interner.get_type(struct_id); let struct_type = struct_type.borrow(); if formatted_parent_module { string.push_str("::"); @@ -685,7 +755,7 @@ impl<'a> TypeLinksGatherer<'a> { self.gather_type_links(typ); } } - Type::Struct(struct_type, generics) => { + Type::DataType(struct_type, generics) => { self.gather_struct_type_links(struct_type); for generic in generics { self.gather_type_links(generic); @@ -739,7 +809,7 @@ impl<'a> TypeLinksGatherer<'a> { } } - fn gather_struct_type_links(&mut self, struct_type: &Shared) { + fn gather_struct_type_links(&mut self, struct_type: &Shared) { let struct_type = struct_type.borrow(); if let Some(lsp_location) = to_lsp_location(self.files, struct_type.location.file, struct_type.name.span()) diff --git a/tooling/lsp/src/requests/inlay_hint.rs b/tooling/lsp/src/requests/inlay_hint.rs index c6415acb545..6629f3d0336 100644 --- a/tooling/lsp/src/requests/inlay_hint.rs +++ b/tooling/lsp/src/requests/inlay_hint.rs @@ -95,13 +95,21 @@ impl<'a> InlayHintCollector<'a> { self.push_type_hint(lsp_location, &typ, editable); } ReferenceId::StructMember(struct_id, field_index) => { - let struct_type = self.interner.get_struct(struct_id); + let struct_type = self.interner.get_type(struct_id); let struct_type = struct_type.borrow(); let field = struct_type.field_at(field_index); self.push_type_hint(lsp_location, &field.typ, false); } + ReferenceId::EnumVariant(type_id, variant_index) => { + let typ = self.interner.get_type(type_id); + let shared_type = typ.clone(); + let typ = typ.borrow(); + let variant_type = typ.variant_function_type(variant_index, shared_type); + self.push_type_hint(lsp_location, &variant_type, false); + } ReferenceId::Module(_) | ReferenceId::Struct(_) + | ReferenceId::Enum(_) | ReferenceId::Trait(_) | ReferenceId::Function(_) | ReferenceId::Alias(_) @@ -410,7 +418,7 @@ fn push_type_parts(typ: &Type, parts: &mut Vec, files: &File } parts.push(string_part(")")); } - Type::Struct(struct_type, generics) => { + Type::DataType(struct_type, generics) => { let struct_type = struct_type.borrow(); let location = Location::new(struct_type.name.span(), struct_type.location.file); parts.push(text_part_with_location(struct_type.name.to_string(), location, files)); diff --git a/tooling/lsp/src/trait_impl_method_stub_generator.rs b/tooling/lsp/src/trait_impl_method_stub_generator.rs index eb1709e34d0..4e505eb5e12 100644 --- a/tooling/lsp/src/trait_impl_method_stub_generator.rs +++ b/tooling/lsp/src/trait_impl_method_stub_generator.rs @@ -181,7 +181,7 @@ impl<'a> TraitImplMethodStubGenerator<'a> { } self.string.push(')'); } - Type::Struct(struct_type, generics) => { + Type::DataType(struct_type, generics) => { let struct_type = struct_type.borrow(); let current_module_data = From f73dc9a0830b8484684b2ce1bc7fdde938cf537b Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Wed, 22 Jan 2025 11:47:04 +0000 Subject: [PATCH 04/11] fix: preserve types when reading from calldata arrays (#7144) --- compiler/noirc_evaluator/src/acir/mod.rs | 2 +- test_programs/execution_success/regression_7143/Nargo.toml | 6 ++++++ test_programs/execution_success/regression_7143/Prover.toml | 3 +++ test_programs/execution_success/regression_7143/src/main.nr | 3 +++ 4 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 test_programs/execution_success/regression_7143/Nargo.toml create mode 100644 test_programs/execution_success/regression_7143/Prover.toml create mode 100644 test_programs/execution_success/regression_7143/src/main.nr diff --git a/compiler/noirc_evaluator/src/acir/mod.rs b/compiler/noirc_evaluator/src/acir/mod.rs index a3c44e055b4..6789cbafb76 100644 --- a/compiler/noirc_evaluator/src/acir/mod.rs +++ b/compiler/noirc_evaluator/src/acir/mod.rs @@ -1334,7 +1334,7 @@ impl<'a> Context<'a> { typ: &Type, ) -> Result { match typ { - Type::Numeric(_) => self.array_get_value(&Type::field(), call_data_block, offset), + Type::Numeric(_) => self.array_get_value(typ, call_data_block, offset), Type::Array(arc, len) => { let mut result = im::Vector::new(); for _i in 0..*len { diff --git a/test_programs/execution_success/regression_7143/Nargo.toml b/test_programs/execution_success/regression_7143/Nargo.toml new file mode 100644 index 00000000000..1f581c8b24d --- /dev/null +++ b/test_programs/execution_success/regression_7143/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "regression_7143" +type = "bin" +authors = [""] + +[dependencies] diff --git a/test_programs/execution_success/regression_7143/Prover.toml b/test_programs/execution_success/regression_7143/Prover.toml new file mode 100644 index 00000000000..f2f801df886 --- /dev/null +++ b/test_programs/execution_success/regression_7143/Prover.toml @@ -0,0 +1,3 @@ +array = [0] +x = 0 +return = 1 diff --git a/test_programs/execution_success/regression_7143/src/main.nr b/test_programs/execution_success/regression_7143/src/main.nr new file mode 100644 index 00000000000..396ddf1a633 --- /dev/null +++ b/test_programs/execution_success/regression_7143/src/main.nr @@ -0,0 +1,3 @@ +fn main(x: u32, array: call_data(0) [bool; 1]) -> pub bool { + !array[x] +} From b00facbbf9f9ec2b4d8887155770c6b288a7bb60 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 22 Jan 2025 06:49:33 -0500 Subject: [PATCH 05/11] fix(ssa): Use post order when mapping instructions in loop invariant pass (#7140) Co-authored-by: Tom French --- .../src/ssa/opt/loop_invariant.rs | 7 +++-- .../regression_7128/Nargo.toml | 6 +++++ .../regression_7128/Prover.toml | 1 + .../regression_7128/src/main.nr | 26 +++++++++++++++++++ .../regression_7128/Nargo.toml | 6 +++++ .../regression_7128/Prover.toml | 1 + .../regression_7128/src/main.nr | 26 +++++++++++++++++++ 7 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 test_programs/execution_failure/regression_7128/Nargo.toml create mode 100644 test_programs/execution_failure/regression_7128/Prover.toml create mode 100644 test_programs/execution_failure/regression_7128/src/main.nr create mode 100644 test_programs/execution_success/regression_7128/Nargo.toml create mode 100644 test_programs/execution_success/regression_7128/Prover.toml create mode 100644 test_programs/execution_success/regression_7128/src/main.nr diff --git a/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs b/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs index 224916c95e9..1e2e783d516 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs @@ -16,6 +16,7 @@ use crate::ssa::{ function::Function, function_inserter::FunctionInserter, instruction::{binary::eval_constant_binary_op, BinaryOp, Instruction, InstructionId}, + post_order::PostOrder, types::Type, value::ValueId, }, @@ -272,8 +273,10 @@ impl<'f> LoopInvariantContext<'f> { /// correct new value IDs based upon the `FunctionInserter` internal map. /// Leaving out this mapping could lead to instructions with values that do not exist. fn map_dependent_instructions(&mut self) { - let blocks = self.inserter.function.reachable_blocks(); - for block in blocks { + let mut block_order = PostOrder::with_function(self.inserter.function).into_vec(); + block_order.reverse(); + + for block in block_order { for instruction_id in self.inserter.function.dfg[block].take_instructions() { self.inserter.push_instruction(instruction_id, block); } diff --git a/test_programs/execution_failure/regression_7128/Nargo.toml b/test_programs/execution_failure/regression_7128/Nargo.toml new file mode 100644 index 00000000000..4d7b621526a --- /dev/null +++ b/test_programs/execution_failure/regression_7128/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "regression_7128" +type = "bin" +authors = [""] + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_failure/regression_7128/Prover.toml b/test_programs/execution_failure/regression_7128/Prover.toml new file mode 100644 index 00000000000..dd9b68d125e --- /dev/null +++ b/test_programs/execution_failure/regression_7128/Prover.toml @@ -0,0 +1 @@ +in0 = "1" diff --git a/test_programs/execution_failure/regression_7128/src/main.nr b/test_programs/execution_failure/regression_7128/src/main.nr new file mode 100644 index 00000000000..46759fe90a2 --- /dev/null +++ b/test_programs/execution_failure/regression_7128/src/main.nr @@ -0,0 +1,26 @@ +fn main(in0: Field) -> pub Field { + let mut out0: Field = 0; + let tmp1: Field = in0; + + if (out0 == out0) // <== changing out0 to in0 or removing + { + // the comparison changes the result + let in0_as_bytes: [u8; 32] = in0.to_be_bytes(); + let mut result: [u8; 32] = [0; 32]; + for i in 0..32 { + result[i] = in0_as_bytes[i]; + } + } + + let mut tmp2: Field = 0; // <== moving this to the top of main, + if (0.lt(in0)) // changes the result + { + tmp2 = 1; + } + + out0 = (tmp2 - tmp1); + + assert(out0 != 0, "soundness violation"); + + out0 +} diff --git a/test_programs/execution_success/regression_7128/Nargo.toml b/test_programs/execution_success/regression_7128/Nargo.toml new file mode 100644 index 00000000000..4d7b621526a --- /dev/null +++ b/test_programs/execution_success/regression_7128/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "regression_7128" +type = "bin" +authors = [""] + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_success/regression_7128/Prover.toml b/test_programs/execution_success/regression_7128/Prover.toml new file mode 100644 index 00000000000..dd9b68d125e --- /dev/null +++ b/test_programs/execution_success/regression_7128/Prover.toml @@ -0,0 +1 @@ +in0 = "1" diff --git a/test_programs/execution_success/regression_7128/src/main.nr b/test_programs/execution_success/regression_7128/src/main.nr new file mode 100644 index 00000000000..454c2220b88 --- /dev/null +++ b/test_programs/execution_success/regression_7128/src/main.nr @@ -0,0 +1,26 @@ +fn main(in0: Field) -> pub Field { + let mut out0: Field = 0; + let tmp1: Field = in0; + + if (out0 == out0) // <== changing out0 to in0 or removing + { + // the comparison changes the result + let in0_as_bytes: [u8; 32] = in0.to_be_bytes(); + let mut result: [u8; 32] = [0; 32]; + for i in 0..32 { + result[i] = in0_as_bytes[i]; + } + } + + let mut tmp2: Field = 0; // <== moving this to the top of main, + if (0.lt(in0)) // changes the result + { + tmp2 = 1; + } + + out0 = (tmp2 - tmp1); + + assert(out0 == 0, "completeness violation"); + + out0 +} From 02056d6a3ca2932a0062552859195a4e3b11f9dc Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Wed, 22 Jan 2025 12:18:33 +0000 Subject: [PATCH 06/11] chore: turn on overflow checks in CI rust tests (#7145) --- .github/workflows/test-rust-workspace-msrv.yml | 2 +- .github/workflows/test-rust-workspace.yml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test-rust-workspace-msrv.yml b/.github/workflows/test-rust-workspace-msrv.yml index f4fbbf79d89..38bc3cba153 100644 --- a/.github/workflows/test-rust-workspace-msrv.yml +++ b/.github/workflows/test-rust-workspace-msrv.yml @@ -52,7 +52,7 @@ jobs: tool: nextest@0.9.67 - name: Build and archive tests - run: cargo nextest archive --workspace --release --archive-file nextest-archive.tar.zst + run: cargo nextest archive --workspace --archive-file nextest-archive.tar.zst - name: Upload archive to workflow uses: actions/upload-artifact@v4 diff --git a/.github/workflows/test-rust-workspace.yml b/.github/workflows/test-rust-workspace.yml index 5d8abbc3e55..fe421361072 100644 --- a/.github/workflows/test-rust-workspace.yml +++ b/.github/workflows/test-rust-workspace.yml @@ -29,7 +29,7 @@ jobs: - uses: Swatinem/rust-cache@v2 with: - key: x86_64-unknown-linux-gnu + key: x86_64-unknown-linux-gnu-debug cache-on-failure: true save-if: ${{ github.event_name != 'merge_group' }} @@ -39,7 +39,7 @@ jobs: tool: nextest@0.9.67 - name: Build and archive tests - run: cargo nextest archive --workspace --release --archive-file nextest-archive.tar.zst + run: cargo nextest archive --workspace --archive-file nextest-archive.tar.zst - name: Upload archive to workflow uses: actions/upload-artifact@v4 From 1927bdaba6fe9ddb8f62948f13e2af84285f0ac7 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 22 Jan 2025 16:17:11 -0300 Subject: [PATCH 07/11] feat: LSP chain inlay hints (#7152) --- tooling/lsp/src/requests/inlay_hint.rs | 109 ++++++++++++++++-- tooling/lsp/src/requests/mod.rs | 18 +++ .../lsp/test_programs/inlay_hints/src/main.nr | 8 ++ 3 files changed, 128 insertions(+), 7 deletions(-) diff --git a/tooling/lsp/src/requests/inlay_hint.rs b/tooling/lsp/src/requests/inlay_hint.rs index 6629f3d0336..1798f845a31 100644 --- a/tooling/lsp/src/requests/inlay_hint.rs +++ b/tooling/lsp/src/requests/inlay_hint.rs @@ -83,29 +83,30 @@ impl<'a> InlayHintCollector<'a> { let location = Location::new(ident.span(), self.file_id); if let Some(lsp_location) = to_lsp_location(self.files, self.file_id, span) { if let Some(referenced) = self.interner.find_referenced(location) { + let include_colon = true; match referenced { ReferenceId::Global(global_id) => { let global_info = self.interner.get_global(global_id); let definition_id = global_info.definition_id; let typ = self.interner.definition_type(definition_id); - self.push_type_hint(lsp_location, &typ, editable); + self.push_type_hint(lsp_location, &typ, editable, include_colon); } ReferenceId::Local(definition_id) => { let typ = self.interner.definition_type(definition_id); - self.push_type_hint(lsp_location, &typ, editable); + self.push_type_hint(lsp_location, &typ, editable, include_colon); } ReferenceId::StructMember(struct_id, field_index) => { let struct_type = self.interner.get_type(struct_id); let struct_type = struct_type.borrow(); let field = struct_type.field_at(field_index); - self.push_type_hint(lsp_location, &field.typ, false); + self.push_type_hint(lsp_location, &field.typ, false, include_colon); } ReferenceId::EnumVariant(type_id, variant_index) => { let typ = self.interner.get_type(type_id); let shared_type = typ.clone(); let typ = typ.borrow(); let variant_type = typ.variant_function_type(variant_index, shared_type); - self.push_type_hint(lsp_location, &variant_type, false); + self.push_type_hint(lsp_location, &variant_type, false, include_colon); } ReferenceId::Module(_) | ReferenceId::Struct(_) @@ -119,11 +120,21 @@ impl<'a> InlayHintCollector<'a> { } } - fn push_type_hint(&mut self, location: lsp_types::Location, typ: &Type, editable: bool) { + fn push_type_hint( + &mut self, + location: lsp_types::Location, + typ: &Type, + editable: bool, + include_colon: bool, + ) { let position = location.range.end; let mut parts = Vec::new(); - parts.push(string_part(": ")); + if include_colon { + parts.push(string_part(": ")); + } else { + parts.push(string_part(" ")); + } push_type_parts(typ, &mut parts, self.files); self.inlay_hints.push(InlayHint { @@ -217,6 +228,36 @@ impl<'a> InlayHintCollector<'a> { } } + fn collect_method_call_chain_hints(&mut self, method: &MethodCallExpression) { + let Some(object_lsp_location) = + to_lsp_location(self.files, self.file_id, method.object.span) + else { + return; + }; + + let Some(name_lsp_location) = + to_lsp_location(self.files, self.file_id, method.method_name.span()) + else { + return; + }; + + if object_lsp_location.range.end.line >= name_lsp_location.range.start.line { + return; + } + + let object_location = Location::new(method.object.span, self.file_id); + let Some(typ) = self.interner.type_at_location(object_location) else { + return; + }; + + self.push_type_hint( + object_lsp_location, + &typ, + false, // not editable + false, // don't include colon + ); + } + fn get_pattern_name(&self, pattern: &HirPattern) -> Option { match pattern { HirPattern::Identifier(ident) => { @@ -357,6 +398,10 @@ impl<'a> Visitor for InlayHintCollector<'a> { &method_call_expression.arguments, ); + if self.options.chaining_hints.enabled { + self.collect_method_call_chain_hints(method_call_expression); + } + true } @@ -548,7 +593,9 @@ fn get_expression_name(expression: &Expression) -> Option { #[cfg(test)] mod inlay_hints_tests { use crate::{ - requests::{ClosingBraceHintsOptions, ParameterHintsOptions, TypeHintsOptions}, + requests::{ + ChainingHintsOptions, ClosingBraceHintsOptions, ParameterHintsOptions, TypeHintsOptions, + }, test_utils, }; @@ -585,6 +632,7 @@ mod inlay_hints_tests { type_hints: TypeHintsOptions { enabled: false }, parameter_hints: ParameterHintsOptions { enabled: false }, closing_brace_hints: ClosingBraceHintsOptions { enabled: false, min_lines: 25 }, + chaining_hints: ChainingHintsOptions { enabled: false }, } } @@ -593,6 +641,7 @@ mod inlay_hints_tests { type_hints: TypeHintsOptions { enabled: true }, parameter_hints: ParameterHintsOptions { enabled: false }, closing_brace_hints: ClosingBraceHintsOptions { enabled: false, min_lines: 25 }, + chaining_hints: ChainingHintsOptions { enabled: false }, } } @@ -601,6 +650,7 @@ mod inlay_hints_tests { type_hints: TypeHintsOptions { enabled: false }, parameter_hints: ParameterHintsOptions { enabled: true }, closing_brace_hints: ClosingBraceHintsOptions { enabled: false, min_lines: 25 }, + chaining_hints: ChainingHintsOptions { enabled: false }, } } @@ -609,6 +659,16 @@ mod inlay_hints_tests { type_hints: TypeHintsOptions { enabled: false }, parameter_hints: ParameterHintsOptions { enabled: false }, closing_brace_hints: ClosingBraceHintsOptions { enabled: true, min_lines }, + chaining_hints: ChainingHintsOptions { enabled: false }, + } + } + + fn chaining_hints() -> InlayHintsOptions { + InlayHintsOptions { + type_hints: TypeHintsOptions { enabled: false }, + parameter_hints: ParameterHintsOptions { enabled: false }, + closing_brace_hints: ClosingBraceHintsOptions { enabled: false, min_lines: 0 }, + chaining_hints: ChainingHintsOptions { enabled: true }, } } @@ -963,4 +1023,39 @@ mod inlay_hints_tests { panic!("Expected InlayHintLabel::String, got {:?}", inlay_hint.label); } } + + #[test] + async fn test_shows_receiver_type_in_multiline_method_call() { + let mut inlay_hints = get_inlay_hints(125, 130, chaining_hints()).await; + assert_eq!(inlay_hints.len(), 3); + + inlay_hints.sort_by_key(|hint| hint.position.line); + + let inlay_hint = &inlay_hints[0]; + assert_eq!(inlay_hint.position.line, 125); + assert_eq!(inlay_hint.position.character, 59); + let InlayHintLabel::LabelParts(parts) = &inlay_hint.label else { + panic!("Expected label parts"); + }; + let label = parts.iter().map(|part| part.value.clone()).collect::>().join(""); + assert_eq!(label, " [u32; 14]"); + + let inlay_hint = &inlay_hints[1]; + assert_eq!(inlay_hint.position.line, 126); + assert_eq!(inlay_hint.position.character, 37); + let InlayHintLabel::LabelParts(parts) = &inlay_hint.label else { + panic!("Expected label parts"); + }; + let label = parts.iter().map(|part| part.value.clone()).collect::>().join(""); + assert_eq!(label, " [u32; 14]"); + + let inlay_hint = &inlay_hints[2]; + assert_eq!(inlay_hint.position.line, 127); + assert_eq!(inlay_hint.position.character, 23); + let InlayHintLabel::LabelParts(parts) = &inlay_hint.label else { + panic!("Expected label parts"); + }; + let label = parts.iter().map(|part| part.value.clone()).collect::>().join(""); + assert_eq!(label, " bool"); + } } diff --git a/tooling/lsp/src/requests/mod.rs b/tooling/lsp/src/requests/mod.rs index 80f4a167a04..334599e8f3d 100644 --- a/tooling/lsp/src/requests/mod.rs +++ b/tooling/lsp/src/requests/mod.rs @@ -90,6 +90,9 @@ pub(crate) struct InlayHintsOptions { #[serde(rename = "closingBraceHints", default = "default_closing_brace_hints")] pub(crate) closing_brace_hints: ClosingBraceHintsOptions, + + #[serde(rename = "ChainingHints", default = "default_chaining_hints")] + pub(crate) chaining_hints: ChainingHintsOptions, } #[derive(Debug, Deserialize, Serialize, Copy, Clone)] @@ -113,6 +116,12 @@ pub(crate) struct ClosingBraceHintsOptions { pub(crate) min_lines: u32, } +#[derive(Debug, Deserialize, Serialize, Copy, Clone)] +pub(crate) struct ChainingHintsOptions { + #[serde(rename = "enabled", default = "default_chaining_hints_enabled")] + pub(crate) enabled: bool, +} + fn default_enable_code_lens() -> bool { true } @@ -126,6 +135,7 @@ fn default_inlay_hints() -> InlayHintsOptions { type_hints: default_type_hints(), parameter_hints: default_parameter_hints(), closing_brace_hints: default_closing_brace_hints(), + chaining_hints: default_chaining_hints(), } } @@ -160,6 +170,14 @@ fn default_closing_brace_min_lines() -> u32 { 25 } +fn default_chaining_hints() -> ChainingHintsOptions { + ChainingHintsOptions { enabled: default_chaining_hints_enabled() } +} + +fn default_chaining_hints_enabled() -> bool { + true +} + impl Default for LspInitializationOptions { fn default() -> Self { Self { diff --git a/tooling/lsp/test_programs/inlay_hints/src/main.nr b/tooling/lsp/test_programs/inlay_hints/src/main.nr index 46a6d3bc558..64eca72a667 100644 --- a/tooling/lsp/test_programs/inlay_hints/src/main.nr +++ b/tooling/lsp/test_programs/inlay_hints/src/main.nr @@ -119,4 +119,12 @@ mod some_module { contract some_contract { +}} + +use std::ops::Not; +pub fn chain() { + let _ = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14] + .map(|x| x + 123456789012345) + .any(|x| x > 5) + .not(); } From d3ca3b711d358e0ac98410121d3c798b9327523b Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 22 Jan 2025 14:19:13 -0500 Subject: [PATCH 08/11] feat(ssa): Reuse constants from the globals graph when making constants in a function DFG (#7153) --- compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index 83b8f2a57ff..1bbd407cf0f 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -116,11 +116,14 @@ pub(crate) struct GlobalsGraph { /// All of the instructions in the global value space. /// These are expected to all be Instruction::MakeArray instructions: DenseMap, + + #[serde(skip)] + constants: HashMap<(FieldElement, NumericType), ValueId>, } impl GlobalsGraph { pub(crate) fn from_dfg(dfg: DataFlowGraph) -> Self { - Self { values: dfg.values, instructions: dfg.instructions } + Self { values: dfg.values, instructions: dfg.instructions, constants: dfg.constants } } /// Iterate over every Value in this DFG in no particular order, including unused Values @@ -386,6 +389,9 @@ impl DataFlowGraph { if let Some(id) = self.constants.get(&(constant, typ)) { return *id; } + if let Some(id) = self.globals.constants.get(&(constant, typ)) { + return *id; + } let id = self.values.insert(Value::NumericConstant { constant, typ }); self.constants.insert((constant, typ), id); id From 1f1e7508e024d187c6a69501e7459a613e381b4a Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 22 Jan 2025 16:34:01 -0300 Subject: [PATCH 09/11] feat: LSP autocomplete module declaration (#7154) --- compiler/fm/src/file_map.rs | 11 +++- compiler/noirc_frontend/src/ast/statement.rs | 1 + .../src/parser/parser/module.rs | 11 +++- tooling/lsp/src/requests/completion.rs | 64 +++++++++++++++++-- 4 files changed, 80 insertions(+), 7 deletions(-) diff --git a/compiler/fm/src/file_map.rs b/compiler/fm/src/file_map.rs index 857c7460fb9..f078ecb8545 100644 --- a/compiler/fm/src/file_map.rs +++ b/compiler/fm/src/file_map.rs @@ -19,6 +19,10 @@ impl PathString { pub fn from_path(p: PathBuf) -> Self { PathString(p) } + + pub fn into_path_buf(self) -> PathBuf { + self.0 + } } impl From for PathString { fn from(pb: PathBuf) -> PathString { @@ -82,7 +86,7 @@ impl FileMap { } pub fn get_name(&self, file_id: FileId) -> Result { - let name = self.files.get(file_id.as_usize())?.name().clone(); + let name = self.get_absolute_name(file_id)?; // See if we can make the file name a bit shorter/easier to read if it starts with the current directory if let Some(current_dir) = &self.current_dir { @@ -93,6 +97,11 @@ impl FileMap { Ok(name) } + + pub fn get_absolute_name(&self, file_id: FileId) -> Result { + let name = self.files.get(file_id.as_usize())?.name().clone(); + Ok(name) + } } impl Default for FileMap { fn default() -> Self { diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index 7695de14d69..02715e8c2d3 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -308,6 +308,7 @@ pub struct ModuleDeclaration { pub visibility: ItemVisibility, pub ident: Ident, pub outer_attributes: Vec, + pub has_semicolon: bool, } impl std::fmt::Display for ModuleDeclaration { diff --git a/compiler/noirc_frontend/src/parser/parser/module.rs b/compiler/noirc_frontend/src/parser/parser/module.rs index da733168099..1bc3d7b5beb 100644 --- a/compiler/noirc_frontend/src/parser/parser/module.rs +++ b/compiler/noirc_frontend/src/parser/parser/module.rs @@ -25,6 +25,7 @@ impl<'a> Parser<'a> { visibility, ident: Ident::default(), outer_attributes, + has_semicolon: false, }); }; @@ -41,10 +42,16 @@ impl<'a> Parser<'a> { is_contract, }) } else { - if !self.eat_semicolons() { + let has_semicolon = self.eat_semicolons(); + if !has_semicolon { self.expected_token(Token::Semicolon); } - ItemKind::ModuleDecl(ModuleDeclaration { visibility, ident, outer_attributes }) + ItemKind::ModuleDecl(ModuleDeclaration { + visibility, + ident, + outer_attributes, + has_semicolon, + }) } } } diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 597b778260a..9948a29691e 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -18,9 +18,9 @@ use noirc_frontend::{ AsTraitPath, AttributeTarget, BlockExpression, CallExpression, ConstructorExpression, Expression, ExpressionKind, ForLoopStatement, GenericTypeArgs, Ident, IfExpression, IntegerBitSize, ItemVisibility, LValue, Lambda, LetStatement, MemberAccessExpression, - MethodCallExpression, NoirFunction, NoirStruct, NoirTraitImpl, Path, PathKind, Pattern, - Signedness, Statement, TraitBound, TraitImplItemKind, TypeImpl, TypePath, - UnresolvedGeneric, UnresolvedGenerics, UnresolvedType, UnresolvedTypeData, + MethodCallExpression, ModuleDeclaration, NoirFunction, NoirStruct, NoirTraitImpl, Path, + PathKind, Pattern, Signedness, Statement, TraitBound, TraitImplItemKind, TypeImpl, + TypePath, UnresolvedGeneric, UnresolvedGenerics, UnresolvedType, UnresolvedTypeData, UnresolvedTypeExpression, UseTree, UseTreeKind, Visitor, }, graph::{CrateId, Dependency}, @@ -1111,7 +1111,55 @@ impl<'a> NodeFinder<'a> { } } - /// Determine where each segment in a `use` statement is located. + /// Try to suggest the name of a module to declare based on which + /// files exist in the filesystem, excluding modules that are already declared. + fn complete_module_delcaration(&mut self, module: &ModuleDeclaration) -> Option<()> { + let filename = self.files.get_absolute_name(self.file).ok()?.into_path_buf(); + + let is_main_lib_or_mod = filename.ends_with("main.nr") + || filename.ends_with("lib.nr") + || filename.ends_with("mod.nr"); + + let paths = if is_main_lib_or_mod { + // For a "main" file we list sibling files + std::fs::read_dir(filename.parent()?) + } else { + // For a non-main files we list directory children + std::fs::read_dir(filename.with_extension("")) + }; + let paths = paths.ok()?; + + // See which modules are already defined via `mod ...;` + let module_data = + &self.def_maps[&self.module_id.krate].modules()[self.module_id.local_id.0]; + let existing_children: HashSet = + module_data.children.keys().map(|ident| ident.to_string()).collect(); + + for path in paths { + let Ok(path) = path else { + continue; + }; + let file_name = path.file_name().to_string_lossy().to_string(); + let Some(name) = file_name.strip_suffix(".nr") else { + continue; + }; + if name == "main" || name == "mod" || name == "lib" { + continue; + } + if existing_children.contains(name) { + continue; + } + + let label = if module.has_semicolon { name.to_string() } else { format!("{};", name) }; + self.completion_items.push(simple_completion_item( + label, + CompletionItemKind::MODULE, + None, + )); + } + + Some(()) + } fn includes_span(&self, span: Span) -> bool { span.start() as usize <= self.byte_index && self.byte_index <= span.end() as usize @@ -1795,6 +1843,14 @@ impl<'a> Visitor for NodeFinder<'a> { trait_bound.trait_generics.accept(self); false } + + fn visit_module_declaration(&mut self, module: &ModuleDeclaration, _: Span) { + if !self.includes_span(module.ident.span()) { + return; + } + + self.complete_module_delcaration(module); + } } fn get_field_type(typ: &Type, name: &str) -> Option { From 7f9525dfdb4036befbd21b0d2546b83884593aa4 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 22 Jan 2025 15:25:05 -0500 Subject: [PATCH 10/11] feat(brillig): Set global memory size at program compile time (#7151) --- .../src/brillig/brillig_gen.rs | 1 + .../brillig/brillig_gen/brillig_globals.rs | 6 ++-- .../noirc_evaluator/src/brillig/brillig_ir.rs | 11 +++++++ .../src/brillig/brillig_ir/entry_point.rs | 27 +++++++++------- .../src/brillig/brillig_ir/registers.rs | 31 ++++++++++++++----- compiler/noirc_evaluator/src/brillig/mod.rs | 4 ++- .../noirc_evaluator/src/ssa/opt/unrolling.rs | 2 +- cspell.json | 1 + 8 files changed, 61 insertions(+), 22 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen.rs index b51a3445a1b..a6117a8f2da 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen.rs @@ -63,6 +63,7 @@ pub(crate) fn gen_brillig_for( FunctionContext::return_values(func), func.id(), true, + brillig.globals_memory_size, ); entry_point.name = func.name().to_string(); diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_globals.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_globals.rs index 99c8ee0fded..f3ce79dd017 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_globals.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_globals.rs @@ -10,7 +10,7 @@ pub(crate) fn convert_ssa_globals( enable_debug_trace: bool, globals: &Function, used_globals: &HashSet, -) -> (BrilligArtifact, HashMap) { +) -> (BrilligArtifact, HashMap, usize) { let mut brillig_context = BrilligContext::new_for_global_init(enable_debug_trace); // The global space does not have globals itself let empty_globals = HashMap::default(); @@ -32,8 +32,10 @@ pub(crate) fn convert_ssa_globals( brillig_block.compile_globals(&globals.dfg, used_globals); + let globals_size = brillig_block.brillig_context.global_space_size(); + brillig_context.return_instruction(); let artifact = brillig_context.artifact(); - (artifact, function_context.ssa_value_allocations) + (artifact, function_context.ssa_value_allocations, globals_size) } diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir.rs index 06f61948337..ad09f73e90f 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir.rs @@ -95,6 +95,8 @@ pub(crate) struct BrilligContext { /// Whether this context can call procedures or not. /// This is used to prevent a procedure from calling another procedure. can_call_procedures: bool, + + globals_memory_size: Option, } /// Regular brillig context to codegen user defined functions @@ -108,6 +110,7 @@ impl BrilligContext { next_section: 1, debug_show: DebugShow::new(enable_debug_trace), can_call_procedures: true, + globals_memory_size: None, } } } @@ -211,6 +214,7 @@ impl BrilligContext { next_section: 1, debug_show: DebugShow::new(enable_debug_trace), can_call_procedures: false, + globals_memory_size: None, } } } @@ -226,8 +230,14 @@ impl BrilligContext { next_section: 1, debug_show: DebugShow::new(enable_debug_trace), can_call_procedures: false, + globals_memory_size: None, } } + + pub(crate) fn global_space_size(&self) -> usize { + // `GlobalSpace::start()` is inclusive so we must add one to get the accurate total global memory size + (self.registers.max_memory_address() + 1) - GlobalSpace::start() + } } impl BrilligContext { @@ -321,6 +331,7 @@ pub(crate) mod tests { returns, FunctionId::test_new(0), false, + 0, ); entry_point_artifact.link_with(&artifact); while let Some(unresolved_fn_label) = entry_point_artifact.first_unresolved_function_call() diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs index b84a15db4ad..030ed7133e8 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs @@ -15,7 +15,6 @@ use acvm::acir::{ pub(crate) const MAX_STACK_SIZE: usize = 16 * MAX_STACK_FRAME_SIZE; pub(crate) const MAX_STACK_FRAME_SIZE: usize = 2048; pub(crate) const MAX_SCRATCH_SPACE: usize = 64; -pub(crate) const MAX_GLOBAL_SPACE: usize = 16384; impl BrilligContext { /// Creates an entry point artifact that will jump to the function label provided. @@ -24,9 +23,12 @@ impl BrilligContext { return_parameters: Vec, target_function: FunctionId, globals_init: bool, + globals_memory_size: usize, ) -> BrilligArtifact { let mut context = BrilligContext::new(false); + context.globals_memory_size = Some(globals_memory_size); + context.codegen_entry_point(&arguments, &return_parameters); if globals_init { @@ -39,12 +41,15 @@ impl BrilligContext { context.artifact() } - fn calldata_start_offset() -> usize { - ReservedRegisters::len() + MAX_STACK_SIZE + MAX_SCRATCH_SPACE + MAX_GLOBAL_SPACE + fn calldata_start_offset(&self) -> usize { + ReservedRegisters::len() + + MAX_STACK_SIZE + + MAX_SCRATCH_SPACE + + self.globals_memory_size.expect("The memory size of globals should be set") } - fn return_data_start_offset(calldata_size: usize) -> usize { - Self::calldata_start_offset() + calldata_size + fn return_data_start_offset(&self, calldata_size: usize) -> usize { + self.calldata_start_offset() + calldata_size } /// Adds the instructions needed to handle entry point parameters @@ -70,7 +75,7 @@ impl BrilligContext { // Set initial value of free memory pointer: calldata_start_offset + calldata_size + return_data_size self.const_instruction( SingleAddrVariable::new_usize(ReservedRegisters::free_memory_pointer()), - (Self::calldata_start_offset() + calldata_size + return_data_size).into(), + (self.calldata_start_offset() + calldata_size + return_data_size).into(), ); // Set initial value of stack pointer: ReservedRegisters.len() @@ -82,7 +87,7 @@ impl BrilligContext { // Copy calldata self.copy_and_cast_calldata(arguments); - let mut current_calldata_pointer = Self::calldata_start_offset(); + let mut current_calldata_pointer = self.calldata_start_offset(); // Initialize the variables with the calldata for (argument_variable, argument) in argument_variables.iter_mut().zip(arguments) { @@ -158,7 +163,7 @@ impl BrilligContext { fn copy_and_cast_calldata(&mut self, arguments: &[BrilligParameter]) { let calldata_size = Self::flattened_tuple_size(arguments); self.calldata_copy_instruction( - MemoryAddress::direct(Self::calldata_start_offset()), + MemoryAddress::direct(self.calldata_start_offset()), calldata_size, 0, ); @@ -178,11 +183,11 @@ impl BrilligContext { if bit_size < F::max_num_bits() { self.cast_instruction( SingleAddrVariable::new( - MemoryAddress::direct(Self::calldata_start_offset() + i), + MemoryAddress::direct(self.calldata_start_offset() + i), bit_size, ), SingleAddrVariable::new_field(MemoryAddress::direct( - Self::calldata_start_offset() + i, + self.calldata_start_offset() + i, )), ); } @@ -336,7 +341,7 @@ impl BrilligContext { let return_data_size = Self::flattened_tuple_size(return_parameters); // Return data has a reserved space after calldata - let return_data_offset = Self::return_data_start_offset(calldata_size); + let return_data_offset = self.return_data_start_offset(calldata_size); let mut return_data_index = return_data_offset; for (return_param, returned_variable) in return_parameters.iter().zip(&returned_variables) { diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/registers.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/registers.rs index b83c03b297a..88b8a598b10 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/registers.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/registers.rs @@ -7,7 +7,7 @@ use crate::brillig::brillig_ir::entry_point::MAX_STACK_SIZE; use super::{ brillig_variable::SingleAddrVariable, - entry_point::{MAX_GLOBAL_SPACE, MAX_SCRATCH_SPACE, MAX_STACK_FRAME_SIZE}, + entry_point::{MAX_SCRATCH_SPACE, MAX_STACK_FRAME_SIZE}, BrilligContext, ReservedRegisters, }; @@ -152,16 +152,32 @@ impl RegisterAllocator for ScratchSpace { /// and is read-only. pub(crate) struct GlobalSpace { storage: DeallocationListAllocator, + max_memory_address: usize, } impl GlobalSpace { - pub(crate) fn new() -> Self { - Self { storage: DeallocationListAllocator::new(Self::start()) } + pub(super) fn new() -> Self { + Self { + storage: DeallocationListAllocator::new(Self::start()), + max_memory_address: Self::start(), + } } fn is_within_bounds(register: MemoryAddress) -> bool { let index = register.unwrap_direct(); - index >= Self::start() && index < Self::end() + index >= Self::start() + } + + fn update_max_address(&mut self, register: MemoryAddress) { + let index = register.unwrap_direct(); + assert!(index >= Self::start(), "Global space malformed"); + if index > self.max_memory_address { + self.max_memory_address = index; + } + } + + pub(super) fn max_memory_address(&self) -> usize { + self.max_memory_address } } @@ -171,12 +187,12 @@ impl RegisterAllocator for GlobalSpace { } fn end() -> usize { - Self::start() + MAX_GLOBAL_SPACE + unreachable!("The global space is set by the program"); } fn allocate_register(&mut self) -> MemoryAddress { let allocated = MemoryAddress::direct(self.storage.allocate_register()); - assert!(Self::is_within_bounds(allocated), "Global space too deep"); + self.update_max_address(allocated); allocated } @@ -185,7 +201,7 @@ impl RegisterAllocator for GlobalSpace { } fn ensure_register_is_allocated(&mut self, register: MemoryAddress) { - assert!(Self::is_within_bounds(register), "Register out of global space bounds"); + self.update_max_address(register); self.storage.ensure_register_is_allocated(register.unwrap_direct()); } @@ -199,6 +215,7 @@ impl RegisterAllocator for GlobalSpace { Self::start(), vecmap(preallocated_registers, |r| r.unwrap_direct()), ), + max_memory_address: Self::start(), } } diff --git a/compiler/noirc_evaluator/src/brillig/mod.rs b/compiler/noirc_evaluator/src/brillig/mod.rs index 3d96a855aa0..8a803c72b67 100644 --- a/compiler/noirc_evaluator/src/brillig/mod.rs +++ b/compiler/noirc_evaluator/src/brillig/mod.rs @@ -32,6 +32,7 @@ pub struct Brillig { /// Maps SSA function labels to their brillig artifact ssa_function_to_brillig: HashMap>, globals: BrilligArtifact, + globals_memory_size: usize, } impl Brillig { @@ -84,9 +85,10 @@ impl Ssa { let mut brillig = Brillig::default(); - let (artifact, brillig_globals) = + let (artifact, brillig_globals, globals_size) = convert_ssa_globals(enable_debug_trace, &self.globals, &self.used_global_values); brillig.globals = artifact; + brillig.globals_memory_size = globals_size; for brillig_function_id in brillig_reachable_function_ids { let func = &self.functions[&brillig_function_id]; diff --git a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs index a6e5c96d638..d0f77d3cee1 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs @@ -82,7 +82,7 @@ impl Ssa { // DIE is run at the end of our SSA optimizations, so we mark all globals as in use here. let used_globals = &self.globals.dfg.values_iter().map(|(id, _)| id).collect(); - let (_, brillig_globals) = + let (_, brillig_globals, _) = convert_ssa_globals(false, &self.globals, used_globals); global_cache = Some(brillig_globals); } diff --git a/cspell.json b/cspell.json index 25a0cc91f52..1174a56dd33 100644 --- a/cspell.json +++ b/cspell.json @@ -32,6 +32,7 @@ "boilerplates", "bridgekeeper", "brillig", + "brillig_", "bunx", "bytecount", "cachix", From c8d5ce5e72f3171f24e4fe2b94ac098ba6d99ef6 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 22 Jan 2025 18:09:41 -0300 Subject: [PATCH 11/11] fix: LSP hover over function with `&mut self` (#7155) --- tooling/lsp/src/requests/hover.rs | 20 ++++++++++++++++++- .../test_programs/workspace/two/src/lib.nr | 3 +++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index bfe2a2a2448..8d845dce13d 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -491,14 +491,24 @@ fn format_function(id: FuncId, args: &ProcessRequestCallbackArgs) -> String { string.push('('); let parameters = &func_meta.parameters; for (index, (pattern, typ, visibility)) in parameters.iter().enumerate() { + let is_self = pattern_is_self(pattern, args.interner); + + // `&mut self` is represented as a mutable reference type, not as a mutable pattern + if is_self && matches!(typ, Type::MutableReference(..)) { + string.push_str("&mut "); + } + format_pattern(pattern, args.interner, &mut string); - if !pattern_is_self(pattern, args.interner) { + + // Don't add type for `self` param + if !is_self { string.push_str(": "); if matches!(visibility, Visibility::Public) { string.push_str("pub "); } string.push_str(&format!("{}", typ)); } + if index != parameters.len() - 1 { string.push_str(", "); } @@ -1238,4 +1248,12 @@ mod hover_tests { .await; assert!(hover_text.contains("Some docs")); } + + #[test] + async fn hover_on_function_with_mut_self() { + let hover_text = + get_hover_text("workspace", "two/src/lib.nr", Position { line: 96, character: 10 }) + .await; + assert!(hover_text.contains("fn mut_self(&mut self)")); + } } diff --git a/tooling/lsp/test_programs/workspace/two/src/lib.nr b/tooling/lsp/test_programs/workspace/two/src/lib.nr index d18a663b276..aacc4508756 100644 --- a/tooling/lsp/test_programs/workspace/two/src/lib.nr +++ b/tooling/lsp/test_programs/workspace/two/src/lib.nr @@ -93,3 +93,6 @@ impl TraitWithDocs for Field { fn foo() {} } +impl Foo { + fn mut_self(&mut self) {} +}