From 92a4c5dea635013bb412266642b141211548bc26 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Thu, 14 Nov 2024 14:06:40 +0000 Subject: [PATCH] chore: pull frontend changes from sync PR (#9935) Please read [contributing guidelines](CONTRIBUTING.md) and remove this line. --- noir/noir-repo/Cargo.lock | 15 +- .../compiler/noirc_driver/src/abi_gen.rs | 20 +- .../compiler/noirc_driver/src/lib.rs | 5 +- .../compiler/noirc_evaluator/src/ssa.rs | 4 +- .../src/ssa/function_builder/data_bus.rs | 2 +- .../compiler/noirc_frontend/Cargo.toml | 2 + .../hir_def/types/arithmetic.txt | 7 + .../tests/arithmetic_generics.txt | 7 + .../noirc_frontend/src/ast/expression.rs | 4 +- .../noirc_frontend/src/ast/function.rs | 4 +- .../compiler/noirc_frontend/src/ast/mod.rs | 4 + .../noirc_frontend/src/ast/statement.rs | 10 +- .../noirc_frontend/src/ast/visitor.rs | 21 +- .../noirc_frontend/src/elaborator/comptime.rs | 62 +- .../noirc_frontend/src/elaborator/lints.rs | 8 +- .../noirc_frontend/src/elaborator/mod.rs | 28 +- .../src/elaborator/path_resolution.rs | 347 +++++++++ .../noirc_frontend/src/elaborator/patterns.rs | 4 +- .../noirc_frontend/src/elaborator/scope.rs | 101 +-- .../noirc_frontend/src/elaborator/traits.rs | 2 + .../noirc_frontend/src/elaborator/types.rs | 40 +- .../src/hir/comptime/display.rs | 3 +- .../noirc_frontend/src/hir/comptime/errors.rs | 20 +- .../src/hir/comptime/hir_to_display_ast.rs | 1 + .../src/hir/comptime/interpreter.rs | 46 +- .../src/hir/comptime/interpreter/builtin.rs | 147 ++-- .../src/hir/def_collector/dc_crate.rs | 103 +-- .../src/hir/def_collector/dc_mod.rs | 3 +- .../noirc_frontend/src/hir/def_map/mod.rs | 2 +- .../src/hir/resolution/errors.rs | 13 +- .../src/hir/resolution/import.rs | 706 ++++++------------ .../noirc_frontend/src/hir/resolution/mod.rs | 1 - .../src/hir/resolution/path_resolver.rs | 102 --- .../src/hir/resolution/visibility.rs | 46 +- .../src/hir/type_check/errors.rs | 48 +- .../noirc_frontend/src/hir_def/function.rs | 5 +- .../noirc_frontend/src/hir_def/stmt.rs | 2 +- .../noirc_frontend/src/hir_def/types.rs | 238 ++++-- .../src/hir_def/types/arithmetic.rs | 478 ++++++++++-- .../noirc_frontend/src/lexer/lexer.rs | 198 +---- .../noirc_frontend/src/lexer/token.rs | 252 ++----- .../compiler/noirc_frontend/src/locations.rs | 59 +- .../src/monomorphization/ast.rs | 12 +- .../src/monomorphization/errors.rs | 18 +- .../src/monomorphization/mod.rs | 121 ++- .../noirc_frontend/src/node_interner.rs | 1 + .../noirc_frontend/src/parser/errors.rs | 9 + .../noirc_frontend/src/parser/parser.rs | 24 + .../src/parser/parser/attributes.rs | 585 ++++++++++++++- .../src/parser/parser/function.rs | 11 +- .../compiler/noirc_frontend/src/tests.rs | 125 +++- .../src/tests/arithmetic_generics.rs | 155 ++++ .../noirc_frontend/src/tests/imports.rs | 27 + .../noirc_frontend/src/tests/traits.rs | 18 +- .../noirc_frontend/src/tests/unused_items.rs | 20 + noir/noir-repo/compiler/wasm/src/compile.rs | 14 +- .../compiler/wasm/src/compile_new.rs | 14 +- noir/noir-repo/noir_stdlib/src/ec/tecurve.nr | 2 +- .../attribute_args/src/main.nr | 8 +- .../lsp/src/attribute_reference_finder.rs | 70 +- .../tooling/lsp/src/requests/completion.rs | 53 +- .../lsp/src/requests/completion/builtins.rs | 9 + .../lsp/src/requests/completion/tests.rs | 4 +- .../lsp/src/requests/goto_definition.rs | 1 - .../tooling/lsp/src/requests/hover.rs | 2 +- .../tooling/lsp/src/requests/inlay_hint.rs | 1 + .../src/trait_impl_method_stub_generator.rs | 1 + noir/noir-repo/tooling/lsp/src/visibility.rs | 27 +- noir/noir-repo/tooling/nargo_cli/build.rs | 14 +- .../tooling/nargo_cli/src/cli/compile_cmd.rs | 2 +- .../noir-repo/tooling/nargo_fmt/src/chunks.rs | 11 +- .../tooling/nargo_fmt/src/formatter.rs | 9 +- .../nargo_fmt/src/formatter/attribute.rs | 346 ++++++++- .../tooling/nargo_fmt/src/formatter/buffer.rs | 4 +- .../src/formatter/comments_and_whitespace.rs | 3 +- .../nargo_fmt/src/formatter/expression.rs | 22 +- .../nargo_fmt/src/formatter/function.rs | 64 +- .../tooling/nargo_fmt/src/formatter/item.rs | 2 +- .../tooling/nargo_fmt/src/formatter/module.rs | 8 +- .../nargo_fmt/src/formatter/statement.rs | 26 +- .../nargo_fmt/src/formatter/structs.rs | 4 +- .../tooling/nargo_fmt/src/formatter/traits.rs | 7 +- .../src/formatter/type_expression.rs | 6 +- .../tooling/nargo_fmt/src/formatter/types.rs | 6 +- .../nargo_fmt/src/formatter/visibility.rs | 3 +- .../nargo_fmt/tests/expected/parens.nr | 1 + 86 files changed, 3264 insertions(+), 1776 deletions(-) create mode 100644 noir/noir-repo/compiler/noirc_frontend/proptest-regressions/hir_def/types/arithmetic.txt create mode 100644 noir/noir-repo/compiler/noirc_frontend/proptest-regressions/tests/arithmetic_generics.txt create mode 100644 noir/noir-repo/compiler/noirc_frontend/src/elaborator/path_resolution.rs delete mode 100644 noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs create mode 100644 noir/noir-repo/compiler/noirc_frontend/src/tests/arithmetic_generics.rs diff --git a/noir/noir-repo/Cargo.lock b/noir/noir-repo/Cargo.lock index 68a604df378..35ff97f55e3 100644 --- a/noir/noir-repo/Cargo.lock +++ b/noir/noir-repo/Cargo.lock @@ -3038,7 +3038,7 @@ dependencies = [ "num-bigint", "num-traits", "proptest", - "proptest-derive", + "proptest-derive 0.4.0", "serde", "serde_json", "strum", @@ -3161,6 +3161,8 @@ dependencies = [ "num-bigint", "num-traits", "petgraph", + "proptest", + "proptest-derive 0.5.0", "rangemap", "regex", "rustc-hash", @@ -3725,6 +3727,17 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "proptest-derive" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6ff7ff745a347b87471d859a377a9a404361e7efc2a971d73424a6d183c0fc77" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.87", +] + [[package]] name = "quick-error" version = "1.2.3" diff --git a/noir/noir-repo/compiler/noirc_driver/src/abi_gen.rs b/noir/noir-repo/compiler/noirc_driver/src/abi_gen.rs index bf64c9c920c..625a35c8d15 100644 --- a/noir/noir-repo/compiler/noirc_driver/src/abi_gen.rs +++ b/noir/noir-repo/compiler/noirc_driver/src/abi_gen.rs @@ -6,6 +6,7 @@ use iter_extended::vecmap; use noirc_abi::{ Abi, AbiErrorType, AbiParameter, AbiReturnType, AbiType, AbiValue, AbiVisibility, Sign, }; +use noirc_errors::Span; use noirc_evaluator::ErrorType; use noirc_frontend::ast::{Signedness, Visibility}; use noirc_frontend::TypeBinding; @@ -40,11 +41,21 @@ pub(super) fn gen_abi( Abi { parameters, return_type, error_types } } +// Get the Span of the root crate's main function, or else a dummy span if that fails +fn get_main_function_span(context: &Context) -> Span { + if let Some(func_id) = context.get_main_function(context.root_crate_id()) { + context.function_meta(&func_id).location.span + } else { + Span::default() + } +} + fn build_abi_error_type(context: &Context, typ: ErrorType) -> AbiErrorType { match typ { ErrorType::Dynamic(typ) => { if let Type::FmtString(len, item_types) = typ { - let length = len.evaluate_to_u32().expect("Cannot evaluate fmt length"); + let span = get_main_function_span(context); + let length = len.evaluate_to_u32(span).expect("Cannot evaluate fmt length"); let Type::Tuple(item_types) = item_types.as_ref() else { unreachable!("FmtString items must be a tuple") }; @@ -63,8 +74,9 @@ pub(super) fn abi_type_from_hir_type(context: &Context, typ: &Type) -> AbiType { match typ { Type::FieldElement => AbiType::Field, Type::Array(size, typ) => { + let span = get_main_function_span(context); let length = size - .evaluate_to_u32() + .evaluate_to_u32(span) .expect("Cannot have variable sized arrays as a parameter to main"); let typ = typ.as_ref(); AbiType::Array { length, typ: Box::new(abi_type_from_hir_type(context, typ)) } @@ -91,8 +103,9 @@ pub(super) fn abi_type_from_hir_type(context: &Context, typ: &Type) -> AbiType { } Type::Bool => AbiType::Boolean, Type::String(size) => { + let span = get_main_function_span(context); let size = size - .evaluate_to_u32() + .evaluate_to_u32(span) .expect("Cannot have variable sized strings as a parameter to main"); AbiType::String { length: size } } @@ -107,6 +120,7 @@ pub(super) fn abi_type_from_hir_type(context: &Context, typ: &Type) -> AbiType { AbiType::Struct { fields, path } } Type::Alias(def, args) => abi_type_from_hir_type(context, &def.borrow().get_type(args)), + Type::CheckedCast { to, .. } => abi_type_from_hir_type(context, to), Type::Tuple(fields) => { let fields = vecmap(fields, |typ| abi_type_from_hir_type(context, typ)); AbiType::Tuple { fields } diff --git a/noir/noir-repo/compiler/noirc_driver/src/lib.rs b/noir/noir-repo/compiler/noirc_driver/src/lib.rs index c4e2491a5fe..d10029d81e6 100644 --- a/noir/noir-repo/compiler/noirc_driver/src/lib.rs +++ b/noir/noir-repo/compiler/noirc_driver/src/lib.rs @@ -456,9 +456,8 @@ fn compile_contract_inner( .secondary .iter() .filter_map(|attr| match attr { - SecondaryAttribute::Tag(attribute) | SecondaryAttribute::Meta(attribute) => { - Some(attribute.contents.clone()) - } + SecondaryAttribute::Tag(attribute) => Some(attribute.contents.clone()), + SecondaryAttribute::Meta(attribute) => Some(attribute.to_string()), _ => None, }) .collect(); diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs index 78151d18b61..5e2c0f0827d 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs @@ -371,8 +371,8 @@ fn split_public_and_private_inputs( func_sig .0 .iter() - .map(|(_, typ, visibility)| { - let num_field_elements_needed = typ.field_count() as usize; + .map(|(pattern, typ, visibility)| { + let num_field_elements_needed = typ.field_count(&pattern.location()) as usize; let witnesses = input_witnesses[idx..idx + num_field_elements_needed].to_vec(); idx += num_field_elements_needed; (visibility, witnesses) diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/function_builder/data_bus.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/function_builder/data_bus.rs index de9ae8a24d7..5a62e9c8e9a 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/function_builder/data_bus.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/function_builder/data_bus.rs @@ -48,7 +48,7 @@ impl DataBusBuilder { ast::Visibility::CallData(id) => DatabusVisibility::CallData(id), ast::Visibility::ReturnData => DatabusVisibility::ReturnData, }; - let len = param.1.field_count() as usize; + let len = param.1.field_count(¶m.0.location()) as usize; params_is_databus.extend(vec![is_databus; len]); } params_is_databus diff --git a/noir/noir-repo/compiler/noirc_frontend/Cargo.toml b/noir/noir-repo/compiler/noirc_frontend/Cargo.toml index d729dabcb04..581d7f1b61d 100644 --- a/noir/noir-repo/compiler/noirc_frontend/Cargo.toml +++ b/noir/noir-repo/compiler/noirc_frontend/Cargo.toml @@ -36,6 +36,8 @@ strum_macros = "0.24" [dev-dependencies] base64.workspace = true +proptest.workspace = true +proptest-derive = "0.5.0" [features] experimental_parser = [] diff --git a/noir/noir-repo/compiler/noirc_frontend/proptest-regressions/hir_def/types/arithmetic.txt b/noir/noir-repo/compiler/noirc_frontend/proptest-regressions/hir_def/types/arithmetic.txt new file mode 100644 index 00000000000..80f5c7f1ead --- /dev/null +++ b/noir/noir-repo/compiler/noirc_frontend/proptest-regressions/hir_def/types/arithmetic.txt @@ -0,0 +1,7 @@ +# Seeds for failure cases proptest has generated in the past. It is +# automatically read and these particular cases re-run before any +# novel cases are generated. +# +# It is recommended to check this file in to source control so that +# everyone who runs the test benefits from these saved cases. +cc fc27f4091dfa5f938973048209b5fcf22aefa1cfaffaaa3e349f30e9b1f93f49 # shrinks to infix_and_bindings = (((0: numeric bool) % (Numeric(Shared(RefCell { value: Unbound('2, Numeric(bool)) }): bool) + Numeric(Shared(RefCell { value: Unbound('0, Numeric(bool)) }): bool))), [('0, (0: numeric bool)), ('1, (0: numeric bool)), ('2, (0: numeric bool))]) diff --git a/noir/noir-repo/compiler/noirc_frontend/proptest-regressions/tests/arithmetic_generics.txt b/noir/noir-repo/compiler/noirc_frontend/proptest-regressions/tests/arithmetic_generics.txt new file mode 100644 index 00000000000..80f5c7f1ead --- /dev/null +++ b/noir/noir-repo/compiler/noirc_frontend/proptest-regressions/tests/arithmetic_generics.txt @@ -0,0 +1,7 @@ +# Seeds for failure cases proptest has generated in the past. It is +# automatically read and these particular cases re-run before any +# novel cases are generated. +# +# It is recommended to check this file in to source control so that +# everyone who runs the test benefits from these saved cases. +cc fc27f4091dfa5f938973048209b5fcf22aefa1cfaffaaa3e349f30e9b1f93f49 # shrinks to infix_and_bindings = (((0: numeric bool) % (Numeric(Shared(RefCell { value: Unbound('2, Numeric(bool)) }): bool) + Numeric(Shared(RefCell { value: Unbound('0, Numeric(bool)) }): bool))), [('0, (0: numeric bool)), ('1, (0: numeric bool)), ('2, (0: numeric bool))]) diff --git a/noir/noir-repo/compiler/noirc_frontend/src/ast/expression.rs b/noir/noir-repo/compiler/noirc_frontend/src/ast/expression.rs index 67ddffe3277..2c8a9b6508d 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/ast/expression.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/ast/expression.rs @@ -98,7 +98,7 @@ impl UnresolvedGeneric { UnresolvedGeneric::Variable(_) => Ok(Kind::Normal), UnresolvedGeneric::Numeric { typ, .. } => { let typ = self.resolve_numeric_kind_type(typ)?; - Ok(Kind::Numeric(Box::new(typ))) + Ok(Kind::numeric(typ)) } UnresolvedGeneric::Resolved(..) => { panic!("Don't know the kind of a resolved generic here") @@ -504,7 +504,7 @@ impl FunctionDefinition { } pub fn is_test(&self) -> bool { - if let Some(attribute) = &self.attributes.function { + if let Some(attribute) = self.attributes.function() { matches!(attribute, FunctionAttribute::Test(..)) } else { false diff --git a/noir/noir-repo/compiler/noirc_frontend/src/ast/function.rs b/noir/noir-repo/compiler/noirc_frontend/src/ast/function.rs index beeea3ffac5..99ae78c93ea 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/ast/function.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/ast/function.rs @@ -77,7 +77,7 @@ impl NoirFunction { &self.def.attributes } pub fn function_attribute(&self) -> Option<&FunctionAttribute> { - self.def.attributes.function.as_ref() + self.def.attributes.function() } pub fn secondary_attributes(&self) -> &[SecondaryAttribute] { self.def.attributes.secondary.as_ref() @@ -108,7 +108,7 @@ impl NoirFunction { impl From for NoirFunction { fn from(fd: FunctionDefinition) -> Self { // The function type is determined by the existence of a function attribute - let kind = match fd.attributes.function { + let kind = match fd.attributes.function() { Some(FunctionAttribute::Builtin(_)) => FunctionKind::Builtin, Some(FunctionAttribute::Foreign(_)) => FunctionKind::LowLevel, Some(FunctionAttribute::Test { .. }) => FunctionKind::Normal, diff --git a/noir/noir-repo/compiler/noirc_frontend/src/ast/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/ast/mod.rs index e85563691ba..3c6664dd569 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/ast/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/ast/mod.rs @@ -19,6 +19,9 @@ pub use visitor::Visitor; pub use expression::*; pub use function::*; +#[cfg(test)] +use proptest_derive::Arbitrary; + use acvm::FieldElement; pub use docs::*; use noirc_errors::Span; @@ -37,6 +40,7 @@ use crate::{ use acvm::acir::AcirField; use iter_extended::vecmap; +#[cfg_attr(test, derive(Arbitrary))] #[derive(Debug, PartialEq, Eq, Clone, Copy, Hash, Ord, PartialOrd)] pub enum IntegerBitSize { One, diff --git a/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs b/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs index 49f585894d9..7244be371af 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs @@ -445,11 +445,6 @@ impl Path { self.span } - pub fn first_segment(&self) -> PathSegment { - assert!(!self.segments.is_empty()); - self.segments.first().unwrap().clone() - } - pub fn last_segment(&self) -> PathSegment { assert!(!self.segments.is_empty()); self.segments.last().unwrap().clone() @@ -459,9 +454,8 @@ impl Path { self.last_segment().ident } - pub fn first_name(&self) -> &str { - assert!(!self.segments.is_empty()); - &self.segments.first().unwrap().ident.0.contents + pub fn first_name(&self) -> Option<&str> { + self.segments.first().map(|segment| segment.ident.0.contents.as_str()) } pub fn last_name(&self) -> &str { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/ast/visitor.rs b/noir/noir-repo/compiler/noirc_frontend/src/ast/visitor.rs index 9e12b29677c..f149c998eca 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/ast/visitor.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/ast/visitor.rs @@ -16,7 +16,7 @@ use crate::{ InternedUnresolvedTypeData, QuotedTypeId, }, parser::{Item, ItemKind, ParsedSubModule}, - token::{CustomAttribute, SecondaryAttribute, Tokens}, + token::{MetaAttribute, SecondaryAttribute, Tokens}, ParsedModule, QuotedType, }; @@ -474,7 +474,9 @@ pub trait Visitor { true } - fn visit_custom_attribute(&mut self, _: &CustomAttribute, _target: AttributeTarget) {} + fn visit_meta_attribute(&mut self, _: &MetaAttribute, _target: AttributeTarget) -> bool { + true + } } impl ParsedModule { @@ -1441,15 +1443,22 @@ impl SecondaryAttribute { } pub fn accept_children(&self, target: AttributeTarget, visitor: &mut impl Visitor) { - if let SecondaryAttribute::Meta(custom) = self { - custom.accept(target, visitor); + if let SecondaryAttribute::Meta(meta_attribute) = self { + meta_attribute.accept(target, visitor); } } } -impl CustomAttribute { +impl MetaAttribute { pub fn accept(&self, target: AttributeTarget, visitor: &mut impl Visitor) { - visitor.visit_custom_attribute(self, target); + if visitor.visit_meta_attribute(self, target) { + self.accept_children(visitor); + } + } + + pub fn accept_children(&self, visitor: &mut impl Visitor) { + self.name.accept(visitor); + visit_expressions(&self.arguments, visitor); } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs index 13f51abe6ba..279adc331ea 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -19,10 +19,9 @@ use crate::{ resolution::errors::ResolverError, }, hir_def::expr::{HirExpression, HirIdent}, - lexer::Lexer, node_interner::{DefinitionKind, DependencyId, FuncId, NodeInterner, StructId, TraitId}, - parser::{Item, ItemKind, Parser}, - token::SecondaryAttribute, + parser::{Item, ItemKind}, + token::{MetaAttribute, SecondaryAttribute}, Type, TypeBindings, UnificationError, }; @@ -162,10 +161,9 @@ impl<'context> Elaborator<'context> { if let SecondaryAttribute::Meta(attribute) = attribute { self.elaborate_in_comptime_context(|this| { if let Err(error) = this.run_comptime_attribute_name_on_item( - &attribute.contents, + attribute, item.clone(), span, - attribute.contents_span, attribute_context, generated_items, ) { @@ -177,27 +175,21 @@ impl<'context> Elaborator<'context> { fn run_comptime_attribute_name_on_item( &mut self, - attribute: &str, + attribute: &MetaAttribute, item: Value, span: Span, - attribute_span: Span, attribute_context: AttributeContext, generated_items: &mut CollectedItems, ) -> Result<(), (CompilationError, FileId)> { self.file = attribute_context.attribute_file; self.local_module = attribute_context.attribute_module; - let location = Location::new(attribute_span, self.file); - let Some((function, arguments)) = Self::parse_attribute(attribute, location)? else { - return Err(( - ResolverError::UnableToParseAttribute { - attribute: attribute.to_string(), - span: attribute_span, - } - .into(), - self.file, - )); + let location = Location::new(attribute.span, self.file); + let function = Expression { + kind: ExpressionKind::Variable(attribute.name.clone()), + span: attribute.span, }; + let arguments = attribute.arguments.clone(); // Elaborate the function, rolling back any errors generated in case it is unknown let error_count = self.errors.len(); @@ -211,7 +203,7 @@ impl<'context> Elaborator<'context> { return Err(( ResolverError::AttributeFunctionIsNotAPath { function: function_string, - span: attribute_span, + span: attribute.span, } .into(), self.file, @@ -223,7 +215,7 @@ impl<'context> Elaborator<'context> { return Err(( ResolverError::AttributeFunctionNotInScope { name: function_string, - span: attribute_span, + span: attribute.span, } .into(), self.file, @@ -269,38 +261,6 @@ impl<'context> Elaborator<'context> { Ok(()) } - /// Parses an attribute in the form of a function call (e.g. `#[foo(a b, c d)]`) into - /// the function and quoted arguments called (e.g. `("foo", vec![(a b, location), (c d, location)])`) - #[allow(clippy::type_complexity)] - pub(crate) fn parse_attribute( - annotation: &str, - location: Location, - ) -> Result)>, (CompilationError, FileId)> { - let (tokens, mut lexing_errors) = Lexer::lex(annotation); - if !lexing_errors.is_empty() { - return Err((lexing_errors.swap_remove(0).into(), location.file)); - } - - let Some(expression) = Parser::for_tokens(tokens).parse_option(Parser::parse_expression) - else { - return Ok(None); - }; - - let (mut func, mut arguments) = match expression.kind { - ExpressionKind::Call(call) => (*call.func, call.arguments), - ExpressionKind::Variable(_) => (expression, Vec::new()), - _ => return Ok(None), - }; - - func.span = func.span.shift_by(location.span.start()); - - for argument in &mut arguments { - argument.span = argument.span.shift_by(location.span.start()); - } - - Ok(Some((func, arguments))) - } - fn handle_attribute_arguments( interpreter: &mut Interpreter, item: &Value, diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/lints.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/lints.rs index 249fed90a60..d3b776bea24 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/lints.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/lints.rs @@ -67,7 +67,7 @@ pub(super) fn low_level_function_outside_stdlib( crate_id: CrateId, ) -> Option { let is_low_level_function = - modifiers.attributes.function.as_ref().map_or(false, |func| func.is_low_level()); + modifiers.attributes.function().map_or(false, |func| func.is_low_level()); if !crate_id.is_stdlib() && is_low_level_function { let ident = func_meta_name_ident(func, modifiers); Some(ResolverError::LowLevelFunctionOutsideOfStdlib { ident }) @@ -81,8 +81,7 @@ pub(super) fn oracle_not_marked_unconstrained( func: &FuncMeta, modifiers: &FunctionModifiers, ) -> Option { - let is_oracle_function = - modifiers.attributes.function.as_ref().map_or(false, |func| func.is_oracle()); + let is_oracle_function = modifiers.attributes.function().map_or(false, |func| func.is_oracle()); if is_oracle_function && !modifiers.is_unconstrained { let ident = func_meta_name_ident(func, modifiers); Some(ResolverError::OracleMarkedAsConstrained { ident }) @@ -105,8 +104,7 @@ pub(super) fn oracle_called_from_constrained_function( } let function_attributes = interner.function_attributes(called_func); - let is_oracle_call = - function_attributes.function.as_ref().map_or(false, |func| func.is_oracle()); + let is_oracle_call = function_attributes.function().map_or(false, |func| func.is_oracle()); if is_oracle_call { Some(ResolverError::UnconstrainedOracleReturnToConstrained { span }) } else { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs index 14901303b92..084bcbe3f8d 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs @@ -4,9 +4,8 @@ use std::{ }; use crate::{ - ast::ItemVisibility, hir::resolution::import::PathResolutionItem, - hir_def::traits::ResolvedTraitBound, usage_tracker::UsageTracker, StructField, StructType, - TypeBindings, + ast::ItemVisibility, hir_def::traits::ResolvedTraitBound, usage_tracker::UsageTracker, + StructField, StructType, TypeBindings, }; use crate::{ ast::{ @@ -24,7 +23,6 @@ use crate::{ def_map::{DefMaps, ModuleData}, def_map::{LocalModuleId, ModuleId, MAIN_FUNCTION}, resolution::errors::ResolverError, - resolution::import::PathResolution, scope::ScopeForest as GenericScopeForest, type_check::{generics::TraitGenerics, TypeCheckError}, Context, @@ -40,13 +38,14 @@ use crate::{ DefinitionKind, DependencyId, ExprId, FuncId, FunctionModifiers, GlobalId, NodeInterner, ReferenceId, StructId, TraitId, TraitImplId, TypeAliasId, }, - token::{CustomAttribute, SecondaryAttribute}, + token::SecondaryAttribute, Shared, Type, TypeVariable, }; mod comptime; mod expressions; mod lints; +mod path_resolution; mod patterns; mod scope; mod statements; @@ -58,6 +57,7 @@ mod unquote; use fm::FileId; use iter_extended::vecmap; use noirc_errors::{Location, Span, Spanned}; +use path_resolution::{PathResolution, PathResolutionItem}; use types::bind_ordered_generics; use self::traits::check_trait_impl_method_matches_declaration; @@ -641,7 +641,7 @@ impl<'context> Elaborator<'context> { let typ = if unresolved_typ.is_type_expression() { self.resolve_type_inner( unresolved_typ.clone(), - &Kind::Numeric(Box::new(Type::default_int_type())), + &Kind::numeric(Type::default_int_type()), ) } else { self.resolve_type(unresolved_typ.clone()) @@ -654,7 +654,7 @@ impl<'context> Elaborator<'context> { }); self.push_err(unsupported_typ_err); } - Kind::Numeric(Box::new(typ)) + Kind::numeric(typ) } else { Kind::Normal } @@ -839,11 +839,6 @@ impl<'context> Elaborator<'context> { None }; - let attributes = func.secondary_attributes().iter(); - let attributes = - attributes.filter_map(|secondary_attribute| secondary_attribute.as_custom()); - let attributes: Vec = attributes.cloned().collect(); - let meta = FuncMeta { name: name_ident, kind: func.kind, @@ -867,7 +862,6 @@ impl<'context> Elaborator<'context> { function_body: FunctionBody::Unresolved(func.kind, body, func.def.span), self_type: self.self_type.clone(), source_file: self.file, - custom_attributes: attributes, }; self.interner.push_fn_meta(meta, func_id); @@ -896,6 +890,10 @@ impl<'context> Elaborator<'context> { Type::Alias(alias_type, generics) => { self.mark_type_as_used(&alias_type.borrow().get_type(generics)); } + Type::CheckedCast { from, to } => { + self.mark_type_as_used(from); + self.mark_type_as_used(to); + } Type::MutableReference(typ) => { self.mark_type_as_used(typ); } @@ -1498,6 +1496,10 @@ impl<'context> Elaborator<'context> { span, ); } + Type::CheckedCast { from, to } => { + self.check_type_is_not_more_private_then_item(name, visibility, from, span); + self.check_type_is_not_more_private_then_item(name, visibility, to, span); + } Type::Function(args, return_type, env, _) => { for arg in args { self.check_type_is_not_more_private_then_item(name, visibility, arg, span); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/path_resolution.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/path_resolution.rs new file mode 100644 index 00000000000..a68991becb7 --- /dev/null +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/path_resolution.rs @@ -0,0 +1,347 @@ +use noirc_errors::{Location, Span}; + +use crate::ast::{Path, PathKind, UnresolvedType}; +use crate::hir::def_map::{ModuleDefId, ModuleId}; +use crate::hir::resolution::import::{resolve_path_kind, PathResolutionError}; + +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::Type; + +use super::types::SELF_TYPE_NAME; +use super::Elaborator; + +#[derive(Debug)] +pub(crate) struct PathResolution { + pub(crate) item: PathResolutionItem, + pub(crate) errors: Vec, +} + +/// All possible items that result from resolving a Path. +/// Note that this item doesn't include the last turbofish in a Path, +/// only intermediate ones, if any. +#[derive(Debug, Clone)] +pub enum PathResolutionItem { + Module(ModuleId), + Struct(StructId), + TypeAlias(TypeAliasId), + Trait(TraitId), + Global(GlobalId), + ModuleFunction(FuncId), + StructFunction(StructId, Option, FuncId), + TypeAliasFunction(TypeAliasId, Option, FuncId), + TraitFunction(TraitId, Option, FuncId), +} + +impl PathResolutionItem { + pub fn function_id(&self) -> Option { + match self { + PathResolutionItem::ModuleFunction(func_id) + | PathResolutionItem::StructFunction(_, _, func_id) + | PathResolutionItem::TypeAliasFunction(_, _, func_id) + | PathResolutionItem::TraitFunction(_, _, func_id) => Some(*func_id), + _ => None, + } + } + + pub fn module_id(&self) -> Option { + match self { + Self::Module(module_id) => Some(*module_id), + _ => None, + } + } + + pub fn description(&self) -> &'static str { + match self { + PathResolutionItem::Module(..) => "module", + PathResolutionItem::Struct(..) => "type", + PathResolutionItem::TypeAlias(..) => "type alias", + PathResolutionItem::Trait(..) => "trait", + PathResolutionItem::Global(..) => "global", + PathResolutionItem::ModuleFunction(..) + | PathResolutionItem::StructFunction(..) + | PathResolutionItem::TypeAliasFunction(..) + | PathResolutionItem::TraitFunction(..) => "function", + } + } +} + +#[derive(Debug, Clone)] +pub struct Turbofish { + pub generics: Vec, + pub span: Span, +} + +/// Any item that can appear before the last segment in a path. +#[derive(Debug)] +enum IntermediatePathResolutionItem { + Module(ModuleId), + Struct(StructId, Option), + TypeAlias(TypeAliasId, Option), + Trait(TraitId, Option), +} + +pub(crate) type PathResolutionResult = Result; + +impl<'context> Elaborator<'context> { + pub(super) fn resolve_path_or_error( + &mut self, + path: Path, + ) -> Result { + let path_resolution = self.resolve_path(path)?; + + for error in path_resolution.errors { + self.push_err(error); + } + + Ok(path_resolution.item) + } + + /// Resolves a path in the current module. + /// If the referenced name can't be found, `Err` will be returned. If it can be found, `Ok` + /// will be returned with a potential list of errors if, for example, one of the segments + /// is not accessible from the current module (e.g. because it's private). + pub(super) fn resolve_path(&mut self, mut path: Path) -> PathResolutionResult { + 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 path.segments.len() == 1 { + return Ok(PathResolution { + item: PathResolutionItem::Struct(struct_type.id), + errors: Vec::new(), + }); + } + + module_id = struct_type.id.module_id(); + path.segments.remove(0); + } + } + + self.resolve_path_in_module(path, module_id) + } + + /// Resolves a path in `current_module`. + /// `importing_module` is the module where the lookup originally started. + fn resolve_path_in_module( + &mut self, + path: Path, + importing_module: ModuleId, + ) -> PathResolutionResult { + let references_tracker = if self.interner.is_in_lsp_mode() { + Some(ReferencesTracker::new(self.interner, self.file)) + } else { + None + }; + let (path, module_id, _) = + resolve_path_kind(path, importing_module, self.def_maps, references_tracker)?; + self.resolve_name_in_module(path, module_id, importing_module) + } + + /// Resolves a Path assuming we are inside `starting_module`. + /// `importing_module` is the module where the lookup originally started. + fn resolve_name_in_module( + &mut self, + path: Path, + starting_module: ModuleId, + importing_module: ModuleId, + ) -> PathResolutionResult { + // There is a possibility that the import path is empty. In that case, early return. + if path.segments.is_empty() { + return Ok(PathResolution { + item: PathResolutionItem::Module(starting_module), + errors: Vec::new(), + }); + } + + let plain_or_crate = matches!(path.kind, PathKind::Plain | PathKind::Crate); + + // The current module and module ID as we resolve path segments + let mut current_module_id = starting_module; + let mut current_module = self.get_module(starting_module); + + let mut intermediate_item = IntermediatePathResolutionItem::Module(current_module_id); + + let first_segment = + &path.segments.first().expect("ice: could not fetch first segment").ident; + let mut current_ns = current_module.find_name(first_segment); + if current_ns.is_none() { + return Err(PathResolutionError::Unresolved(first_segment.clone())); + } + + self.usage_tracker.mark_as_referenced(current_module_id, first_segment); + + let mut errors = Vec::new(); + for (index, (last_segment, current_segment)) in + path.segments.iter().zip(path.segments.iter().skip(1)).enumerate() + { + let last_ident = &last_segment.ident; + let current_ident = ¤t_segment.ident; + let last_segment_generics = &last_segment.generics; + + let (typ, visibility) = match current_ns.types { + None => return Err(PathResolutionError::Unresolved(last_ident.clone())), + Some((typ, visibility, _)) => (typ, visibility), + }; + + let location = Location::new(last_segment.span, self.file); + self.interner.add_module_def_id_reference( + typ, + location, + last_segment.ident.is_self_type_name(), + ); + + (current_module_id, intermediate_item) = match typ { + ModuleDefId::ModuleId(id) => { + if last_segment_generics.is_some() { + errors.push(PathResolutionError::TurbofishNotAllowedOnItem { + item: format!("module `{last_ident}`"), + span: last_segment.turbofish_span(), + }); + } + + (id, IntermediatePathResolutionItem::Module(id)) + } + ModuleDefId::TypeId(id) => ( + id.module_id(), + IntermediatePathResolutionItem::Struct( + id, + last_segment_generics.as_ref().map(|generics| Turbofish { + generics: generics.clone(), + span: last_segment.turbofish_span(), + }), + ), + ), + ModuleDefId::TypeAliasId(id) => { + let type_alias = self.interner.get_type_alias(id); + let type_alias = type_alias.borrow(); + + let module_id = match &type_alias.typ { + Type::Struct(struct_id, _generics) => struct_id.borrow().id.module_id(), + Type::Error => { + return Err(PathResolutionError::Unresolved(last_ident.clone())); + } + _ => { + // For now we only allow type aliases that point to structs. + // 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") + } + }; + + ( + module_id, + IntermediatePathResolutionItem::TypeAlias( + id, + last_segment_generics.as_ref().map(|generics| Turbofish { + generics: generics.clone(), + span: last_segment.turbofish_span(), + }), + ), + ) + } + ModuleDefId::TraitId(id) => ( + id.0, + IntermediatePathResolutionItem::Trait( + id, + last_segment_generics.as_ref().map(|generics| Turbofish { + generics: generics.clone(), + span: last_segment.turbofish_span(), + }), + ), + ), + ModuleDefId::FunctionId(_) => panic!("functions cannot be in the type namespace"), + ModuleDefId::GlobalId(_) => panic!("globals cannot be in the type namespace"), + }; + + // If the path is plain or crate, the first segment will always refer to + // something that's visible from the current module. + if !((plain_or_crate && index == 0) + || item_in_module_is_visible( + self.def_maps, + importing_module, + current_module_id, + visibility, + )) + { + errors.push(PathResolutionError::Private(last_ident.clone())); + } + + current_module = self.get_module(current_module_id); + + // Check if namespace + let found_ns = current_module.find_name(current_ident); + if found_ns.is_none() { + return Err(PathResolutionError::Unresolved(current_ident.clone())); + } + + self.usage_tracker.mark_as_referenced(current_module_id, current_ident); + + current_ns = found_ns; + } + + let (module_def_id, visibility, _) = + current_ns.values.or(current_ns.types).expect("Found empty namespace"); + + let name = path.last_ident(); + let is_self_type = name.is_self_type_name(); + let location = Location::new(name.span(), self.file); + self.interner.add_module_def_id_reference(module_def_id, location, is_self_type); + + let item = merge_intermediate_path_resolution_item_with_module_def_id( + intermediate_item, + module_def_id, + ); + + if !(self.self_type_module_id() == Some(current_module_id) + || item_in_module_is_visible( + self.def_maps, + importing_module, + current_module_id, + visibility, + )) + { + errors.push(PathResolutionError::Private(name.clone())); + } + + Ok(PathResolution { item, errors }) + } + + fn self_type_module_id(&self) -> Option { + if let Some(Type::Struct(struct_type, _)) = &self.self_type { + Some(struct_type.borrow().id.module_id()) + } else { + None + } + } +} + +fn merge_intermediate_path_resolution_item_with_module_def_id( + intermediate_item: IntermediatePathResolutionItem, + module_def_id: ModuleDefId, +) -> PathResolutionItem { + match module_def_id { + ModuleDefId::ModuleId(module_id) => PathResolutionItem::Module(module_id), + ModuleDefId::TypeId(struct_id) => PathResolutionItem::Struct(struct_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::TypeAlias(alias_id, generics) => { + PathResolutionItem::TypeAliasFunction(alias_id, generics, func_id) + } + IntermediatePathResolutionItem::Trait(trait_id, generics) => { + PathResolutionItem::TraitFunction(trait_id, generics, func_id) + } + }, + } +} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/patterns.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/patterns.rs index 9e60adcbc6f..3928362db11 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -9,7 +9,7 @@ use crate::{ }, hir::{ def_collector::dc_crate::CompilationError, - resolution::{errors::ResolverError, import::PathResolutionItem}, + resolution::errors::ResolverError, type_check::{Source, TypeCheckError}, }, hir_def::{ @@ -20,7 +20,7 @@ use crate::{ Kind, Shared, StructType, Type, TypeAlias, TypeBindings, }; -use super::{Elaborator, ResolverMeta}; +use super::{path_resolution::PathResolutionItem, Elaborator, ResolverMeta}; impl<'context> Elaborator<'context> { pub(super) fn elaborate_pattern( diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/scope.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/scope.rs index 33e97c1df22..fe01e3cb7f3 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/scope.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/scope.rs @@ -1,9 +1,8 @@ -use noirc_errors::{Location, Spanned}; +use noirc_errors::Spanned; -use crate::ast::{Ident, Path, PathKind, ERROR_IDENT}; +use crate::ast::{Ident, Path, ERROR_IDENT}; use crate::hir::def_map::{LocalModuleId, ModuleId}; -use crate::hir::resolution::import::{PathResolution, PathResolutionItem, PathResolutionResult}; -use crate::hir::resolution::path_resolver::{PathResolver, StandardPathResolver}; + use crate::hir::scope::{Scope as GenericScope, ScopeTree as GenericScopeTree}; use crate::{ hir::resolution::errors::ResolverError, @@ -16,6 +15,7 @@ use crate::{ }; use crate::{Type, TypeAlias}; +use super::path_resolution::PathResolutionItem; use super::types::SELF_TYPE_NAME; use super::{Elaborator, ResolverMeta}; @@ -37,99 +37,6 @@ impl<'context> Elaborator<'context> { current_module } - pub(super) fn resolve_path_or_error( - &mut self, - path: Path, - ) -> Result { - let path_resolution = self.resolve_path(path)?; - - for error in path_resolution.errors { - self.push_err(error); - } - - Ok(path_resolution.item) - } - - pub(super) fn resolve_path(&mut self, path: Path) -> PathResolutionResult { - let mut module_id = self.module_id(); - let mut path = path; - - if path.kind == PathKind::Plain && path.first_name() == SELF_TYPE_NAME { - if let Some(Type::Struct(struct_type, _)) = &self.self_type { - let struct_type = struct_type.borrow(); - if path.segments.len() == 1 { - return Ok(PathResolution { - item: PathResolutionItem::Struct(struct_type.id), - errors: Vec::new(), - }); - } - - module_id = struct_type.id.module_id(); - path = Path { - segments: path.segments[1..].to_vec(), - kind: PathKind::Plain, - span: path.span(), - }; - } - } - - self.resolve_path_in_module(path, module_id) - } - - fn resolve_path_in_module(&mut self, path: Path, module_id: ModuleId) -> PathResolutionResult { - let self_type_module_id = if let Some(Type::Struct(struct_type, _)) = &self.self_type { - Some(struct_type.borrow().id.module_id()) - } else { - None - }; - - let resolver = StandardPathResolver::new(module_id, self_type_module_id); - - if !self.interner.lsp_mode { - return resolver.resolve( - self.interner, - self.def_maps, - path, - self.usage_tracker, - &mut None, - ); - } - - let last_segment = path.last_ident(); - let location = Location::new(last_segment.span(), self.file); - let is_self_type_name = last_segment.is_self_type_name(); - - let mut references: Vec<_> = Vec::new(); - let path_resolution = resolver.resolve( - self.interner, - self.def_maps, - path.clone(), - self.usage_tracker, - &mut Some(&mut references), - ); - - for (referenced, segment) in references.iter().zip(path.segments) { - self.interner.add_reference( - *referenced, - Location::new(segment.ident.span(), self.file), - segment.ident.is_self_type_name(), - ); - } - - let path_resolution = match path_resolution { - Ok(path_resolution) => path_resolution, - Err(err) => return Err(err), - }; - - self.interner.add_path_resolution_kind_reference( - path_resolution.item.clone(), - location, - is_self_type_name, - ); - - Ok(path_resolution) - } - pub(super) fn get_struct(&self, type_id: StructId) -> Shared { self.interner.get_struct(type_id) } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/traits.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/traits.rs index ae278616e03..e1be45927ca 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/traits.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/traits.rs @@ -23,6 +23,8 @@ use super::Elaborator; impl<'context> Elaborator<'context> { pub fn collect_traits(&mut self, traits: &BTreeMap) { for (trait_id, unresolved_trait) in traits { + self.local_module = unresolved_trait.module_id; + self.recover_generics(|this| { this.current_trait = Some(*trait_id); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs index c8a16a6cd9b..b296c4f1805 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs @@ -14,10 +14,7 @@ use crate::{ hir::{ comptime::{Interpreter, Value}, def_collector::dc_crate::CompilationError, - resolution::{ - errors::ResolverError, - import::{PathResolutionError, PathResolutionItem}, - }, + resolution::{errors::ResolverError, import::PathResolutionError}, type_check::{ generics::{Generic, TraitGenerics}, NoMatchingImplFoundError, Source, TypeCheckError, @@ -41,7 +38,7 @@ use crate::{ UnificationError, }; -use super::{lints, Elaborator}; +use super::{lints, path_resolution::PathResolutionItem, Elaborator}; pub const SELF_TYPE_NAME: &str = "Self"; pub const WILDCARD_TYPE: &str = "_"; @@ -195,7 +192,7 @@ impl<'context> Elaborator<'context> { // Resolve Self::Foo to an associated type on the current trait or trait impl fn lookup_associated_type_on_self(&self, path: &Path) -> Option { - if path.segments.len() == 2 && path.first_name() == SELF_TYPE_NAME { + if path.segments.len() == 2 && path.first_name() == Some(SELF_TYPE_NAME) { if let Some(trait_id) = self.current_trait { let the_trait = self.interner.get_trait(trait_id); if let Some(typ) = the_trait.get_associated_type(path.last_name()) { @@ -417,7 +414,7 @@ impl<'context> Elaborator<'context> { let kind = self .interner .get_global_let_statement(id) - .map(|let_statement| Kind::Numeric(Box::new(let_statement.r#type))) + .map(|let_statement| Kind::numeric(let_statement.r#type)) .unwrap_or(Kind::u32()); // TODO(https://github.com/noir-lang/noir/issues/6238): @@ -465,14 +462,26 @@ impl<'context> Elaborator<'context> { }); return Type::Error; } - if let Some(result) = op.function(lhs, rhs, &lhs_kind) { - Type::Constant(result, lhs_kind) - } else { - self.push_err(ResolverError::OverflowInType { lhs, op, rhs, span }); - Type::Error + match op.function(lhs, rhs, &lhs_kind, span) { + Ok(result) => Type::Constant(result, lhs_kind), + Err(err) => { + let err = Box::new(err); + self.push_err(ResolverError::BinaryOpError { + lhs, + op, + rhs, + err, + span, + }); + Type::Error + } } } - (lhs, rhs) => Type::InfixExpr(Box::new(lhs), op, Box::new(rhs)).canonicalize(), + (lhs, rhs) => { + let infix = Type::InfixExpr(Box::new(lhs), op, Box::new(rhs)); + Type::CheckedCast { from: Box::new(infix.clone()), to: Box::new(infix) } + .canonicalize() + } } } UnresolvedTypeExpression::AsTraitPath(path) => { @@ -1741,6 +1750,11 @@ impl<'context> Elaborator<'context> { | Type::Quoted(_) | Type::Forall(_, _) => (), + Type::CheckedCast { from, to } => { + Self::find_numeric_generics_in_type(from, found); + Self::find_numeric_generics_in_type(to, found); + } + Type::TraitAsType(_, _, args) => { for arg in &args.ordered { Self::find_numeric_generics_in_type(arg, found); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/display.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/display.rs index bfbe39df241..560d11cfa2e 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/display.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/display.rs @@ -281,8 +281,7 @@ impl<'interner> TokenPrettyPrinter<'interner> { | Token::Whitespace(_) | Token::LineComment(..) | Token::BlockComment(..) - | Token::Attribute(..) - | Token::InnerAttribute(..) + | Token::AttributeStart { .. } | Token::Invalid(_) => { if last_was_alphanumeric { write!(f, " ")?; diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/errors.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/errors.rs index dfd328f85f0..198ba91156e 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/errors.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/errors.rs @@ -3,7 +3,10 @@ use std::rc::Rc; use crate::{ ast::TraitBound, - hir::{def_collector::dc_crate::CompilationError, type_check::NoMatchingImplFoundError}, + hir::{ + def_collector::dc_crate::CompilationError, + type_check::{NoMatchingImplFoundError, TypeCheckError}, + }, parser::ParserError, Type, }; @@ -87,6 +90,7 @@ pub enum InterpreterError { }, NonIntegerArrayLength { typ: Type, + err: Option>, location: Location, }, NonNumericCasted { @@ -228,6 +232,7 @@ pub enum InterpreterError { }, UnknownArrayLength { length: Type, + err: Box, location: Location, }, @@ -435,9 +440,13 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic { let secondary = "This is likely a bug".into(); CustomDiagnostic::simple_error(msg, secondary, location.span) } - InterpreterError::NonIntegerArrayLength { typ, location } => { + InterpreterError::NonIntegerArrayLength { typ, err, location } => { let msg = format!("Non-integer array length: `{typ}`"); - let secondary = "Array lengths must be integers".into(); + let secondary = if let Some(err) = err { + format!("Array lengths must be integers, but evaluating `{typ}` resulted in `{err}`") + } else { + "Array lengths must be integers".to_string() + }; CustomDiagnostic::simple_error(msg, secondary, location.span) } InterpreterError::NonNumericCasted { typ, location } => { @@ -640,9 +649,10 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic { let msg = format!("`{expression}` is not a valid function body"); CustomDiagnostic::simple_error(msg, String::new(), location.span) } - InterpreterError::UnknownArrayLength { length, location } => { + InterpreterError::UnknownArrayLength { length, err, location } => { let msg = format!("Could not determine array length `{length}`"); - CustomDiagnostic::simple_error(msg, String::new(), location.span) + let secondary = format!("Evaluating the length failed with: `{err}`"); + CustomDiagnostic::simple_error(msg, secondary, location.span) } } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs index 260c4e3848a..5540a199cec 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs @@ -340,6 +340,7 @@ impl Type { let name = Path::from_single(name.as_ref().clone(), Span::default()); UnresolvedTypeData::Named(name, GenericTypeArgs::default(), true) } + Type::CheckedCast { to, .. } => to.to_display_ast().typ, Type::Function(args, ret, env, unconstrained) => { let args = vecmap(args, |arg| arg.to_display_ast()); let ret = Box::new(ret.to_display_ast()); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 7205242ead9..994318a371a 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -223,7 +223,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { location: Location, ) -> IResult { let attributes = self.elaborator.interner.function_attributes(&function); - let func_attrs = attributes.function.as_ref() + let func_attrs = attributes.function() .expect("all builtin functions must contain a function attribute which contains the opcode which it links to"); if let Some(builtin) = func_attrs.builtin() { @@ -585,20 +585,25 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { } DefinitionKind::NumericGeneric(type_variable, numeric_typ) => { let value = match &*type_variable.borrow() { - TypeBinding::Unbound(_, _) => None, + TypeBinding::Unbound(_, _) => { + let typ = self.elaborator.interner.id_type(id); + let location = self.elaborator.interner.expr_location(&id); + Err(InterpreterError::NonIntegerArrayLength { typ, err: None, location }) + } TypeBinding::Bound(binding) => { - binding.evaluate_to_field_element(&Kind::Numeric(numeric_typ.clone())) + let span = self.elaborator.interner.id_location(id).span; + binding + .evaluate_to_field_element(&Kind::Numeric(numeric_typ.clone()), span) + .map_err(|err| { + let typ = Type::TypeVariable(type_variable.clone()); + let err = Some(Box::new(err)); + let location = self.elaborator.interner.expr_location(&id); + InterpreterError::NonIntegerArrayLength { typ, err, location } + }) } - }; + }?; - if let Some(value) = value { - let typ = self.elaborator.interner.id_type(id); - self.evaluate_integer(value, false, id) - } else { - let location = self.elaborator.interner.expr_location(&id); - let typ = Type::TypeVariable(type_variable.clone()); - Err(InterpreterError::NonIntegerArrayLength { typ, location }) - } + self.evaluate_integer(value, false, id) } } } @@ -805,12 +810,17 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { HirArrayLiteral::Repeated { repeated_element, length } => { let element = self.evaluate(repeated_element)?; - if let Some(length) = length.evaluate_to_u32() { - let elements = (0..length).map(|_| element.clone()).collect(); - Ok(Value::Array(elements, typ)) - } else { - let location = self.elaborator.interner.expr_location(&id); - Err(InterpreterError::NonIntegerArrayLength { typ: length, location }) + let span = self.elaborator.interner.id_location(id).span; + match length.evaluate_to_u32(span) { + Ok(length) => { + let elements = (0..length).map(|_| element.clone()).collect(); + Ok(Value::Array(elements, typ)) + } + Err(err) => { + let err = Some(Box::new(err)); + let location = self.elaborator.interner.expr_location(&id); + Err(InterpreterError::NonIntegerArrayLength { typ: length, err, location }) + } } } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index d8842215a29..8a6c46ca50c 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -12,7 +12,7 @@ use builtin_helpers::{ }; use im::Vector; use iter_extended::{try_vecmap, vecmap}; -use noirc_errors::Location; +use noirc_errors::{Location, Span}; use num_bigint::BigUint; use rustc_hash::FxHashMap as HashMap; @@ -37,7 +37,7 @@ use crate::{ hir_def::{self}, node_interner::{DefinitionKind, NodeInterner, TraitImplKind}, parser::{Parser, StatementOrExpressionOrLValue}, - token::{Attribute, SecondaryAttribute, Token}, + token::{Attribute, Token}, Kind, QuotedType, ResolvedGeneric, Shared, Type, TypeVariable, }; @@ -230,7 +230,7 @@ impl<'local, 'context> Interpreter<'local, 'context> { "unresolved_type_is_bool" => unresolved_type_is_bool(interner, arguments, location), "unresolved_type_is_field" => unresolved_type_is_field(interner, arguments, location), "unresolved_type_is_unit" => unresolved_type_is_unit(interner, arguments, location), - "zeroed" => zeroed(return_type), + "zeroed" => zeroed(return_type, location.span), _ => { let item = format!("Comptime evaluation for builtin function {name}"); Err(InterpreterError::Unimplemented { item, location }) @@ -348,24 +348,9 @@ fn struct_def_add_attribute( let (self_argument, attribute) = check_two_arguments(arguments, location)?; let attribute_location = attribute.1; let attribute = get_str(interner, attribute)?; - - let mut tokens = lex(&format!("#[{}]", attribute)); - if tokens.len() != 1 { - return Err(InterpreterError::InvalidAttribute { - attribute: attribute.to_string(), - location: attribute_location, - }); - } - - let token = tokens.remove(0); - let Token::Attribute(attribute) = token else { - return Err(InterpreterError::InvalidAttribute { - attribute: attribute.to_string(), - location: attribute_location, - }); - }; - - let Attribute::Secondary(attribute) = attribute else { + let attribute = format!("#[{}]", attribute); + let mut parser = Parser::for_str(&attribute); + let Some((Attribute::Secondary(attribute), _span)) = parser.parse_attribute() else { return Err(InterpreterError::InvalidAttribute { attribute: attribute.to_string(), location: attribute_location, @@ -374,7 +359,7 @@ fn struct_def_add_attribute( let struct_id = get_struct(self_argument)?; interner.update_struct_attributes(struct_id, |attributes| { - attributes.push(attribute.clone()); + attributes.push(attribute); }); Ok(Value::Unit) @@ -710,7 +695,7 @@ fn quoted_as_expr( }, ); - option(return_type, value) + option(return_type, value, location.span) } // fn as_module(quoted: Quoted) -> Option @@ -737,7 +722,7 @@ fn quoted_as_module( module.map(Value::ModuleDefinition) }); - option(return_type, option_value) + option(return_type, option_value, location.span) } // fn as_trait_constraint(quoted: Quoted) -> TraitConstraint @@ -869,11 +854,22 @@ fn type_as_constant( return_type: Type, location: Location, ) -> IResult { - type_as(arguments, return_type, location, |typ| { + type_as_or_err(arguments, return_type, location, |typ| { // Prefer to use `evaluate_to_u32` over matching on `Type::Constant` // since arithmetic generics may be `Type::InfixExpr`s which evaluate to // constants but are not actually the `Type::Constant` variant. - typ.evaluate_to_u32().map(Value::U32) + match typ.evaluate_to_u32(location.span) { + Ok(constant) => Ok(Some(Value::U32(constant))), + Err(err) => { + // Evaluating to a non-constant returns 'None' in user code + if err.is_non_constant_evaluated() { + Ok(None) + } else { + let err = Some(Box::new(err)); + Err(InterpreterError::NonIntegerArrayLength { typ, err, location }) + } + } + } }) } @@ -979,7 +975,6 @@ fn type_as_tuple( }) } -// Helper function for implementing the `type_as_...` functions. fn type_as( arguments: Vec<(Value, Location)>, return_type: Type, @@ -988,13 +983,26 @@ fn type_as( ) -> IResult where F: FnOnce(Type) -> Option, +{ + type_as_or_err(arguments, return_type, location, |x| Ok(f(x))) +} + +// Helper function for implementing the `type_as_...` functions. +fn type_as_or_err( + arguments: Vec<(Value, Location)>, + return_type: Type, + location: Location, + f: F, +) -> IResult +where + F: FnOnce(Type) -> IResult>, { let value = check_one_argument(arguments, location)?; let typ = get_type(value)?.follow_bindings(); - let option_value = f(typ); + let option_value = f(typ)?; - option(return_type, option_value) + option(return_type, option_value, location.span) } // fn type_eq(_first: Type, _second: Type) -> bool @@ -1029,7 +1037,7 @@ fn type_get_trait_impl( _ => None, }; - option(return_type, option_value) + option(return_type, option_value, location.span) } // fn implements(self, constraint: TraitConstraint) -> bool @@ -1150,7 +1158,7 @@ fn typed_expr_as_function_definition( } else { None }; - option(return_type, option_value) + option(return_type, option_value, location.span) } // fn get_type(self) -> Option @@ -1172,7 +1180,7 @@ fn typed_expr_get_type( } else { None }; - option(return_type, option_value) + option(return_type, option_value, location.span) } // fn as_mutable_reference(self) -> Option @@ -1256,16 +1264,16 @@ where let option_value = f(typ); - option(return_type, option_value) + option(return_type, option_value, location.span) } // fn zeroed() -> T -fn zeroed(return_type: Type) -> IResult { +fn zeroed(return_type: Type, span: Span) -> IResult { match return_type { Type::FieldElement => Ok(Value::Field(0u128.into())), Type::Array(length_type, elem) => { - if let Some(length) = length_type.evaluate_to_u32() { - let element = zeroed(elem.as_ref().clone())?; + if let Ok(length) = length_type.evaluate_to_u32(span) { + let element = zeroed(elem.as_ref().clone(), span)?; let array = std::iter::repeat(element).take(length as usize).collect(); Ok(Value::Array(array, Type::Array(length_type, elem))) } else { @@ -1288,7 +1296,7 @@ fn zeroed(return_type: Type) -> IResult { }, Type::Bool => Ok(Value::Bool(false)), Type::String(length_type) => { - if let Some(length) = length_type.evaluate_to_u32() { + if let Ok(length) = length_type.evaluate_to_u32(span) { Ok(Value::String(Rc::new("\0".repeat(length as usize)))) } else { // Assume we can resolve the length later @@ -1296,9 +1304,9 @@ fn zeroed(return_type: Type) -> IResult { } } Type::FmtString(length_type, captures) => { - let length = length_type.evaluate_to_u32(); + let length = length_type.evaluate_to_u32(span); let typ = Type::FmtString(length_type, captures); - if let Some(length) = length { + if let Ok(length) = length { Ok(Value::FormatString(Rc::new("\0".repeat(length as usize)), typ)) } else { // Assume we can resolve the length later @@ -1306,26 +1314,27 @@ fn zeroed(return_type: Type) -> IResult { } } Type::Unit => Ok(Value::Unit), - Type::Tuple(fields) => Ok(Value::Tuple(try_vecmap(fields, zeroed)?)), + Type::Tuple(fields) => Ok(Value::Tuple(try_vecmap(fields, |field| zeroed(field, span))?)), Type::Struct(struct_type, generics) => { let fields = struct_type.borrow().get_fields(&generics); let mut values = HashMap::default(); for (field_name, field_type) in fields { - let field_value = zeroed(field_type)?; + let field_value = zeroed(field_type, span)?; values.insert(Rc::new(field_name), field_value); } let typ = Type::Struct(struct_type, generics); Ok(Value::Struct(values, typ)) } - Type::Alias(alias, generics) => zeroed(alias.borrow().get_type(&generics)), + Type::Alias(alias, generics) => zeroed(alias.borrow().get_type(&generics), span), + Type::CheckedCast { to, .. } => zeroed(*to, span), typ @ Type::Function(..) => { // Using Value::Zeroed here is probably safer than using FuncId::dummy_id() or similar Ok(Value::Zeroed(typ)) } Type::MutableReference(element) => { - let element = zeroed(*element)?; + let element = zeroed(*element, span)?; Ok(Value::Pointer(Shared::new(element), false)) } // Optimistically assume we can resolve this type later or that the value is unused @@ -1389,7 +1398,7 @@ fn expr_as_assert( let option_type = tuple_types.pop().unwrap(); let message = message.map(|msg| Value::expression(msg.kind)); - let message = option(option_type, message).ok()?; + let message = option(option_type, message, location.span).ok()?; Some(Value::Tuple(vec![predicate, message])) } else { @@ -1435,7 +1444,7 @@ fn expr_as_assert_eq( let option_type = tuple_types.pop().unwrap(); let message = message.map(|message| Value::expression(message.kind)); - let message = option(option_type, message).ok()?; + let message = option(option_type, message, location.span).ok()?; Some(Value::Tuple(vec![lhs, rhs, message])) } else { @@ -1611,7 +1620,7 @@ fn expr_as_constructor( None }; - option(return_type, option_value) + option(return_type, option_value, location.span) } // fn as_for(self) -> Option<(Quoted, Expr, Expr)> @@ -1705,6 +1714,7 @@ fn expr_as_if( let alternative = option( alternative_option_type, if_expr.alternative.map(|e| Value::expression(e.kind)), + location.span, ); Some(Value::Tuple(vec![ @@ -1793,7 +1803,7 @@ fn expr_as_lambda( } else { Some(Value::UnresolvedType(typ.typ)) }; - let typ = option(option_unresolved_type.clone(), typ).unwrap(); + let typ = option(option_unresolved_type.clone(), typ, location.span).unwrap(); Value::Tuple(vec![pattern, typ]) }) .collect(); @@ -1812,7 +1822,7 @@ fn expr_as_lambda( Some(return_type) }; let return_type = return_type.map(Value::UnresolvedType); - let return_type = option(option_unresolved_type, return_type).ok()?; + let return_type = option(option_unresolved_type, return_type, location.span).ok()?; let body = Value::expression(lambda.body.kind); @@ -1846,7 +1856,7 @@ fn expr_as_let( Some(Value::UnresolvedType(let_statement.r#type.typ)) }; - let typ = option(option_type, typ).ok()?; + let typ = option(option_type, typ, location.span).ok()?; Some(Value::Tuple(vec![ Value::pattern(let_statement.pattern), @@ -2098,7 +2108,7 @@ where let expr_value = unwrap_expr_value(interner, expr_value); let option_value = f(expr_value); - option(return_type, option_value) + option(return_type, option_value, location.span) } // fn resolve(self, in_function: Option) -> TypedExpr @@ -2225,17 +2235,9 @@ fn function_def_add_attribute( let (self_argument, attribute) = check_two_arguments(arguments, location)?; let attribute_location = attribute.1; let attribute = get_str(interpreter.elaborator.interner, attribute)?; - - let mut tokens = lex(&format!("#[{}]", attribute)); - if tokens.len() != 1 { - return Err(InterpreterError::InvalidAttribute { - attribute: attribute.to_string(), - location: attribute_location, - }); - } - - let token = tokens.remove(0); - let Token::Attribute(attribute) = token else { + let attribute = format!("#[{}]", attribute); + let mut parser = Parser::for_str(&attribute); + let Some((attribute, _span)) = parser.parse_attribute() else { return Err(InterpreterError::InvalidAttribute { attribute: attribute.to_string(), location: attribute_location, @@ -2249,21 +2251,13 @@ fn function_def_add_attribute( match &attribute { Attribute::Function(attribute) => { - function_modifiers.attributes.function = Some(attribute.clone()); + function_modifiers.attributes.set_function(attribute.clone()); } Attribute::Secondary(attribute) => { function_modifiers.attributes.secondary.push(attribute.clone()); } } - if let Attribute::Secondary( - SecondaryAttribute::Tag(attribute) | SecondaryAttribute::Meta(attribute), - ) = attribute - { - let func_meta = interpreter.elaborator.interner.function_meta_mut(&func_id); - func_meta.custom_attributes.push(attribute); - } - Ok(Value::Unit) } @@ -2295,7 +2289,7 @@ fn function_def_has_named_attribute( let name = &*get_str(interner, name)?; let modifiers = interner.function_modifiers(&func_id); - if let Some(attribute) = &modifiers.attributes.function { + if let Some(attribute) = modifiers.attributes.function() { if name == attribute.name() { return Ok(Value::Bool(true)); } @@ -2764,12 +2758,12 @@ fn trait_def_as_trait_constraint( /// Creates a value that holds an `Option`. /// `option_type` must be a Type referencing the `Option` type. -pub(crate) fn option(option_type: Type, value: Option) -> IResult { +pub(crate) fn option(option_type: Type, value: Option, span: Span) -> IResult { let t = extract_option_generic_type(option_type.clone()); let (is_some, value) = match value { Some(value) => (Value::Bool(true), value), - None => (Value::Bool(false), zeroed(t)?), + None => (Value::Bool(false), zeroed(t, span)?), }; let mut fields = HashMap::default(); @@ -2818,9 +2812,10 @@ fn derive_generators( _ => panic!("ICE: Should only have an array return type"), }; - let Some(num_generators) = size.evaluate_to_u32() else { - return Err(InterpreterError::UnknownArrayLength { length: *size, location }); - }; + let num_generators = size.evaluate_to_u32(location.span).map_err(|err| { + let err = Box::new(err); + InterpreterError::UnknownArrayLength { length: *size, err, location } + })?; let generators = bn254_blackbox_solver::derive_generators( &domain_separator_string, diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 9917207d217..51e62599b05 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -5,13 +5,13 @@ use crate::graph::CrateId; use crate::hir::comptime::InterpreterError; use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleId}; use crate::hir::resolution::errors::ResolverError; -use crate::hir::resolution::path_resolver; use crate::hir::type_check::TypeCheckError; +use crate::locations::ReferencesTracker; use crate::token::SecondaryAttribute; use crate::usage_tracker::UnusedItem; use crate::{Generics, Type}; -use crate::hir::resolution::import::{resolve_import, ImportDirective, PathResolution}; +use crate::hir::resolution::import::{resolve_import, ImportDirective}; use crate::hir::Context; use crate::ast::Expression; @@ -347,45 +347,23 @@ impl DefCollector { // Resolve unresolved imports collected from the crate, one by one. for collected_import in std::mem::take(&mut def_collector.imports) { - let module_id = collected_import.module_id; - let resolved_import = if context.def_interner.lsp_mode { - let mut references: Vec = Vec::new(); - let resolved_import = resolve_import( - crate_id, - &collected_import, - &context.def_interner, - &context.def_maps, - &mut context.usage_tracker, - &mut Some(&mut references), - ); - - let current_def_map = context.def_maps.get(&crate_id).unwrap(); - let file_id = current_def_map.file_id(module_id); + let local_module_id = collected_import.module_id; + let module_id = ModuleId { krate: crate_id, local_id: local_module_id }; + let current_def_map = context.def_maps.get(&crate_id).unwrap(); + let file_id = current_def_map.file_id(local_module_id); + + let resolved_import = resolve_import( + collected_import.path.clone(), + module_id, + &context.def_maps, + &mut context.usage_tracker, + Some(ReferencesTracker::new(&mut context.def_interner, file_id)), + ); - for (referenced, segment) in references.iter().zip(&collected_import.path.segments) - { - context.def_interner.add_reference( - *referenced, - Location::new(segment.ident.span(), file_id), - false, - ); - } - - resolved_import - } else { - resolve_import( - crate_id, - &collected_import, - &context.def_interner, - &context.def_maps, - &mut context.usage_tracker, - &mut None, - ) - }; match resolved_import { Ok(resolved_import) => { let current_def_map = context.def_maps.get_mut(&crate_id).unwrap(); - let file_id = current_def_map.file_id(module_id); + let file_id = current_def_map.file_id(local_module_id); let has_path_resolution_error = !resolved_import.errors.is_empty(); for error in resolved_import.errors { @@ -396,11 +374,11 @@ impl DefCollector { } // Populate module namespaces according to the imports used - let name = resolved_import.name; + let name = collected_import.name(); let visibility = collected_import.visibility; - let is_prelude = resolved_import.is_prelude; + let is_prelude = collected_import.is_prelude; for (module_def_id, item_visibility, _) in - resolved_import.resolved_namespace.iter_items() + resolved_import.namespace.iter_items() { if item_visibility < visibility { errors.push(( @@ -414,25 +392,26 @@ impl DefCollector { } let visibility = visibility.min(item_visibility); - let result = current_def_map.modules[resolved_import.module_scope.0] - .import(name.clone(), visibility, module_def_id, is_prelude); + let result = current_def_map.modules[local_module_id.0].import( + name.clone(), + visibility, + module_def_id, + is_prelude, + ); // If we error on path resolution don't also say it's unused (in case it ends up being unused) if !has_path_resolution_error { - let module_id = ModuleId { - krate: crate_id, - local_id: resolved_import.module_scope, - }; + let defining_module = + ModuleId { krate: crate_id, local_id: local_module_id }; + context.usage_tracker.add_unused_item( - module_id, + defining_module, name.clone(), UnusedItem::Import, visibility, ); if visibility != ItemVisibility::Private { - let local_id = resolved_import.module_scope; - let defining_module = ModuleId { krate: crate_id, local_id }; context.def_interner.register_name_for_auto_import( name.to_string(), module_def_id, @@ -442,14 +421,6 @@ impl DefCollector { } } - let last_segment = collected_import.path.last_ident(); - - add_import_reference( - module_def_id, - &last_segment, - &mut context.def_interner, - file_id, - ); if let Some(ref alias) = collected_import.alias { add_import_reference( module_def_id, @@ -558,17 +529,18 @@ fn inject_prelude( span: Span::default(), }; - if let Ok(PathResolution { item, errors }) = path_resolver::resolve_path( - &context.def_interner, - &context.def_maps, - ModuleId { krate: crate_id, local_id: crate_root }, - None, + if let Ok(resolved_import) = resolve_import( path, + ModuleId { krate: crate_id, local_id: crate_root }, + &context.def_maps, &mut context.usage_tracker, - &mut None, + None, // references tracker ) { - assert!(errors.is_empty(), "Tried to add private item to prelude"); - let module_id = item.module_id().expect("std::prelude should be a module"); + assert!(resolved_import.errors.is_empty(), "Tried to add private item to prelude"); + + let (module_def_id, _, _) = + resolved_import.namespace.types.expect("couldn't resolve std::prelude"); + let module_id = module_def_id.as_module().expect("std::prelude should be a module"); let prelude = context.module(module_id).scope().names(); for path in prelude { @@ -580,7 +552,6 @@ fn inject_prelude( ImportDirective { visibility: ItemVisibility::Private, module_id: crate_root, - self_type_module_id: None, path: Path { segments, kind: PathKind::Plain, span: Span::default() }, alias: None, is_prelude: true, diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 94c091f9791..a373441b4e0 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -81,7 +81,6 @@ pub fn collect_defs( collector.def_collector.imports.push(ImportDirective { visibility: import.visibility, module_id: collector.module_id, - self_type_module_id: None, path: import.path, alias: import.alias, is_prelude: false, @@ -542,7 +541,7 @@ impl<'a> ModCollector<'a> { name: Rc::new(name.to_string()), type_var: TypeVariable::unbound( type_variable_id, - Kind::Numeric(Box::new(typ)), + Kind::numeric(typ), ), span: name.span(), }); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/mod.rs index 60ee2c52842..de94f73b44b 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -169,7 +169,7 @@ impl CrateDefMap { module.value_definitions().filter_map(|id| { if let Some(func_id) = id.as_function() { let attributes = interner.function_attributes(&func_id); - match &attributes.function { + match attributes.function() { Some(FunctionAttribute::Test(scope)) => { let location = interner.function_meta(&func_id).name.location; Some(TestFunction::new(func_id, scope.clone(), location)) diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs index 67820bfd1b7..b82eafa5b9d 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -5,7 +5,7 @@ use thiserror::Error; use crate::{ ast::{Ident, UnsupportedNumericGenericType}, - hir::comptime::InterpreterError, + hir::{comptime::InterpreterError, type_check::TypeCheckError}, parser::ParserError, usage_tracker::UnusedItem, Type, @@ -125,11 +125,12 @@ pub enum ResolverError { NamedTypeArgs { span: Span, item_kind: &'static str }, #[error("Associated constants may only be a field or integer type")] AssociatedConstantsMustBeNumeric { span: Span }, - #[error("Overflow in `{lhs} {op} {rhs}`")] - OverflowInType { + #[error("Computing `{lhs} {op} {rhs}` failed with error {err}")] + BinaryOpError { lhs: FieldElement, op: crate::BinaryTypeOperator, rhs: FieldElement, + err: Box, span: Span, }, #[error("`quote` cannot be used in runtime code")] @@ -518,10 +519,10 @@ impl<'a> From<&'a ResolverError> for Diagnostic { *span, ) } - ResolverError::OverflowInType { lhs, op, rhs, span } => { + ResolverError::BinaryOpError { lhs, op, rhs, err, span } => { Diagnostic::simple_error( - format!("Overflow in `{lhs} {op} {rhs}`"), - "Overflow here".to_string(), + format!("Computing `{lhs} {op} {rhs}` failed with error {err}"), + String::new(), *span, ) } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/import.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/import.rs index 7afb056cbaf..376b85bfbd9 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -4,110 +4,37 @@ use thiserror::Error; use crate::graph::CrateId; use crate::hir::def_collector::dc_crate::CompilationError; -use crate::node_interner::{ - FuncId, GlobalId, NodeInterner, ReferenceId, StructId, TraitId, TypeAliasId, -}; +use crate::locations::ReferencesTracker; use crate::usage_tracker::UsageTracker; -use crate::Type; use std::collections::BTreeMap; -use crate::ast::{Ident, ItemVisibility, Path, PathKind, PathSegment, UnresolvedType}; -use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleDefId, ModuleId, PerNs}; +use crate::ast::{Ident, ItemVisibility, Path, PathKind}; +use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleData, ModuleDefId, ModuleId, PerNs}; use super::errors::ResolverError; -use super::visibility::can_reference_module_id; +use super::visibility::item_in_module_is_visible; #[derive(Debug, Clone)] pub struct ImportDirective { pub visibility: ItemVisibility, pub module_id: LocalModuleId, - pub self_type_module_id: Option, pub path: Path, pub alias: Option, pub is_prelude: bool, } -struct NamespaceResolution { - module_id: ModuleId, - item: PathResolutionItem, - namespace: PerNs, - errors: Vec, -} - -type NamespaceResolutionResult = Result; - -#[derive(Debug)] -pub struct PathResolution { - pub item: PathResolutionItem, - pub errors: Vec, -} - -/// All possible items that result from resolving a Path. -/// Note that this item doesn't include the last turbofish in a Path, -/// only intermediate ones, if any. -#[derive(Debug, Clone)] -pub enum PathResolutionItem { - Module(ModuleId), - Struct(StructId), - TypeAlias(TypeAliasId), - Trait(TraitId), - Global(GlobalId), - ModuleFunction(FuncId), - StructFunction(StructId, Option, FuncId), - TypeAliasFunction(TypeAliasId, Option, FuncId), - TraitFunction(TraitId, Option, FuncId), -} - -impl PathResolutionItem { - pub fn function_id(&self) -> Option { - match self { - PathResolutionItem::ModuleFunction(func_id) - | PathResolutionItem::StructFunction(_, _, func_id) - | PathResolutionItem::TypeAliasFunction(_, _, func_id) - | PathResolutionItem::TraitFunction(_, _, func_id) => Some(*func_id), - _ => None, - } - } - - pub fn module_id(&self) -> Option { - match self { - Self::Module(module_id) => Some(*module_id), - _ => None, - } - } - - pub fn description(&self) -> &'static str { - match self { - PathResolutionItem::Module(..) => "module", - PathResolutionItem::Struct(..) => "type", - PathResolutionItem::TypeAlias(..) => "type alias", - PathResolutionItem::Trait(..) => "trait", - PathResolutionItem::Global(..) => "global", - PathResolutionItem::ModuleFunction(..) - | PathResolutionItem::StructFunction(..) - | PathResolutionItem::TypeAliasFunction(..) - | PathResolutionItem::TraitFunction(..) => "function", +impl ImportDirective { + /// Returns the name that's brought into scope: either the alias or the last segment of the path + pub fn name(&self) -> Ident { + match &self.alias { + None => self.path.last_ident(), + Some(ident) => ident.clone(), } } } -#[derive(Debug, Clone)] -pub struct Turbofish { - pub generics: Vec, - pub span: Span, -} - -/// Any item that can appear before the last segment in a path. -#[derive(Debug)] -enum IntermediatePathResolutionItem { - Module(ModuleId), - Struct(StructId, Option), - TypeAlias(TypeAliasId, Option), - Trait(TraitId, Option), -} - -pub(crate) type PathResolutionResult = Result; +type ImportResolutionResult = Result; #[derive(Debug, Clone, PartialEq, Eq, Error)] pub enum PathResolutionError { @@ -119,19 +46,15 @@ pub enum PathResolutionError { NoSuper(Span), #[error("turbofish (`::<_>`) not allowed on {item}")] TurbofishNotAllowedOnItem { item: String, span: Span }, + #[error("{ident} is a {kind}, not a module")] + NotAModule { ident: Ident, kind: &'static str }, } #[derive(Debug)] pub struct ResolvedImport { - // name of the namespace, either last path segment or an alias - pub name: Ident, // The symbol which we have resolved to - pub resolved_namespace: PerNs, - // The item which we have resolved to - pub item: PathResolutionItem, + pub namespace: PerNs, // The module which we must add the resolved namespace to - pub module_scope: LocalModuleId, - pub is_prelude: bool, pub errors: Vec, } @@ -159,452 +82,255 @@ impl<'a> From<&'a PathResolutionError> for CustomDiagnostic { PathResolutionError::TurbofishNotAllowedOnItem { item: _, span } => { CustomDiagnostic::simple_error(error.to_string(), String::new(), *span) } + PathResolutionError::NotAModule { ident, kind: _ } => { + CustomDiagnostic::simple_error(error.to_string(), String::new(), ident.span()) + } } } } +/// Resolves a Path in a `use` statement, assuming it's located in `importing_module`. +/// +/// If the imported name can't be found, `Err` will be returned. If it can be found, `Ok` +/// will be returned with a potential list of errors if, for example, one of the segments +/// is not accessible from the importing module (e.g. because it's private). pub fn resolve_import( - crate_id: CrateId, - import_directive: &ImportDirective, - interner: &NodeInterner, + path: Path, + importing_module: ModuleId, def_maps: &BTreeMap, usage_tracker: &mut UsageTracker, - path_references: &mut Option<&mut Vec>, -) -> Result { - let module_scope = import_directive.module_id; - let NamespaceResolution { - module_id: resolved_module, - item, - namespace: resolved_namespace, - mut errors, - } = resolve_path_to_ns( - import_directive, - crate_id, - crate_id, - interner, - def_maps, - usage_tracker, - path_references, - )?; - - let name = resolve_path_name(import_directive); - - let visibility = resolved_namespace - .values - .or(resolved_namespace.types) - .map(|(_, visibility, _)| visibility) - .expect("Found empty namespace"); - - if !(import_directive.self_type_module_id == Some(resolved_module) - || can_reference_module_id( - def_maps, - crate_id, - import_directive.module_id, - resolved_module, - visibility, - )) - { - errors.push(PathResolutionError::Private(name.clone())); - } - - Ok(ResolvedImport { - name, - resolved_namespace, - item, - module_scope, - is_prelude: import_directive.is_prelude, - errors, - }) + references_tracker: Option, +) -> ImportResolutionResult { + let (path, module_id, references_tracker) = + resolve_path_kind(path, importing_module, def_maps, references_tracker)?; + let mut solver = + ImportSolver::new(importing_module, def_maps, usage_tracker, references_tracker); + solver.resolve_name_in_module(path, module_id) } -fn resolve_path_to_ns( - import_directive: &ImportDirective, - crate_id: CrateId, - importing_crate: CrateId, - interner: &NodeInterner, +/// Given a Path and a ModuleId it's being used in, this function returns a plain Path +/// and a ModuleId where that plain Path should be resolved. That is, this method will +/// resolve the Path kind and translate it to a plain path. +/// +/// The third value in the tuple is a reference tracker that must be passed to this +/// method, which is used in case the path kind is `dep`: the segment after `dep` +/// will be linked to the root module of the external dependency. +pub fn resolve_path_kind<'r>( + path: Path, + importing_module: ModuleId, def_maps: &BTreeMap, - usage_tracker: &mut UsageTracker, - path_references: &mut Option<&mut Vec>, -) -> NamespaceResolutionResult { - let import_path = &import_directive.path.segments; - - match import_directive.path.kind { - crate::ast::PathKind::Crate => { - // Resolve from the root of the crate - resolve_path_from_crate_root( - crate_id, - importing_crate, - import_path, - interner, - def_maps, - usage_tracker, - path_references, - ) + references_tracker: Option>, +) -> Result<(Path, ModuleId, Option>), PathResolutionError> { + let mut solver = + PathResolutionTargetResolver { importing_module, def_maps, references_tracker }; + let (path, module_id) = solver.resolve(path)?; + Ok((path, module_id, solver.references_tracker)) +} + +struct PathResolutionTargetResolver<'def_maps, 'references_tracker> { + importing_module: ModuleId, + def_maps: &'def_maps BTreeMap, + references_tracker: Option>, +} + +impl<'def_maps, 'references_tracker> PathResolutionTargetResolver<'def_maps, 'references_tracker> { + fn resolve(&mut self, path: Path) -> Result<(Path, ModuleId), PathResolutionError> { + match path.kind { + PathKind::Crate => self.resolve_crate_path(path), + PathKind::Plain => self.resolve_plain_path(path, self.importing_module), + PathKind::Dep => self.resolve_dep_path(path), + PathKind::Super => self.resolve_super_path(path), } - crate::ast::PathKind::Plain => { - // There is a possibility that the import path is empty - // In that case, early return - if import_path.is_empty() { - return resolve_name_in_module( - crate_id, - importing_crate, - import_path, - import_directive.module_id, - interner, - def_maps, - true, // plain or crate - usage_tracker, - path_references, - ); - } + } - let def_map = &def_maps[&crate_id]; - let current_mod_id = ModuleId { krate: crate_id, local_id: import_directive.module_id }; - let current_mod = &def_map.modules[current_mod_id.local_id.0]; - let first_segment = - &import_path.first().expect("ice: could not fetch first segment").ident; - if current_mod.find_name(first_segment).is_none() { - // Resolve externally when first segment is unresolved - return resolve_external_dep( - crate_id, - // def_map, - import_directive, - interner, - def_maps, - usage_tracker, - path_references, - importing_crate, - ); - } + fn resolve_crate_path(&mut self, path: Path) -> Result<(Path, ModuleId), PathResolutionError> { + let root_module = self.def_maps[&self.importing_module.krate].root; + let current_module = ModuleId { krate: self.importing_module.krate, local_id: root_module }; + Ok((path, current_module)) + } - resolve_name_in_module( - crate_id, - importing_crate, - import_path, - import_directive.module_id, - interner, - def_maps, - true, // plain or crate - usage_tracker, - path_references, - ) + fn resolve_plain_path( + &mut self, + path: Path, + current_module: ModuleId, + ) -> Result<(Path, ModuleId), PathResolutionError> { + // There is a possibility that the import path is empty. In that case, early return. + // This happens on import statements such as `use crate` or `use std`. + if path.segments.is_empty() { + return Ok((path, current_module)); } - crate::ast::PathKind::Dep => resolve_external_dep( - crate_id, - import_directive, - interner, - def_maps, - usage_tracker, - path_references, - importing_crate, - ), - - crate::ast::PathKind::Super => { - if let Some(parent_module_id) = - def_maps[&crate_id].modules[import_directive.module_id.0].parent - { - resolve_name_in_module( - crate_id, - importing_crate, - import_path, - parent_module_id, - interner, - def_maps, - false, // plain or crate - usage_tracker, - path_references, - ) - } else { - let span_start = import_directive.path.span().start(); - let span = Span::from(span_start..span_start + 5); // 5 == "super".len() - Err(PathResolutionError::NoSuper(span)) - } + let first_segment = + &path.segments.first().expect("ice: could not fetch first segment").ident; + if get_module(self.def_maps, current_module).find_name(first_segment).is_none() { + // Resolve externally when first segment is unresolved + return self.resolve_dep_path(path); + } + + Ok((path, current_module)) + } + + fn resolve_dep_path( + &mut self, + mut path: Path, + ) -> Result<(Path, ModuleId), PathResolutionError> { + // Use extern_prelude to get the dep + let current_def_map = &self.def_maps[&self.importing_module.krate]; + + // Fetch the root module from the prelude + let crate_name = &path.segments.first().unwrap().ident; + let dep_module = current_def_map + .extern_prelude + .get(&crate_name.0.contents) + .ok_or_else(|| PathResolutionError::Unresolved(crate_name.to_owned()))?; + + if let Some(references_tracker) = &mut self.references_tracker { + let span = crate_name.span(); + references_tracker.add_reference(ModuleDefId::ModuleId(*dep_module), span, false); } + + // Now the path can be solved starting from the second segment as a plain path + path.kind = PathKind::Plain; + path.segments.remove(0); + + Ok((path, *dep_module)) + } + + fn resolve_super_path(&mut self, path: Path) -> Result<(Path, ModuleId), PathResolutionError> { + let Some(parent_module_id) = get_module(self.def_maps, self.importing_module).parent else { + let span_start = path.span.start(); + let span = Span::from(span_start..span_start + 5); // 5 == "super".len() + return Err(PathResolutionError::NoSuper(span)); + }; + + let current_module = + ModuleId { krate: self.importing_module.krate, local_id: parent_module_id }; + Ok((path, current_module)) } } -fn resolve_path_from_crate_root( - crate_id: CrateId, - importing_crate: CrateId, - import_path: &[PathSegment], - interner: &NodeInterner, - def_maps: &BTreeMap, - usage_tracker: &mut UsageTracker, - path_references: &mut Option<&mut Vec>, -) -> NamespaceResolutionResult { - let starting_mod = def_maps[&crate_id].root; - resolve_name_in_module( - crate_id, - importing_crate, - import_path, - starting_mod, - interner, - def_maps, - true, // plain or crate - usage_tracker, - path_references, - ) +struct ImportSolver<'def_maps, 'usage_tracker, 'references_tracker> { + importing_module: ModuleId, + def_maps: &'def_maps BTreeMap, + usage_tracker: &'usage_tracker mut UsageTracker, + references_tracker: Option>, } -#[allow(clippy::too_many_arguments)] -fn resolve_name_in_module( - krate: CrateId, - importing_crate: CrateId, - import_path: &[PathSegment], - starting_mod: LocalModuleId, - interner: &NodeInterner, - def_maps: &BTreeMap, - plain_or_crate: bool, - usage_tracker: &mut UsageTracker, - path_references: &mut Option<&mut Vec>, -) -> NamespaceResolutionResult { - let def_map = &def_maps[&krate]; - let mut current_mod_id = ModuleId { krate, local_id: starting_mod }; - let mut current_mod = &def_map.modules[current_mod_id.local_id.0]; - - let mut intermediate_item = IntermediatePathResolutionItem::Module(current_mod_id); - - // There is a possibility that the import path is empty - // In that case, early return - if import_path.is_empty() { - return Ok(NamespaceResolution { - module_id: current_mod_id, - item: PathResolutionItem::Module(current_mod_id), - namespace: PerNs::types(current_mod_id.into()), - errors: Vec::new(), - }); +impl<'def_maps, 'usage_tracker, 'references_tracker> + ImportSolver<'def_maps, 'usage_tracker, 'references_tracker> +{ + fn new( + importing_module: ModuleId, + def_maps: &'def_maps BTreeMap, + usage_tracker: &'usage_tracker mut UsageTracker, + references_tracker: Option>, + ) -> Self { + Self { importing_module, def_maps, usage_tracker, references_tracker } } - let first_segment = &import_path.first().expect("ice: could not fetch first segment").ident; - let mut current_ns = current_mod.find_name(first_segment); - if current_ns.is_none() { - return Err(PathResolutionError::Unresolved(first_segment.clone())); - } + fn resolve_name_in_module( + &mut self, + path: Path, + starting_module: ModuleId, + ) -> ImportResolutionResult { + // There is a possibility that the import path is empty. In that case, early return. + if path.segments.is_empty() { + return Ok(ResolvedImport { + namespace: PerNs::types(starting_module.into()), + errors: Vec::new(), + }); + } - usage_tracker.mark_as_referenced(current_mod_id, first_segment); + let plain_or_crate = matches!(path.kind, PathKind::Plain | PathKind::Crate); - let mut errors = Vec::new(); - for (index, (last_segment, current_segment)) in - import_path.iter().zip(import_path.iter().skip(1)).enumerate() - { - let last_ident = &last_segment.ident; - let current_ident = ¤t_segment.ident; - let last_segment_generics = &last_segment.generics; + // The current module and module ID as we resolve path segments + let mut current_module_id = starting_module; + let mut current_module = get_module(self.def_maps, starting_module); - let (typ, visibility) = match current_ns.types { - None => return Err(PathResolutionError::Unresolved(last_ident.clone())), - Some((typ, visibility, _)) => (typ, visibility), - }; + let first_segment = + &path.segments.first().expect("ice: could not fetch first segment").ident; + let mut current_ns = current_module.find_name(first_segment); + if current_ns.is_none() { + return Err(PathResolutionError::Unresolved(first_segment.clone())); + } - // In the type namespace, only Mod can be used in a path. - (current_mod_id, intermediate_item) = match typ { - ModuleDefId::ModuleId(id) => { - if let Some(path_references) = path_references { - path_references.push(ReferenceId::Module(id)); - } + self.usage_tracker.mark_as_referenced(current_module_id, first_segment); - if last_segment_generics.is_some() { - errors.push(PathResolutionError::TurbofishNotAllowedOnItem { - item: format!("module `{last_ident}`"), - span: last_segment.turbofish_span(), + let mut errors = Vec::new(); + for (index, (last_segment, current_segment)) in + path.segments.iter().zip(path.segments.iter().skip(1)).enumerate() + { + let last_ident = &last_segment.ident; + let current_ident = ¤t_segment.ident; + + let (typ, visibility) = match current_ns.types { + None => return Err(PathResolutionError::Unresolved(last_ident.clone())), + Some((typ, visibility, _)) => (typ, visibility), + }; + + self.add_reference(typ, last_segment.span, last_segment.ident.is_self_type_name()); + + // In the type namespace, only Mod can be used in a path. + current_module_id = match typ { + ModuleDefId::ModuleId(id) => id, + ModuleDefId::TypeId(id) => id.module_id(), + ModuleDefId::TypeAliasId(..) => { + return Err(PathResolutionError::NotAModule { + ident: last_segment.ident.clone(), + kind: "type alias", }); } - - (id, IntermediatePathResolutionItem::Module(id)) + ModuleDefId::TraitId(id) => id.0, + ModuleDefId::FunctionId(_) => panic!("functions cannot be in the type namespace"), + ModuleDefId::GlobalId(_) => panic!("globals cannot be in the type namespace"), + }; + + // If the path is plain or crate, the first segment will always refer to + // something that's visible from the current module. + if !((plain_or_crate && index == 0) + || self.item_in_module_is_visible(current_module_id, visibility)) + { + errors.push(PathResolutionError::Private(last_ident.clone())); } - ModuleDefId::FunctionId(_) => panic!("functions cannot be in the type namespace"), - // TODO: If impls are ever implemented, types can be used in a path - ModuleDefId::TypeId(id) => { - if let Some(path_references) = path_references { - path_references.push(ReferenceId::Struct(id)); - } - ( - id.module_id(), - IntermediatePathResolutionItem::Struct( - id, - last_segment_generics.as_ref().map(|generics| Turbofish { - generics: generics.clone(), - span: last_segment.turbofish_span(), - }), - ), - ) - } - ModuleDefId::TypeAliasId(id) => { - if let Some(path_references) = path_references { - path_references.push(ReferenceId::Alias(id)); - } + current_module = + &self.def_maps[¤t_module_id.krate].modules[current_module_id.local_id.0]; - let type_alias = interner.get_type_alias(id); - let type_alias = type_alias.borrow(); - - let module_id = match &type_alias.typ { - Type::Struct(struct_id, _generics) => struct_id.borrow().id.module_id(), - Type::Error => { - return Err(PathResolutionError::Unresolved(last_ident.clone())); - } - _ => { - // For now we only allow type aliases that point to structs. - // 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") - } - }; - - ( - module_id, - IntermediatePathResolutionItem::TypeAlias( - id, - last_segment_generics.as_ref().map(|generics| Turbofish { - generics: generics.clone(), - span: last_segment.turbofish_span(), - }), - ), - ) + // Check if namespace + let found_ns = current_module.find_name(current_ident); + if found_ns.is_none() { + return Err(PathResolutionError::Unresolved(current_ident.clone())); } - ModuleDefId::TraitId(id) => { - if let Some(path_references) = path_references { - path_references.push(ReferenceId::Trait(id)); - } - ( - id.0, - IntermediatePathResolutionItem::Trait( - id, - last_segment_generics.as_ref().map(|generics| Turbofish { - generics: generics.clone(), - span: last_segment.turbofish_span(), - }), - ), - ) - } - ModuleDefId::GlobalId(_) => panic!("globals cannot be in the type namespace"), - }; + self.usage_tracker.mark_as_referenced(current_module_id, current_ident); - // If the path is plain or crate, the first segment will always refer to - // something that's visible from the current module. - if !((plain_or_crate && index == 0) - || can_reference_module_id( - def_maps, - importing_crate, - starting_mod, - current_mod_id, - visibility, - )) - { - errors.push(PathResolutionError::Private(last_ident.clone())); + current_ns = found_ns; } - current_mod = &def_maps[¤t_mod_id.krate].modules[current_mod_id.local_id.0]; + let (module_def_id, visibility, _) = + current_ns.values.or(current_ns.types).expect("Found empty namespace"); - // Check if namespace - let found_ns = current_mod.find_name(current_ident); + self.add_reference(module_def_id, path.segments.last().unwrap().ident.span(), false); - if found_ns.is_none() { - return Err(PathResolutionError::Unresolved(current_ident.clone())); + if !self.item_in_module_is_visible(current_module_id, visibility) { + errors.push(PathResolutionError::Private(path.last_ident())); } - usage_tracker.mark_as_referenced(current_mod_id, current_ident); - - current_ns = found_ns; + Ok(ResolvedImport { namespace: current_ns, errors }) } - let module_def_id = - current_ns.values.or(current_ns.types).map(|(id, _, _)| id).expect("Found empty namespace"); - - let item = merge_intermediate_path_resolution_item_with_module_def_id( - intermediate_item, - module_def_id, - ); - - Ok(NamespaceResolution { module_id: current_mod_id, item, namespace: current_ns, errors }) -} - -fn resolve_path_name(import_directive: &ImportDirective) -> Ident { - match &import_directive.alias { - None => import_directive.path.last_ident(), - Some(ident) => ident.clone(), + fn add_reference(&mut self, reference_id: ModuleDefId, span: Span, is_self_type_name: bool) { + if let Some(references_tracker) = &mut self.references_tracker { + references_tracker.add_reference(reference_id, span, is_self_type_name); + } } -} -fn resolve_external_dep( - crate_id: CrateId, - directive: &ImportDirective, - interner: &NodeInterner, - def_maps: &BTreeMap, - usage_tracker: &mut UsageTracker, - path_references: &mut Option<&mut Vec>, - importing_crate: CrateId, -) -> NamespaceResolutionResult { - // Use extern_prelude to get the dep - let path = &directive.path.segments; - - let current_def_map = &def_maps[&crate_id]; - - // Fetch the root module from the prelude - let crate_name = &path.first().unwrap().ident; - let dep_module = current_def_map - .extern_prelude - .get(&crate_name.0.contents) - .ok_or_else(|| PathResolutionError::Unresolved(crate_name.to_owned()))?; - - // Create an import directive for the dependency crate - // XXX: This will panic if the path is of the form `use std`. Ideal algorithm will not distinguish between crate and module - // See `singleton_import.nr` test case for a check that such cases are handled elsewhere. - let path_without_crate_name = &path[1..]; - - if let Some(path_references) = path_references { - path_references.push(ReferenceId::Module(*dep_module)); + fn item_in_module_is_visible(&self, module: ModuleId, visibility: ItemVisibility) -> bool { + item_in_module_is_visible(self.def_maps, self.importing_module, module, visibility) } - - let path = Path { - segments: path_without_crate_name.to_vec(), - kind: PathKind::Plain, - span: Span::default(), - }; - let dep_directive = ImportDirective { - visibility: ItemVisibility::Private, - module_id: dep_module.local_id, - self_type_module_id: directive.self_type_module_id, - path, - alias: directive.alias.clone(), - is_prelude: false, - }; - - resolve_path_to_ns( - &dep_directive, - dep_module.krate, - importing_crate, - interner, - def_maps, - usage_tracker, - path_references, - ) } -fn merge_intermediate_path_resolution_item_with_module_def_id( - intermediate_item: IntermediatePathResolutionItem, - module_def_id: ModuleDefId, -) -> PathResolutionItem { - match module_def_id { - ModuleDefId::ModuleId(module_id) => PathResolutionItem::Module(module_id), - ModuleDefId::TypeId(struct_id) => PathResolutionItem::Struct(struct_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::TypeAlias(alias_id, generics) => { - PathResolutionItem::TypeAliasFunction(alias_id, generics, func_id) - } - IntermediatePathResolutionItem::Trait(trait_id, generics) => { - PathResolutionItem::TraitFunction(trait_id, generics, func_id) - } - }, - } +fn get_module(def_maps: &BTreeMap, module: ModuleId) -> &ModuleData { + let message = "A crate should always be present for a given crate id"; + &def_maps.get(&module.krate).expect(message).modules[module.local_id.0] } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/mod.rs index 223b88b5c5d..44a0114074e 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/mod.rs @@ -7,5 +7,4 @@ //! will have the same DefinitionId. pub mod errors; pub mod import; -pub mod path_resolver; pub mod visibility; diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs deleted file mode 100644 index 98d4aee3f5c..00000000000 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs +++ /dev/null @@ -1,102 +0,0 @@ -use super::import::{resolve_import, ImportDirective, PathResolution, PathResolutionResult}; -use crate::ast::{ItemVisibility, Path}; -use crate::node_interner::{NodeInterner, ReferenceId}; -use crate::usage_tracker::UsageTracker; - -use std::collections::BTreeMap; - -use crate::graph::CrateId; -use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleId}; - -pub trait PathResolver { - /// Resolve the given path returning the resolved ModuleDefId. - /// If `path_references` is `Some`, a `ReferenceId` for each segment in `path` - /// will be resolved and pushed (some entries will be None if they don't refer to - /// a module or type). - fn resolve( - &self, - interner: &NodeInterner, - def_maps: &BTreeMap, - path: Path, - usage_tracker: &mut UsageTracker, - path_references: &mut Option<&mut Vec>, - ) -> PathResolutionResult; - - fn local_module_id(&self) -> LocalModuleId; - - fn module_id(&self) -> ModuleId; -} - -pub struct StandardPathResolver { - // Module that we are resolving the path in - module_id: ModuleId, - // The module of the self type, if any (for example, the ModuleId of a struct) - self_type_module_id: Option, -} - -impl StandardPathResolver { - pub fn new(module_id: ModuleId, self_type_module_id: Option) -> StandardPathResolver { - Self { module_id, self_type_module_id } - } -} - -impl PathResolver for StandardPathResolver { - fn resolve( - &self, - interner: &NodeInterner, - def_maps: &BTreeMap, - path: Path, - usage_tracker: &mut UsageTracker, - path_references: &mut Option<&mut Vec>, - ) -> PathResolutionResult { - resolve_path( - interner, - def_maps, - self.module_id, - self.self_type_module_id, - path, - usage_tracker, - path_references, - ) - } - - fn local_module_id(&self) -> LocalModuleId { - self.module_id.local_id - } - - fn module_id(&self) -> ModuleId { - self.module_id - } -} - -/// Resolve the given path to a function or a type. -/// In the case of a conflict, functions are given priority -pub fn resolve_path( - interner: &NodeInterner, - def_maps: &BTreeMap, - module_id: ModuleId, - self_type_module_id: Option, - path: Path, - usage_tracker: &mut UsageTracker, - path_references: &mut Option<&mut Vec>, -) -> PathResolutionResult { - // lets package up the path into an ImportDirective and resolve it using that - let import = ImportDirective { - visibility: ItemVisibility::Private, - module_id: module_id.local_id, - self_type_module_id, - path, - alias: None, - is_prelude: false, - }; - let resolved_import = resolve_import( - module_id.krate, - &import, - interner, - def_maps, - usage_tracker, - path_references, - )?; - - Ok(PathResolution { item: resolved_import.item, errors: resolved_import.errors }) -} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/visibility.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/visibility.rs index 492f303d2c4..c2fe887fe62 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/visibility.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/visibility.rs @@ -7,34 +7,43 @@ use std::collections::BTreeMap; use crate::ast::ItemVisibility; use crate::hir::def_map::{CrateDefMap, DefMaps, LocalModuleId, ModuleId}; -// Returns false if the given private function is being called from a non-child module, or -// if the given pub(crate) function is being called from another crate. Otherwise returns true. -pub fn can_reference_module_id( +/// Returns true if an item with the given visibility in the target module +/// is visible from the current module. For example: +/// ```text +/// mod foo { +/// ^^^ <-- target module +/// pub(crate) fn bar() {} +/// ^^^^^^^^^^ <- visibility +/// } +/// ``` +pub fn item_in_module_is_visible( def_maps: &BTreeMap, - importing_crate: CrateId, - current_module: LocalModuleId, + current_module: ModuleId, target_module: ModuleId, visibility: ItemVisibility, ) -> bool { // Note that if the target module is in a different crate from the current module then we will either // return true as the target module is public or return false as it is private without looking at the `CrateDefMap` in either case. - let same_crate = target_module.krate == importing_crate; + let same_crate = target_module.krate == current_module.krate; match visibility { ItemVisibility::Public => true, ItemVisibility::PublicCrate => same_crate, ItemVisibility::Private => { + if !same_crate { + return false; + } + let target_crate_def_map = &def_maps[&target_module.krate]; - same_crate - && (module_descendent_of_target( - target_crate_def_map, - target_module.local_id, - current_module, - ) || module_is_parent_of_struct_module( - target_crate_def_map, - current_module, - target_module.local_id, - )) + module_descendent_of_target( + target_crate_def_map, + target_module.local_id, + current_module.local_id, + ) || module_is_parent_of_struct_module( + target_crate_def_map, + current_module.local_id, + target_module.local_id, + ) } } } @@ -116,10 +125,9 @@ pub fn method_call_is_visible( ItemVisibility::Private => { if object_type.is_primitive() { let func_module = interner.function_module(func_id); - can_reference_module_id( + item_in_module_is_visible( def_maps, - current_module.krate, - current_module.local_id, + current_module, func_module, modifiers.visibility, ) diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/type_check/errors.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/type_check/errors.rs index cb69053b72a..a63601a4280 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/type_check/errors.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/type_check/errors.rs @@ -11,7 +11,7 @@ use crate::ast::{ use crate::hir::resolution::errors::ResolverError; use crate::hir_def::expr::HirBinaryOp; use crate::hir_def::traits::TraitConstraint; -use crate::hir_def::types::Type; +use crate::hir_def::types::{BinaryTypeOperator, Kind, Type}; use crate::node_interner::NodeInterner; #[derive(Error, Debug, Clone, PartialEq, Eq)] @@ -34,12 +34,22 @@ pub enum Source { Return(FunctionReturnType, Span), } -#[derive(Error, Debug, Clone)] +#[derive(Error, Debug, Clone, PartialEq, Eq)] pub enum TypeCheckError { #[error("Operator {op:?} cannot be used in a {place:?}")] OpCannotBeUsed { op: HirBinaryOp, place: &'static str, span: Span }, + #[error("Division by zero: {lhs} / {rhs}")] + DivisionByZero { lhs: FieldElement, rhs: FieldElement, span: Span }, + #[error("Modulo on Field elements: {lhs} % {rhs}")] + ModuloOnFields { lhs: FieldElement, rhs: FieldElement, span: Span }, #[error("The value `{expr:?}` cannot fit into `{ty}` which has range `{range}`")] OverflowingAssignment { expr: FieldElement, ty: Type, range: String, span: Span }, + #[error( + "The value `{value}` cannot fit into `{kind}` which has a maximum size of `{maximum_size}`" + )] + OverflowingConstant { value: FieldElement, kind: Kind, maximum_size: FieldElement, span: Span }, + #[error("Evaluating `{op}` on `{lhs}`, `{rhs}` failed")] + FailingBinaryOp { op: BinaryTypeOperator, lhs: i128, rhs: i128, span: Span }, #[error("Type {typ:?} cannot be used in a {place:?}")] TypeCannotBeUsed { typ: Type, place: &'static str, span: Span }, #[error("Expected type {expected_typ:?} is not the same as {expr_typ:?}")] @@ -48,6 +58,14 @@ pub enum TypeCheckError { TypeMismatchWithSource { expected: Type, actual: Type, span: Span, source: Source }, #[error("Expected type {expected_kind:?} is not the same as {expr_kind:?}")] TypeKindMismatch { expected_kind: String, expr_kind: String, expr_span: Span }, + #[error("Evaluating {to} resulted in {to_value}, but {from_value} was expected")] + TypeCanonicalizationMismatch { + to: Type, + from: Type, + to_value: FieldElement, + from_value: FieldElement, + span: Span, + }, // TODO(https://github.com/noir-lang/noir/issues/6238): implement handling for larger types #[error("Expected type {expected_kind} when evaluating globals, but found {expr_kind} (this warning may become an error in the future)")] EvaluatedGlobalIsntU32 { expected_kind: String, expr_kind: String, expr_span: Span }, @@ -158,6 +176,8 @@ pub enum TypeCheckError { Unsafe { span: Span }, #[error("Converting an unconstrained fn to a non-unconstrained fn is unsafe")] UnsafeFn { span: Span }, + #[error("Expected a constant, but found `{typ}`")] + NonConstantEvaluated { typ: Type, span: Span }, #[error("Slices must have constant length")] NonConstantSliceLength { span: Span }, #[error("Only sized types may be used in the entry point to a program")] @@ -196,6 +216,10 @@ impl TypeCheckError { pub fn add_context(self, ctx: &'static str) -> Self { TypeCheckError::Context { err: Box::new(self), ctx } } + + pub(crate) fn is_non_constant_evaluated(&self) -> bool { + matches!(self, TypeCheckError::NonConstantEvaluated { .. }) + } } impl<'a> From<&'a TypeCheckError> for Diagnostic { @@ -216,6 +240,16 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic { String::new(), *span, ), + TypeCheckError::DivisionByZero { lhs, rhs, span } => Diagnostic::simple_error( + format!("Division by zero: {lhs} / {rhs}"), + String::new(), + *span, + ), + TypeCheckError::ModuloOnFields { lhs, rhs, span } => Diagnostic::simple_error( + format!("Modulo on Field elements: {lhs} % {rhs}"), + String::new(), + *span, + ), TypeCheckError::TypeMismatch { expected_typ, expr_typ, expr_span } => { Diagnostic::simple_error( format!("Expected type {expected_typ}, found type {expr_typ}"), @@ -230,6 +264,13 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic { *expr_span, ) } + TypeCheckError::TypeCanonicalizationMismatch { to, from, to_value, from_value, span } => { + Diagnostic::simple_error( + format!("Evaluating {to} resulted in {to_value}, but {from_value} was expected"), + format!("from evaluating {from} without simplifications"), + *span, + ) + } // TODO(https://github.com/noir-lang/noir/issues/6238): implement // handling for larger types TypeCheckError::EvaluatedGlobalIsntU32 { expected_kind, expr_kind, expr_span } => { @@ -320,11 +361,14 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic { | TypeCheckError::AmbiguousBitWidth { span, .. } | TypeCheckError::IntegerAndFieldBinaryOperation { span } | TypeCheckError::OverflowingAssignment { span, .. } + | TypeCheckError::OverflowingConstant { span, .. } + | TypeCheckError::FailingBinaryOp { span, .. } | TypeCheckError::FieldModulo { span } | TypeCheckError::FieldNot { span } | TypeCheckError::ConstrainedReferenceToUnconstrained { span } | TypeCheckError::UnconstrainedReferenceToConstrained { span } | TypeCheckError::UnconstrainedSliceReturnToConstrained { span } + | TypeCheckError::NonConstantEvaluated { span, .. } | TypeCheckError::NonConstantSliceLength { span } | TypeCheckError::StringIndexAssign { span } | TypeCheckError::InvalidShiftSize { span } => { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/function.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/function.rs index 6ecfdefe996..db6c3507b15 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/function.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/function.rs @@ -9,7 +9,7 @@ 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::token::CustomAttribute; + use crate::{ResolvedGeneric, Type}; /// A Hir function is a block expression with a list of statements. @@ -164,9 +164,6 @@ pub struct FuncMeta { /// If this function is from an impl (trait or regular impl), this /// is the object type of the impl. Otherwise this is None. pub self_type: Option, - - /// Custom attributes attached to this function. - pub custom_attributes: Vec, } #[derive(Debug, Clone)] diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/stmt.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/stmt.rs index b97e99583bb..0258cfd8ddb 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/stmt.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/stmt.rs @@ -105,7 +105,7 @@ impl HirPattern { } } - pub(crate) fn location(&self) -> Location { + pub fn location(&self) -> Location { match self { HirPattern::Identifier(ident) => ident.location, HirPattern::Mutable(_, location) diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/types.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/types.rs index 0eed79348e2..a0fea3aa774 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/types.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/types.rs @@ -5,6 +5,9 @@ use std::{ rc::Rc, }; +#[cfg(test)] +use proptest_derive::Arbitrary; + use acvm::{AcirField, FieldElement}; use crate::{ @@ -91,6 +94,14 @@ pub enum Type { /// like `fn foo(...) {}`. Unlike TypeVariables, they cannot be bound over. NamedGeneric(TypeVariable, Rc), + /// A cast (to, from) that's checked at monomorphization. + /// + /// Simplifications on arithmetic generics are only allowed on the LHS. + CheckedCast { + from: Box, + to: Box, + }, + /// A functions with arguments, a return type and environment. /// the environment should be `Unit` by default, /// for closures it should contain a `Tuple` type with the captured @@ -161,6 +172,11 @@ pub enum Kind { } impl Kind { + // Kind::Numeric constructor helper + pub fn numeric(typ: Type) -> Kind { + Kind::Numeric(Box::new(typ)) + } + pub(crate) fn is_error(&self) -> bool { match self.follow_bindings() { Self::Numeric(typ) => *typ == Type::Error, @@ -188,7 +204,7 @@ impl Kind { } pub(crate) fn u32() -> Self { - Self::Numeric(Box::new(Type::Integer(Signedness::Unsigned, IntegerBitSize::ThirtyTwo))) + Self::numeric(Type::Integer(Signedness::Unsigned, IntegerBitSize::ThirtyTwo)) } pub(crate) fn follow_bindings(&self) -> Self { @@ -197,7 +213,7 @@ impl Kind { Self::Normal => Self::Normal, Self::Integer => Self::Integer, Self::IntegerOrField => Self::IntegerOrField, - Self::Numeric(typ) => Self::Numeric(Box::new(typ.follow_bindings())), + Self::Numeric(typ) => Self::numeric(typ.follow_bindings()), } } @@ -255,10 +271,21 @@ impl Kind { } /// Ensure the given value fits in self.integral_maximum_size() - fn ensure_value_fits(&self, value: FieldElement) -> Option { + fn ensure_value_fits( + &self, + value: FieldElement, + span: Span, + ) -> Result { match self.integral_maximum_size() { - None => Some(value), - Some(maximum_size) => (value <= maximum_size).then_some(value), + None => Ok(value), + Some(maximum_size) => (value <= maximum_size).then_some(value).ok_or_else(|| { + TypeCheckError::OverflowingConstant { + value, + kind: self.clone(), + maximum_size, + span, + } + }), } } } @@ -652,6 +679,7 @@ impl Shared { /// A restricted subset of binary operators useable on /// type level integers for use in the array length positions of types. +#[cfg_attr(test, derive(Arbitrary))] #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)] pub enum BinaryTypeOperator { Addition, @@ -863,6 +891,7 @@ impl std::fmt::Display for Type { TypeBinding::Unbound(_, _) if name.is_empty() => write!(f, "_"), TypeBinding::Unbound(_, _) => write!(f, "{name}"), }, + Type::CheckedCast { to, .. } => write!(f, "{to}"), Type::Constant(x, _kind) => write!(f, "{x}"), Type::Forall(typevars, typ) => { let typevars = vecmap(typevars, |var| var.id().to_string()); @@ -887,7 +916,7 @@ impl std::fmt::Display for Type { } Type::Quoted(quoted) => write!(f, "{}", quoted), Type::InfixExpr(lhs, op, rhs) => { - let this = self.canonicalize(); + let this = self.canonicalize_checked(); // Prevent infinite recursion if this != *self { @@ -1064,6 +1093,7 @@ impl Type { | Type::TypeVariable(..) | Type::TraitAsType(..) | Type::NamedGeneric(..) + | Type::CheckedCast { .. } | Type::Forall(..) | Type::Constant(..) | Type::Quoted(..) @@ -1106,6 +1136,10 @@ impl Type { Type::NamedGeneric(_, _) => { named_generic_is_numeric(self, found_names); } + Type::CheckedCast { from, to } => { + to.find_numeric_type_vars(found_names); + from.find_numeric_type_vars(found_names); + } Type::TraitAsType(_, _, args) => { for arg in args.ordered.iter() { @@ -1187,9 +1221,10 @@ impl Type { | Type::Forall(_, _) | Type::Quoted(_) | Type::Slice(_) - | Type::InfixExpr(_, _, _) | Type::TraitAsType(..) => false, + Type::CheckedCast { to, .. } => to.is_valid_for_program_input(), + Type::Alias(alias, generics) => { let alias = alias.borrow(); alias.get_type(generics).is_valid_for_program_input() @@ -1205,6 +1240,10 @@ impl Type { .get_fields(generics) .into_iter() .all(|(_, field)| field.is_valid_for_program_input()), + + Type::InfixExpr(lhs, _, rhs) => { + lhs.is_valid_for_program_input() && rhs.is_valid_for_program_input() + } } } @@ -1239,6 +1278,8 @@ impl Type { | Type::Quoted(_) | Type::TraitAsType(..) => false, + Type::CheckedCast { to, .. } => to.is_valid_non_inlined_function_input(), + Type::Alias(alias, generics) => { let alias = alias.borrow(); alias.get_type(generics).is_valid_non_inlined_function_input() @@ -1280,6 +1321,8 @@ impl Type { } } + Type::CheckedCast { to, .. } => to.is_valid_for_unconstrained_boundary(), + // Quoted objects only exist at compile-time where the only execution // environment is the interpreter. In this environment, they are valid. Type::Quoted(_) => true, @@ -1312,6 +1355,7 @@ impl Type { pub fn generic_count(&self) -> usize { match self { Type::Forall(generics, _) => generics.len(), + Type::CheckedCast { to, .. } => to.generic_count(), Type::TypeVariable(type_variable) | Type::NamedGeneric(type_variable, _) => { match &*type_variable.borrow() { TypeBinding::Bound(binding) => binding.generic_count(), @@ -1351,6 +1395,7 @@ impl Type { pub(crate) fn kind(&self) -> Kind { match self { + Type::CheckedCast { to, .. } => to.kind(), Type::NamedGeneric(var, _) => var.kind(), Type::Constant(_, kind) => kind.clone(), Type::TypeVariable(var) => match &*var.borrow() { @@ -1386,32 +1431,33 @@ impl Type { if self_kind.unifies(&other_kind) { self_kind } else { - Kind::Numeric(Box::new(Type::Error)) + Kind::numeric(Type::Error) } } /// Returns the number of field elements required to represent the type once encoded. - pub fn field_count(&self) -> u32 { + pub fn field_count(&self, location: &Location) -> u32 { match self { Type::FieldElement | Type::Integer { .. } | Type::Bool => 1, Type::Array(size, typ) => { let length = size - .evaluate_to_u32() + .evaluate_to_u32(location.span) .expect("Cannot have variable sized arrays as a parameter to main"); let typ = typ.as_ref(); - length * typ.field_count() + length * typ.field_count(location) } Type::Struct(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()) + fields.iter().fold(0, |acc, (_, field_type)| acc + field_type.field_count(location)) } - Type::Alias(def, generics) => def.borrow().get_type(generics).field_count(), + Type::CheckedCast { to, .. } => to.field_count(location), + Type::Alias(def, generics) => def.borrow().get_type(generics).field_count(location), Type::Tuple(fields) => { - fields.iter().fold(0, |acc, field_typ| acc + field_typ.field_count()) + fields.iter().fold(0, |acc, field_typ| acc + field_typ.field_count(location)) } Type::String(size) => size - .evaluate_to_u32() + .evaluate_to_u32(location.span) .expect("Cannot have variable sized strings as a parameter to main"), Type::FmtString(_, _) | Type::Unit @@ -1582,6 +1628,7 @@ impl Type { match self { Type::TypeVariable(var) => Some((var.1.clone(), var.kind())), Type::NamedGeneric(var, _) => Some((var.1.clone(), var.kind())), + Type::CheckedCast { to, .. } => to.get_inner_type_variable(), _ => None, } } @@ -1696,6 +1743,10 @@ impl Type { } } + (CheckedCast { to, .. }, other) | (other, CheckedCast { to, .. }) => { + to.try_unify(other, bindings) + } + (NamedGeneric(binding, _), other) | (other, NamedGeneric(binding, _)) if !binding.borrow().is_unbound() => { @@ -1758,7 +1809,8 @@ impl Type { } (Constant(value, kind), other) | (other, Constant(value, kind)) => { - if let Some(other_value) = other.evaluate_to_field_element(kind) { + let dummy_span = Span::default(); + if let Ok(other_value) = other.evaluate_to_field_element(kind, dummy_span) { if *value == other_value && kind.unifies(&other.kind()) { Ok(()) } else { @@ -1926,41 +1978,106 @@ impl Type { /// If this type is a Type::Constant (used in array lengths), or is bound /// to a Type::Constant, return the constant as a u32. - pub fn evaluate_to_u32(&self) -> Option { - self.evaluate_to_field_element(&Kind::u32()) - .and_then(|field_element| field_element.try_to_u32()) + pub fn evaluate_to_u32(&self, span: Span) -> Result { + self.evaluate_to_field_element(&Kind::u32(), span).map(|field_element| { + field_element + .try_to_u32() + .expect("ICE: size should have already been checked by evaluate_to_field_element") + }) } // TODO(https://github.com/noir-lang/noir/issues/6260): remove // the unifies checks once all kinds checks are implemented? - pub(crate) fn evaluate_to_field_element(&self, kind: &Kind) -> Option { + pub(crate) fn evaluate_to_field_element( + &self, + kind: &Kind, + span: Span, + ) -> Result { + let run_simplifications = true; + self.evaluate_to_field_element_helper(kind, span, run_simplifications) + } + + /// evaluate_to_field_element with optional generic arithmetic simplifications + pub(crate) fn evaluate_to_field_element_helper( + &self, + kind: &Kind, + span: Span, + run_simplifications: bool, + ) -> Result { if let Some((binding, binding_kind)) = self.get_inner_type_variable() { if let TypeBinding::Bound(binding) = &*binding.borrow() { if kind.unifies(&binding_kind) { - return binding.evaluate_to_field_element(&binding_kind); + return binding.evaluate_to_field_element_helper( + &binding_kind, + span, + run_simplifications, + ); } } } - match self.canonicalize() { + let could_be_checked_cast = false; + match self.canonicalize_helper(could_be_checked_cast, run_simplifications) { Type::Constant(x, constant_kind) => { if kind.unifies(&constant_kind) { - kind.ensure_value_fits(x) + kind.ensure_value_fits(x, span) } else { - None + Err(TypeCheckError::TypeKindMismatch { + expected_kind: format!("{}", constant_kind), + expr_kind: format!("{}", kind), + expr_span: span, + }) } } Type::InfixExpr(lhs, op, rhs) => { let infix_kind = lhs.infix_kind(&rhs); if kind.unifies(&infix_kind) { - let lhs_value = lhs.evaluate_to_field_element(&infix_kind)?; - let rhs_value = rhs.evaluate_to_field_element(&infix_kind)?; - op.function(lhs_value, rhs_value, &infix_kind) + let lhs_value = lhs.evaluate_to_field_element_helper( + &infix_kind, + span, + run_simplifications, + )?; + let rhs_value = rhs.evaluate_to_field_element_helper( + &infix_kind, + span, + run_simplifications, + )?; + op.function(lhs_value, rhs_value, &infix_kind, span) } else { - None + Err(TypeCheckError::TypeKindMismatch { + expected_kind: format!("{}", kind), + expr_kind: format!("{}", infix_kind), + expr_span: span, + }) } } - _ => None, + Type::CheckedCast { from, to } => { + let to_value = to.evaluate_to_field_element(kind, span)?; + + // if both 'to' and 'from' evaluate to a constant, + // return None unless they match + let skip_simplifications = false; + if let Ok(from_value) = + from.evaluate_to_field_element_helper(kind, span, skip_simplifications) + { + if to_value == from_value { + Ok(to_value) + } else { + let to = *to.clone(); + let from = *from.clone(); + Err(TypeCheckError::TypeCanonicalizationMismatch { + to, + from, + to_value, + from_value, + span, + }) + } + } else { + Ok(to_value) + } + } + other => Err(TypeCheckError::NonConstantEvaluated { typ: other, span }), } } @@ -2182,6 +2299,11 @@ impl Type { let fields = fields.substitute_helper(type_bindings, substitute_bound_typevars); Type::FmtString(Box::new(size), Box::new(fields)) } + Type::CheckedCast { from, to } => { + let from = from.substitute_helper(type_bindings, substitute_bound_typevars); + let to = to.substitute_helper(type_bindings, substitute_bound_typevars); + Type::CheckedCast { from: Box::new(from), to: Box::new(to) } + } Type::NamedGeneric(binding, _) | Type::TypeVariable(binding) => { substitute_binding(binding) } @@ -2271,6 +2393,7 @@ impl Type { || args.named.iter().any(|arg| arg.typ.occurs(target_id)) } Type::Tuple(fields) => fields.iter().any(|field| field.occurs(target_id)), + Type::CheckedCast { from, to } => from.occurs(target_id) || to.occurs(target_id), Type::NamedGeneric(type_var, _) | Type::TypeVariable(type_var) => { match &*type_var.borrow() { TypeBinding::Bound(binding) => { @@ -2329,6 +2452,11 @@ impl Type { def.borrow().get_type(args).follow_bindings() } Tuple(args) => Tuple(vecmap(args, |arg| arg.follow_bindings())), + CheckedCast { from, to } => { + let from = Box::new(from.follow_bindings()); + let to = Box::new(to.follow_bindings()); + CheckedCast { from, to } + } TypeVariable(var) | NamedGeneric(var, _) => { if let TypeBinding::Bound(typ) = &*var.borrow() { return typ.follow_bindings(); @@ -2425,6 +2553,10 @@ impl Type { generic.typ.replace_named_generics_with_type_variables(); } } + Type::CheckedCast { from, to } => { + from.replace_named_generics_with_type_variables(); + to.replace_named_generics_with_type_variables(); + } Type::NamedGeneric(var, _) => { let type_binding = var.borrow(); if let TypeBinding::Bound(binding) = &*type_binding { @@ -2482,6 +2614,7 @@ impl Type { } } Type::Alias(alias, args) => alias.borrow().get_type(args).integral_maximum_size(), + Type::CheckedCast { to, .. } => to.integral_maximum_size(), Type::NamedGeneric(binding, _name) => match &*binding.borrow() { TypeBinding::Bound(typ) => typ.integral_maximum_size(), TypeBinding::Unbound(_, kind) => kind.integral_maximum_size(), @@ -2541,28 +2674,39 @@ fn convert_array_expression_to_slice( impl BinaryTypeOperator { /// Perform the actual rust numeric operation associated with this operator - pub fn function(self, a: FieldElement, b: FieldElement, kind: &Kind) -> Option { + pub fn function( + self, + a: FieldElement, + b: FieldElement, + kind: &Kind, + span: Span, + ) -> Result { match kind.follow_bindings().integral_maximum_size() { None => match self { - BinaryTypeOperator::Addition => Some(a + b), - BinaryTypeOperator::Subtraction => Some(a - b), - BinaryTypeOperator::Multiplication => Some(a * b), - BinaryTypeOperator::Division => (b != FieldElement::zero()).then(|| a / b), - BinaryTypeOperator::Modulo => None, + BinaryTypeOperator::Addition => Ok(a + b), + BinaryTypeOperator::Subtraction => Ok(a - b), + BinaryTypeOperator::Multiplication => Ok(a * b), + BinaryTypeOperator::Division => (b != FieldElement::zero()) + .then(|| a / b) + .ok_or(TypeCheckError::DivisionByZero { lhs: a, rhs: b, span }), + BinaryTypeOperator::Modulo => { + Err(TypeCheckError::ModuloOnFields { lhs: a, rhs: b, span }) + } }, Some(_maximum_size) => { let a = a.to_i128(); let b = b.to_i128(); + let err = TypeCheckError::FailingBinaryOp { op: self, lhs: a, rhs: b, span }; let result = match self { - BinaryTypeOperator::Addition => a.checked_add(b)?, - BinaryTypeOperator::Subtraction => a.checked_sub(b)?, - BinaryTypeOperator::Multiplication => a.checked_mul(b)?, - BinaryTypeOperator::Division => a.checked_div(b)?, - BinaryTypeOperator::Modulo => a.checked_rem(b)?, + BinaryTypeOperator::Addition => a.checked_add(b).ok_or(err)?, + BinaryTypeOperator::Subtraction => a.checked_sub(b).ok_or(err)?, + BinaryTypeOperator::Multiplication => a.checked_mul(b).ok_or(err)?, + BinaryTypeOperator::Division => a.checked_div(b).ok_or(err)?, + BinaryTypeOperator::Modulo => a.checked_rem(b).ok_or(err)?, }; - Some(result.into()) + Ok(result.into()) } } } @@ -2607,7 +2751,9 @@ impl From<&Type> for PrintableType { match value { Type::FieldElement => PrintableType::Field, Type::Array(size, typ) => { - let length = size.evaluate_to_u32().expect("Cannot print variable sized arrays"); + let dummy_span = Span::default(); + let length = + size.evaluate_to_u32(dummy_span).expect("Cannot print variable sized arrays"); let typ = typ.as_ref(); PrintableType::Array { length, typ: Box::new(typ.into()) } } @@ -2632,7 +2778,9 @@ impl From<&Type> for PrintableType { }, Type::Bool => PrintableType::Boolean, Type::String(size) => { - let size = size.evaluate_to_u32().expect("Cannot print variable sized strings"); + let dummy_span = Span::default(); + let size = + size.evaluate_to_u32(dummy_span).expect("Cannot print variable sized strings"); PrintableType::String { length: size } } Type::FmtString(_, _) => unreachable!("format strings cannot be printed"), @@ -2648,6 +2796,7 @@ impl From<&Type> for PrintableType { Type::Alias(alias, args) => alias.borrow().get_type(args).into(), Type::TraitAsType(..) => unreachable!(), Type::Tuple(types) => PrintableType::Tuple { types: vecmap(types, |typ| typ.into()) }, + Type::CheckedCast { to, .. } => to.as_ref().into(), Type::NamedGeneric(..) => unreachable!(), Type::Forall(..) => unreachable!(), Type::Function(arguments, return_type, env, unconstrained) => PrintableType::Function { @@ -2722,6 +2871,7 @@ impl std::fmt::Debug for Type { } Type::Unit => write!(f, "()"), Type::Error => write!(f, "error"), + Type::CheckedCast { to, .. } => write!(f, "{:?}", to), Type::NamedGeneric(binding, name) => match binding.kind() { Kind::Any | Kind::Normal | Kind::Integer | Kind::IntegerOrField => { write!(f, "{}{:?}", name, binding) @@ -2836,6 +2986,7 @@ impl std::hash::Hash for Type { vars.hash(state); typ.hash(state); } + Type::CheckedCast { to, .. } => to.hash(state), Type::Constant(value, _) => value.hash(state), Type::Quoted(typ) => typ.hash(state), Type::InfixExpr(lhs, op, rhs) => { @@ -2902,6 +3053,7 @@ impl PartialEq for Type { (Forall(lhs_vars, lhs_type), Forall(rhs_vars, rhs_type)) => { lhs_vars == rhs_vars && lhs_type == rhs_type } + (CheckedCast { to, .. }, other) | (other, CheckedCast { to, .. }) => **to == *other, (Constant(lhs, lhs_kind), Constant(rhs, rhs_kind)) => { lhs == rhs && lhs_kind == rhs_kind } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/types/arithmetic.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/types/arithmetic.rs index 81f638ebca4..8cdf6f5502c 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/types/arithmetic.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/types/arithmetic.rs @@ -1,6 +1,7 @@ use std::collections::BTreeMap; use acvm::{AcirField, FieldElement}; +use noirc_errors::Span; use crate::{BinaryTypeOperator, Type, TypeBindings, UnificationError}; @@ -15,34 +16,81 @@ impl Type { /// - `canonicalize[((1 + N) + M) + 2] = (M + N) + 3` /// - `canonicalize[A + 2 * B + 3 - 2] = A + (B * 2) + 3 - 2` pub fn canonicalize(&self) -> Type { + match self.follow_bindings() { + Type::CheckedCast { from, to } => Type::CheckedCast { + from: Box::new(from.canonicalize_checked()), + to: Box::new(to.canonicalize_unchecked()), + }, + + other => { + let non_checked_cast = false; + let run_simplifications = true; + other.canonicalize_helper(non_checked_cast, run_simplifications) + } + } + } + + /// Only simplify constants and drop/skip any CheckedCast's + pub(crate) fn canonicalize_checked(&self) -> Type { + let found_checked_cast = true; + let skip_simplifications = false; + self.canonicalize_helper(found_checked_cast, skip_simplifications) + } + + /// Run all simplifications and drop/skip any CheckedCast's + fn canonicalize_unchecked(&self) -> Type { + let found_checked_cast = true; + let run_simplifications = true; + self.canonicalize_helper(found_checked_cast, run_simplifications) + } + + /// If found_checked_cast, then drop additional CheckedCast's + /// + /// If run_simplifications is false, then only: + /// - Attempt to evaluate each sub-expression to a constant + /// - Drop nested CheckedCast's + /// + /// Otherwise also attempt try_simplify_partial_constants, sort_commutative, + /// and other simplifications + pub(crate) fn canonicalize_helper( + &self, + found_checked_cast: bool, + run_simplifications: bool, + ) -> Type { match self.follow_bindings() { Type::InfixExpr(lhs, op, rhs) => { let kind = lhs.infix_kind(&rhs); + let dummy_span = Span::default(); // evaluate_to_field_element also calls canonicalize so if we just called // `self.evaluate_to_field_element(..)` we'd get infinite recursion. - if let (Some(lhs_u32), Some(rhs_u32)) = - (lhs.evaluate_to_field_element(&kind), rhs.evaluate_to_field_element(&kind)) - { - let kind = lhs.infix_kind(&rhs); - if let Some(result) = op.function(lhs_u32, rhs_u32, &kind) { + if let (Ok(lhs_value), Ok(rhs_value)) = ( + lhs.evaluate_to_field_element_helper(&kind, dummy_span, run_simplifications), + rhs.evaluate_to_field_element_helper(&kind, dummy_span, run_simplifications), + ) { + if let Ok(result) = op.function(lhs_value, rhs_value, &kind, dummy_span) { return Type::Constant(result, kind); } } - let lhs = lhs.canonicalize(); - let rhs = rhs.canonicalize(); + let lhs = lhs.canonicalize_helper(found_checked_cast, run_simplifications); + let rhs = rhs.canonicalize_helper(found_checked_cast, run_simplifications); + + if !run_simplifications { + return Type::InfixExpr(Box::new(lhs), op, Box::new(rhs)); + } + if let Some(result) = Self::try_simplify_non_constants_in_lhs(&lhs, op, &rhs) { - return result.canonicalize(); + return result.canonicalize_unchecked(); } if let Some(result) = Self::try_simplify_non_constants_in_rhs(&lhs, op, &rhs) { - return result.canonicalize(); + return result.canonicalize_unchecked(); } // Try to simplify partially constant expressions in the form `(N op1 C1) op2 C2` // where C1 and C2 are constants that can be combined (e.g. N + 5 - 3 = N + 2) if let Some(result) = Self::try_simplify_partial_constants(&lhs, op, &rhs) { - return result.canonicalize(); + return result.canonicalize_unchecked(); } if op.is_commutative() { @@ -51,6 +99,18 @@ impl Type { Type::InfixExpr(Box::new(lhs), op, Box::new(rhs)) } + Type::CheckedCast { from, to } => { + let inner_found_checked_cast = true; + let to = to.canonicalize_helper(inner_found_checked_cast, run_simplifications); + + if found_checked_cast { + return to; + } + + let from = from.canonicalize_checked(); + + Type::CheckedCast { from: Box::new(from), to: Box::new(to) } + } other => other, } } @@ -70,13 +130,16 @@ impl Type { // Push each non-constant term to `sorted` to sort them. Recur on InfixExprs with the same operator. while let Some(item) = queue.pop() { - match item.canonicalize() { - Type::InfixExpr(lhs, new_op, rhs) if new_op == op => { - queue.push(*lhs); - queue.push(*rhs); + match item.canonicalize_unchecked() { + Type::InfixExpr(lhs_inner, new_op, rhs_inner) if new_op == op => { + queue.push(*lhs_inner); + queue.push(*rhs_inner); } Type::Constant(new_constant, new_constant_kind) => { - if let Some(result) = op.function(constant, new_constant, &new_constant_kind) { + let dummy_span = Span::default(); + if let Ok(result) = + op.function(constant, new_constant, &new_constant_kind, dummy_span) + { constant = result; } else { let constant = Type::Constant(new_constant, new_constant_kind); @@ -126,20 +189,27 @@ impl Type { op: BinaryTypeOperator, rhs: &Type, ) -> Option { - let Type::InfixExpr(l_lhs, l_op, l_rhs) = lhs.follow_bindings() else { - return None; - }; + match lhs.follow_bindings() { + Type::CheckedCast { from, to } => { + // Apply operation directly to `from` while attempting simplification to `to`. + let from = Type::InfixExpr(from, op, Box::new(rhs.clone())); + let to = Self::try_simplify_non_constants_in_lhs(&to, op, rhs)?; + Some(Type::CheckedCast { from: Box::new(from), to: Box::new(to) }) + } + Type::InfixExpr(l_lhs, l_op, l_rhs) => { + // Note that this is exact, syntactic equality, not unification. + // `rhs` is expected to already be in canonical form. + if l_op.approx_inverse() != Some(op) + || l_op == BinaryTypeOperator::Division + || l_rhs.canonicalize_unchecked() != *rhs + { + return None; + } - // Note that this is exact, syntactic equality, not unification. - // `rhs` is expected to already be in canonical form. - if l_op.approx_inverse() != Some(op) - || l_op == BinaryTypeOperator::Division - || l_rhs.canonicalize() != *rhs - { - return None; + Some(*l_lhs) + } + _ => None, } - - Some(*l_lhs) } /// Try to simplify non-constant expressions in the form `N op1 (M op1 N)` @@ -156,23 +226,31 @@ impl Type { op: BinaryTypeOperator, rhs: &Type, ) -> Option { - let Type::InfixExpr(r_lhs, r_op, r_rhs) = rhs.follow_bindings() else { - return None; - }; + match rhs.follow_bindings() { + Type::CheckedCast { from, to } => { + // Apply operation directly to `from` while attempting simplification to `to`. + let from = Type::InfixExpr(Box::new(lhs.clone()), op, from); + let to = Self::try_simplify_non_constants_in_rhs(lhs, op, &to)?; + Some(Type::CheckedCast { from: Box::new(from), to: Box::new(to) }) + } + Type::InfixExpr(r_lhs, r_op, r_rhs) => { + // `N / (M * N)` should be simplified to `1 / M`, but we only handle + // simplifying to `M` in this function. + if op == BinaryTypeOperator::Division && r_op == BinaryTypeOperator::Multiplication + { + return None; + } - // `N / (M * N)` should be simplified to `1 / M`, but we only handle - // simplifying to `M` in this function. - if op == BinaryTypeOperator::Division && r_op == BinaryTypeOperator::Multiplication { - return None; - } + // Note that this is exact, syntactic equality, not unification. + // `lhs` is expected to already be in canonical form. + if r_op.inverse() != Some(op) || *lhs != r_rhs.canonicalize_unchecked() { + return None; + } - // Note that this is exact, syntactic equality, not unification. - // `lhs` is expected to already be in canonical form. - if r_op.inverse() != Some(op) || *lhs != r_rhs.canonicalize() { - return None; + Some(*r_lhs) + } + _ => None, } - - Some(*r_lhs) } /// Given: @@ -187,13 +265,15 @@ impl Type { rhs: &Type, ) -> Option<(Box, BinaryTypeOperator, FieldElement, FieldElement)> { let kind = lhs.infix_kind(rhs); - let rhs = rhs.evaluate_to_field_element(&kind)?; + let dummy_span = Span::default(); + let rhs = rhs.evaluate_to_field_element(&kind, dummy_span).ok()?; let Type::InfixExpr(l_type, l_op, l_rhs) = lhs.follow_bindings() else { return None; }; - let l_rhs = l_rhs.evaluate_to_field_element(&kind)?; + let dummy_span = Span::default(); + let l_rhs = l_rhs.evaluate_to_field_element(&kind, dummy_span).ok()?; Some((l_type, l_op, l_rhs, rhs)) } @@ -218,7 +298,9 @@ impl Type { if l_op == Subtraction { op = op.inverse()?; } - let result = op.function(l_const, r_const, &lhs.infix_kind(rhs))?; + let dummy_span = Span::default(); + let result = + op.function(l_const, r_const, &lhs.infix_kind(rhs), dummy_span).ok()?; let constant = Type::Constant(result, lhs.infix_kind(rhs)); Some(Type::InfixExpr(l_type, l_op, Box::new(constant))) } @@ -231,7 +313,9 @@ impl Type { if op == Division && (r_const == FieldElement::zero() || !divides_evenly) { None } else { - let result = op.function(l_const, r_const, &lhs.infix_kind(rhs))?; + let dummy_span = Span::default(); + let result = + op.function(l_const, r_const, &lhs.infix_kind(rhs), dummy_span).ok()?; let constant = Box::new(Type::Constant(result, lhs.infix_kind(rhs))); Some(Type::InfixExpr(l_type, l_op, constant)) } @@ -250,7 +334,8 @@ impl Type { if let Type::InfixExpr(lhs_a, op_a, rhs_a) = self { if let Some(inverse) = op_a.approx_inverse() { let kind = lhs_a.infix_kind(rhs_a); - if let Some(rhs_a_value) = rhs_a.evaluate_to_field_element(&kind) { + let dummy_span = Span::default(); + if let Ok(rhs_a_value) = rhs_a.evaluate_to_field_element(&kind, dummy_span) { let rhs_a = Box::new(Type::Constant(rhs_a_value, kind)); let new_other = Type::InfixExpr(Box::new(other.clone()), inverse, rhs_a); @@ -266,7 +351,8 @@ impl Type { if let Type::InfixExpr(lhs_b, op_b, rhs_b) = other { if let Some(inverse) = op_b.approx_inverse() { let kind = lhs_b.infix_kind(rhs_b); - if let Some(rhs_b_value) = rhs_b.evaluate_to_field_element(&kind) { + let dummy_span = Span::default(); + if let Ok(rhs_b_value) = rhs_b.evaluate_to_field_element(&kind, dummy_span) { let rhs_b = Box::new(Type::Constant(rhs_b_value, kind)); let new_self = Type::InfixExpr(Box::new(self.clone()), inverse, rhs_b); @@ -282,3 +368,301 @@ impl Type { Err(UnificationError) } } + +#[cfg(test)] +mod tests { + use acvm::{AcirField, FieldElement}; + + use crate::hir_def::types::{BinaryTypeOperator, Kind, Type, TypeVariable, TypeVariableId}; + + #[test] + fn solves_n_minus_one_plus_one_through_checked_casts() { + // We want to test that the inclusion of a `CheckedCast` won't prevent us from canonicalizing + // the expression `(N - 1) + 1` to `N` if there exists a `CheckedCast` on the `N - 1` term. + + let n = Type::NamedGeneric( + TypeVariable::unbound(TypeVariableId(0), Kind::u32()), + std::rc::Rc::new("N".to_owned()), + ); + let n_minus_one = Type::InfixExpr( + Box::new(n.clone()), + BinaryTypeOperator::Subtraction, + Box::new(Type::Constant(FieldElement::one(), Kind::u32())), + ); + let checked_cast_n_minus_one = + Type::CheckedCast { from: Box::new(n_minus_one.clone()), to: Box::new(n_minus_one) }; + + let n_minus_one_plus_one = Type::InfixExpr( + Box::new(checked_cast_n_minus_one.clone()), + BinaryTypeOperator::Addition, + Box::new(Type::Constant(FieldElement::one(), Kind::u32())), + ); + + let canonicalized_typ = n_minus_one_plus_one.canonicalize(); + + assert_eq!(n, canonicalized_typ); + + // We also want to check that if the `CheckedCast` is on the RHS then we'll still be able to canonicalize + // the expression `1 + (N - 1)` to `N`. + + let one_plus_n_minus_one = Type::InfixExpr( + Box::new(Type::Constant(FieldElement::one(), Kind::u32())), + BinaryTypeOperator::Addition, + Box::new(checked_cast_n_minus_one), + ); + + let canonicalized_typ = one_plus_n_minus_one.canonicalize(); + + assert_eq!(n, canonicalized_typ); + } + + #[test] + fn instantiate_after_canonicalize_smoke_test() { + let field_element_kind = Kind::numeric(Type::FieldElement); + let x_var = TypeVariable::unbound(TypeVariableId(0), field_element_kind.clone()); + let x_type = Type::TypeVariable(x_var.clone()); + let one = Type::Constant(FieldElement::one(), field_element_kind.clone()); + + let lhs = Type::InfixExpr( + Box::new(x_type.clone()), + BinaryTypeOperator::Addition, + Box::new(one.clone()), + ); + let rhs = + Type::InfixExpr(Box::new(one), BinaryTypeOperator::Addition, Box::new(x_type.clone())); + + // canonicalize + let lhs = lhs.canonicalize(); + let rhs = rhs.canonicalize(); + + // bind vars + let two = Type::Constant(FieldElement::from(2u128), field_element_kind.clone()); + x_var.bind(two); + + // canonicalize (expect constant) + let lhs = lhs.canonicalize(); + let rhs = rhs.canonicalize(); + + // ensure we've canonicalized to constants + assert!(matches!(lhs, Type::Constant(..))); + assert!(matches!(rhs, Type::Constant(..))); + + // ensure result kinds are the same as the original kind + assert_eq!(lhs.kind(), field_element_kind); + assert_eq!(rhs.kind(), field_element_kind); + + // ensure results are the same + assert_eq!(lhs, rhs); + } +} + +#[cfg(test)] +mod proptests { + + use acvm::{AcirField, FieldElement}; + use proptest::arbitrary::any; + use proptest::collection; + use proptest::prelude::*; + use proptest::result::maybe_ok; + use proptest::strategy; + + use crate::ast::{IntegerBitSize, Signedness}; + use crate::hir_def::types::{BinaryTypeOperator, Kind, Type, TypeVariable, TypeVariableId}; + + prop_compose! { + // maximum_size must be non-zero + fn arbitrary_u128_field_element(maximum_size: u128) + (u128_value in any::()) + -> FieldElement + { + assert!(maximum_size != 0); + FieldElement::from(u128_value % maximum_size) + } + } + + // NOTE: this is roughly the same method from acvm/tests/solver + prop_compose! { + // Use both `u128` and hex proptest strategies + fn arbitrary_field_element() + (u128_or_hex in maybe_ok(any::(), "[0-9a-f]{64}")) + -> FieldElement + { + match u128_or_hex { + Ok(number) => FieldElement::from(number), + Err(hex) => FieldElement::from_hex(&hex).expect("should accept any 32 byte hex string"), + } + } + } + + // Generate (arbitrary_unsigned_type, generator for that type) + fn arbitrary_unsigned_type_with_generator() -> BoxedStrategy<(Type, BoxedStrategy)> + { + prop_oneof![ + strategy::Just((Type::FieldElement, arbitrary_field_element().boxed())), + any::().prop_map(|bit_size| { + let typ = Type::Integer(Signedness::Unsigned, bit_size); + let maximum_size = typ.integral_maximum_size().unwrap().to_u128(); + (typ, arbitrary_u128_field_element(maximum_size).boxed()) + }), + strategy::Just((Type::Bool, arbitrary_u128_field_element(1).boxed())), + ] + .boxed() + } + + prop_compose! { + fn arbitrary_variable(typ: Type, num_variables: usize) + (variable_index in any::()) + -> Type { + assert!(num_variables != 0); + let id = TypeVariableId(variable_index % num_variables); + let kind = Kind::numeric(typ.clone()); + let var = TypeVariable::unbound(id, kind); + Type::TypeVariable(var) + } + } + + fn first_n_variables(typ: Type, num_variables: usize) -> impl Iterator { + (0..num_variables).map(move |id| { + let id = TypeVariableId(id); + let kind = Kind::numeric(typ.clone()); + TypeVariable::unbound(id, kind) + }) + } + + fn arbitrary_infix_expr( + typ: Type, + arbitrary_value: BoxedStrategy, + num_variables: usize, + ) -> impl Strategy { + let leaf = prop_oneof![ + arbitrary_variable(typ.clone(), num_variables), + arbitrary_value + .prop_map(move |value| Type::Constant(value, Kind::numeric(typ.clone()))), + ]; + + leaf.prop_recursive( + 8, // 8 levels deep maximum + 256, // Shoot for maximum size of 256 nodes + 10, // We put up to 10 items per collection + |inner| { + (inner.clone(), any::(), inner) + .prop_map(|(lhs, op, rhs)| Type::InfixExpr(Box::new(lhs), op, Box::new(rhs))) + }, + ) + } + + prop_compose! { + // (infix_expr, type, generator) + fn arbitrary_infix_expr_type_gen(num_variables: usize) + (type_and_gen in arbitrary_unsigned_type_with_generator()) + (infix_expr in arbitrary_infix_expr(type_and_gen.clone().0, type_and_gen.clone().1, num_variables), type_and_gen in Just(type_and_gen)) + -> (Type, Type, BoxedStrategy) { + let (typ, value_generator) = type_and_gen; + (infix_expr, typ, value_generator) + } + } + + prop_compose! { + // (Type::InfixExpr, numeric kind, bindings) + fn arbitrary_infix_expr_with_bindings_sized(num_variables: usize) + (infix_type_gen in arbitrary_infix_expr_type_gen(num_variables)) + (values in collection::vec(infix_type_gen.clone().2, num_variables), infix_type_gen in Just(infix_type_gen)) + -> (Type, Type, Vec<(TypeVariable, Type)>) { + let (infix_expr, typ, _value_generator) = infix_type_gen; + let bindings: Vec<_> = first_n_variables(typ.clone(), num_variables) + .zip(values.iter().map(|value| { + Type::Constant(*value, Kind::numeric(typ.clone())) + })) + .collect(); + (infix_expr, typ, bindings) + } + } + + prop_compose! { + // the lint misfires on 'num_variables' + #[allow(unused_variables)] + fn arbitrary_infix_expr_with_bindings(max_num_variables: usize) + (num_variables in any::().prop_map(move |num_variables| (num_variables % max_num_variables).clamp(1, max_num_variables))) + (infix_type_bindings in arbitrary_infix_expr_with_bindings_sized(num_variables), num_variables in Just(num_variables)) + -> (Type, Type, Vec<(TypeVariable, Type)>) { + infix_type_bindings + } + } + + proptest! { + #[test] + // Expect cases that don't resolve to constants, e.g. see + // `arithmetic_generics_checked_cast_indirect_zeros` + #[should_panic(expected = "matches!(infix, Type :: Constant(..))")] + fn instantiate_before_or_after_canonicalize(infix_type_bindings in arbitrary_infix_expr_with_bindings(10)) { + let (infix, typ, bindings) = infix_type_bindings; + + // canonicalize + let infix_canonicalized = infix.canonicalize(); + + // bind vars + for (var, binding) in bindings { + var.bind(binding); + } + + // attempt to canonicalize to a constant + let infix = infix.canonicalize(); + let infix_canonicalized = infix_canonicalized.canonicalize(); + + // ensure we've canonicalized to constants + prop_assert!(matches!(infix, Type::Constant(..))); + prop_assert!(matches!(infix_canonicalized, Type::Constant(..))); + + // ensure result kinds are the same as the original kind + let kind = Kind::numeric(typ); + prop_assert_eq!(infix.kind(), kind.clone()); + prop_assert_eq!(infix_canonicalized.kind(), kind); + + // ensure results are the same + prop_assert_eq!(infix, infix_canonicalized); + } + + #[test] + fn instantiate_before_or_after_canonicalize_checked_cast(infix_type_bindings in arbitrary_infix_expr_with_bindings(10)) { + let (infix, typ, bindings) = infix_type_bindings; + + // wrap in CheckedCast + let infix = Type::CheckedCast { + from: Box::new(infix.clone()), + to: Box::new(infix) + }; + + // canonicalize + let infix_canonicalized = infix.canonicalize(); + + // bind vars + for (var, binding) in bindings { + var.bind(binding); + } + + // attempt to canonicalize to a constant + let infix = infix.canonicalize(); + let infix_canonicalized = infix_canonicalized.canonicalize(); + + // ensure result kinds are the same as the original kind + let kind = Kind::numeric(typ); + prop_assert_eq!(infix.kind(), kind.clone()); + prop_assert_eq!(infix_canonicalized.kind(), kind.clone()); + + // ensure the results are still wrapped in CheckedCast's + match (&infix, &infix_canonicalized) { + (Type::CheckedCast { from, to }, Type::CheckedCast { from: from_canonicalized, to: to_canonicalized }) => { + // ensure from's are the same + prop_assert_eq!(from, from_canonicalized); + + // ensure to's have the same kinds + prop_assert_eq!(to.kind(), kind.clone()); + prop_assert_eq!(to_canonicalized.kind(), kind); + } + _ => { + prop_assert!(false, "expected CheckedCast"); + } + } + } + } +} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/lexer/lexer.rs b/noir/noir-repo/compiler/noirc_frontend/src/lexer/lexer.rs index 5c592c84022..68dc142ff10 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/lexer/lexer.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/lexer/lexer.rs @@ -1,4 +1,4 @@ -use crate::token::{Attribute, DocStyle}; +use crate::token::DocStyle; use super::{ errors::LexerErrorKind, @@ -139,7 +139,7 @@ impl<'a> Lexer<'a> { Some('f') => self.eat_format_string_or_alpha_numeric(), Some('r') => self.eat_raw_string_or_alpha_numeric(), Some('q') => self.eat_quote_or_alpha_numeric(), - Some('#') => self.eat_attribute(), + Some('#') => self.eat_attribute_start(), Some(ch) if ch.is_ascii_alphanumeric() || ch == '_' => self.eat_alpha_numeric(ch), Some(ch) => { // We don't report invalid tokens in the source as errors until parsing to @@ -282,7 +282,7 @@ impl<'a> Lexer<'a> { } } - fn eat_attribute(&mut self) -> SpannedTokenResult { + fn eat_attribute_start(&mut self) -> SpannedTokenResult { let start = self.position; let is_inner = if self.peek_char_is('!') { @@ -306,40 +306,9 @@ impl<'a> Lexer<'a> { self.next_char(); } - let contents_start = self.position + 1; - - let word = self.eat_while(None, |ch| ch != ']'); - - let contents_end = self.position; - - if !self.peek_char_is(']') { - return Err(LexerErrorKind::UnexpectedCharacter { - span: Span::single_char(self.position), - expected: "]".to_owned(), - found: self.next_char(), - }); - } - self.next_char(); - let end = self.position; - let span = Span::inclusive(start, end); - let contents_span = Span::inclusive(contents_start, contents_end); - - let attribute = Attribute::lookup_attribute(&word, span, contents_span, is_tag)?; - if is_inner { - match attribute { - Attribute::Function(attribute) => Err(LexerErrorKind::InvalidInnerAttribute { - span: Span::from(start..end), - found: attribute.to_string(), - }), - Attribute::Secondary(attribute) => { - Ok(Token::InnerAttribute(attribute).into_span(start, end)) - } - } - } else { - Ok(Token::Attribute(attribute).into_span(start, end)) - } + Ok(Token::AttributeStart { is_inner, is_tag }.into_span(start, end)) } //XXX(low): Can increase performance if we use iterator semantic and utilize some of the methods on String. See below @@ -725,7 +694,6 @@ mod tests { use iter_extended::vecmap; use super::*; - use crate::token::{CustomAttribute, FunctionAttribute, SecondaryAttribute, TestScope}; #[test] fn test_single_multi_char() { @@ -785,173 +753,39 @@ mod tests { } #[test] - fn deprecated_attribute() { - let input = r#"#[deprecated]"#; - let mut lexer = Lexer::new(input); - - let token = lexer.next_token().unwrap(); - assert_eq!( - token.token(), - &Token::Attribute(Attribute::Secondary(SecondaryAttribute::Deprecated(None))) - ); - } - - #[test] - fn test_attribute_with_common_punctuation() { - let input = - r#"#[test(should_fail_with = "stmt. q? exclaim! & symbols, 1% shouldn't fail")]"#; - let mut lexer = Lexer::new(input); - - let token = lexer.next_token().unwrap().token().clone(); - assert_eq!( - token, - Token::Attribute(Attribute::Function(FunctionAttribute::Test( - TestScope::ShouldFailWith { - reason: "stmt. q? exclaim! & symbols, 1% shouldn't fail".to_owned().into() - } - ))) - ); - } - - #[test] - fn deprecated_attribute_with_note() { - let input = r#"#[deprecated("hello")]"#; - let mut lexer = Lexer::new(input); - - let token = lexer.next_token().unwrap(); - assert_eq!( - token.token(), - &Token::Attribute(Attribute::Secondary(crate::token::SecondaryAttribute::Deprecated( - "hello".to_string().into() - ))) - ); - } - - #[test] - fn test_custom_gate_syntax() { - let input = "#[foreign(sha256)]#[foreign(blake2s)]#[builtin(sum)]"; - - let expected = vec![ - Token::Attribute(Attribute::Function(FunctionAttribute::Foreign("sha256".to_string()))), - Token::Attribute(Attribute::Function(FunctionAttribute::Foreign( - "blake2s".to_string(), - ))), - Token::Attribute(Attribute::Function(FunctionAttribute::Builtin("sum".to_string()))), - ]; - - let mut lexer = Lexer::new(input); - for token in expected.into_iter() { - let got = lexer.next_token().unwrap(); - assert_eq!(got, token); - } - } - - #[test] - fn tag_attribute() { - let input = r#"#['custom(hello)]"#; + fn test_attribute_start() { + let input = r#"#[something]"#; let mut lexer = Lexer::new(input); let token = lexer.next_token().unwrap(); - assert_eq!( - token.token(), - &Token::Attribute(Attribute::Secondary(SecondaryAttribute::Tag(CustomAttribute { - contents: "custom(hello)".to_string(), - span: Span::from(0..17), - contents_span: Span::from(3..16) - }))) - ); + assert_eq!(token.token(), &Token::AttributeStart { is_inner: false, is_tag: false }); } #[test] - fn test_attribute() { - let input = r#"#[test]"#; + fn test_attribute_start_with_tag() { + let input = r#"#['something]"#; let mut lexer = Lexer::new(input); let token = lexer.next_token().unwrap(); - assert_eq!( - token.token(), - &Token::Attribute(Attribute::Function(FunctionAttribute::Test(TestScope::None))) - ); + assert_eq!(token.token(), &Token::AttributeStart { is_inner: false, is_tag: true }); } #[test] - fn fold_attribute() { - let input = r#"#[fold]"#; - - let mut lexer = Lexer::new(input); - let token = lexer.next_token().unwrap(); - - assert_eq!(token.token(), &Token::Attribute(Attribute::Function(FunctionAttribute::Fold))); - } - - #[test] - fn contract_library_method_attribute() { - let input = r#"#[contract_library_method]"#; - let mut lexer = Lexer::new(input); - - let token = lexer.next_token().unwrap(); - assert_eq!( - token.token(), - &Token::Attribute(Attribute::Secondary(SecondaryAttribute::ContractLibraryMethod)) - ); - } - - #[test] - fn test_attribute_with_valid_scope() { - let input = r#"#[test(should_fail)]"#; - let mut lexer = Lexer::new(input); - - let token = lexer.next_token().unwrap(); - assert_eq!( - token.token(), - &Token::Attribute(Attribute::Function(FunctionAttribute::Test( - TestScope::ShouldFailWith { reason: None } - ))) - ); - } - - #[test] - fn test_attribute_with_valid_scope_should_fail_with() { - let input = r#"#[test(should_fail_with = "hello")]"#; + fn test_inner_attribute_start() { + let input = r#"#![something]"#; let mut lexer = Lexer::new(input); let token = lexer.next_token().unwrap(); - assert_eq!( - token.token(), - &Token::Attribute(Attribute::Function(FunctionAttribute::Test( - TestScope::ShouldFailWith { reason: Some("hello".to_owned()) } - ))) - ); + assert_eq!(token.token(), &Token::AttributeStart { is_inner: true, is_tag: false }); } #[test] - fn test_attribute_with_invalid_scope() { - let input = r#"#[test(invalid_scope)]"#; - let mut lexer = Lexer::new(input); - - let token = lexer.next().unwrap(); - let err = match token { - Ok(_) => panic!("test has an invalid scope, so expected an error"), - Err(err) => err, - }; - - assert!(matches!(err, LexerErrorKind::MalformedTestAttribute { .. })); - } - - #[test] - fn test_inner_attribute() { - let input = r#"#![something]"#; + fn test_inner_attribute_start_with_tag() { + let input = r#"#!['something]"#; let mut lexer = Lexer::new(input); let token = lexer.next_token().unwrap(); - assert_eq!( - token.token(), - &Token::InnerAttribute(SecondaryAttribute::Meta(CustomAttribute { - contents: "something".to_string(), - span: Span::from(0..13), - contents_span: Span::from(3..12), - })) - ); + assert_eq!(token.token(), &Token::AttributeStart { is_inner: true, is_tag: true }); } #[test] diff --git a/noir/noir-repo/compiler/noirc_frontend/src/lexer/token.rs b/noir/noir-repo/compiler/noirc_frontend/src/lexer/token.rs index 1f34af8313d..dbb28cf78c0 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/lexer/token.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/lexer/token.rs @@ -1,9 +1,9 @@ use acvm::FieldElement; use noirc_errors::{Position, Span, Spanned}; -use std::fmt; +use std::fmt::{self, Display}; use crate::{ - lexer::errors::LexerErrorKind, + ast::{Expression, Path}, node_interner::{ ExprId, InternedExpressionKind, InternedPattern, InternedStatementKind, InternedUnresolvedTypeData, QuotedTypeId, @@ -28,8 +28,10 @@ pub enum BorrowedToken<'input> { FmtStr(&'input str), Keyword(Keyword), IntType(IntType), - Attribute(Attribute), - InnerAttribute(SecondaryAttribute), + AttributeStart { + is_inner: bool, + is_tag: bool, + }, LineComment(&'input str, Option), BlockComment(&'input str, Option), Quote(&'input Tokens), @@ -137,8 +139,10 @@ pub enum Token { FmtStr(String), Keyword(Keyword), IntType(IntType), - Attribute(Attribute), - InnerAttribute(SecondaryAttribute), + AttributeStart { + is_inner: bool, + is_tag: bool, + }, LineComment(String, Option), BlockComment(String, Option), // A `quote { ... }` along with the tokens in its token stream. @@ -254,8 +258,9 @@ pub fn token_to_borrowed_token(token: &Token) -> BorrowedToken<'_> { Token::FmtStr(ref b) => BorrowedToken::FmtStr(b), Token::RawStr(ref b, hashes) => BorrowedToken::RawStr(b, *hashes), Token::Keyword(k) => BorrowedToken::Keyword(*k), - Token::Attribute(ref a) => BorrowedToken::Attribute(a.clone()), - Token::InnerAttribute(ref a) => BorrowedToken::InnerAttribute(a.clone()), + Token::AttributeStart { is_inner, is_tag } => { + BorrowedToken::AttributeStart { is_inner: *is_inner, is_tag: *is_tag } + } Token::LineComment(ref s, _style) => BorrowedToken::LineComment(s, *_style), Token::BlockComment(ref s, _style) => BorrowedToken::BlockComment(s, *_style), Token::Quote(stream) => BorrowedToken::Quote(stream), @@ -376,8 +381,17 @@ impl fmt::Display for Token { write!(f, "r{h}{b:?}{h}") } Token::Keyword(k) => write!(f, "{k}"), - Token::Attribute(ref a) => write!(f, "{a}"), - Token::InnerAttribute(ref a) => write!(f, "#![{}]", a.contents()), + Token::AttributeStart { is_inner, is_tag } => { + write!(f, "#")?; + if is_inner { + write!(f, "!")?; + } + write!(f, "[")?; + if is_tag { + write!(f, "'")?; + } + Ok(()) + } Token::LineComment(ref s, style) => match style { Some(DocStyle::Inner) => write!(f, "//!{s}"), Some(DocStyle::Outer) => write!(f, "///{s}"), @@ -503,8 +517,6 @@ impl Token { | Token::RawStr(..) | Token::FmtStr(_) => TokenKind::Literal, Token::Keyword(_) => TokenKind::Keyword, - Token::Attribute(_) => TokenKind::Attribute, - Token::InnerAttribute(_) => TokenKind::InnerAttribute, Token::UnquoteMarker(_) => TokenKind::UnquoteMarker, Token::Quote(_) => TokenKind::Quote, Token::QuotedType(_) => TokenKind::QuotedType, @@ -622,25 +634,6 @@ pub enum TestScope { None, } -impl TestScope { - fn lookup_str(string: &str) -> Option { - match string.trim() { - "should_fail" => Some(TestScope::ShouldFailWith { reason: None }), - s if s.starts_with("should_fail_with") => { - let parts: Vec<&str> = s.splitn(2, '=').collect(); - if parts.len() == 2 { - let reason = parts[1].trim(); - let reason = reason.trim_matches('"'); - Some(TestScope::ShouldFailWith { reason: Some(reason.to_string()) }) - } else { - None - } - } - _ => None, - } - } -} - impl fmt::Display for TestScope { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { @@ -653,13 +646,13 @@ impl fmt::Display for TestScope { } } -#[derive(PartialEq, Eq, Hash, Debug, Clone, PartialOrd, Ord)] +#[derive(PartialEq, Eq, Debug, Clone)] // Attributes are special language markers in the target language // An example of one is `#[SHA256]` . Currently only Foreign attributes are supported // Calls to functions which have the foreign attribute are executed in the host language pub struct Attributes { // Each function can have a single Primary Attribute - pub function: Option, + pub function: Option<(FunctionAttribute, usize /* index in list */)>, // Each function can have many Secondary Attributes pub secondary: Vec, } @@ -669,6 +662,15 @@ impl Attributes { Self { function: None, secondary: Vec::new() } } + pub fn function(&self) -> Option<&FunctionAttribute> { + self.function.as_ref().map(|(attr, _)| attr) + } + + pub fn set_function(&mut self, function: FunctionAttribute) { + // Assume the index in the list doesn't matter anymore at this point + self.function = Some((function, 0)); + } + /// Returns true if one of the secondary attributes is `contract_library_method` /// /// This is useful for finding out if we should compile a contract method @@ -680,7 +682,7 @@ impl Attributes { } pub fn is_test_function(&self) -> bool { - matches!(self.function, Some(FunctionAttribute::Test(_))) + matches!(self.function(), Some(FunctionAttribute::Test(_))) } /// True if these attributes mean the given function is an entry point function if it was @@ -708,11 +710,11 @@ impl Attributes { } pub fn is_foldable(&self) -> bool { - self.function.as_ref().map_or(false, |func_attribute| func_attribute.is_foldable()) + self.function().map_or(false, |func_attribute| func_attribute.is_foldable()) } pub fn is_no_predicates(&self) -> bool { - self.function.as_ref().map_or(false, |func_attribute| func_attribute.is_no_predicates()) + self.function().map_or(false, |func_attribute| func_attribute.is_no_predicates()) } pub fn has_varargs(&self) -> bool { @@ -727,7 +729,7 @@ impl Attributes { /// An Attribute can be either a Primary Attribute or a Secondary Attribute /// A Primary Attribute can alter the function type, thus there can only be one /// A secondary attribute has no effect and is either consumed by a library or used as a notice for the developer -#[derive(PartialEq, Eq, Hash, Debug, Clone, PartialOrd, Ord)] +#[derive(PartialEq, Eq, Debug, Clone)] pub enum Attribute { Function(FunctionAttribute), Secondary(SecondaryAttribute), @@ -742,115 +744,6 @@ impl fmt::Display for Attribute { } } -impl Attribute { - /// If the string is a fixed attribute return that, else - /// return the custom attribute - pub(crate) fn lookup_attribute( - word: &str, - span: Span, - contents_span: Span, - is_tag: bool, - ) -> Result { - // See if we can parse the word into "name ( contents )". - // We first split into "first_segment ( rest". - let word_segments = if let Some((first_segment, rest)) = word.trim().split_once('(') { - // Now we try to remove the final ")" (it must be at the end, if it exists) - if let Some(middle) = rest.strip_suffix(')') { - vec![first_segment.trim(), middle.trim()] - } else { - vec![word] - } - } else { - vec![word] - }; - - let validate = |slice: &str| { - let is_valid = slice - .chars() - .all(|ch| { - ch.is_ascii_alphabetic() - || ch.is_numeric() - || ch.is_ascii_punctuation() - || ch == ' ' - }) - .then_some(()); - - is_valid.ok_or(LexerErrorKind::MalformedFuncAttribute { span, found: word.to_owned() }) - }; - - if is_tag { - return Ok(Attribute::Secondary(SecondaryAttribute::Tag(CustomAttribute { - contents: word.to_owned(), - span, - contents_span, - }))); - } - - let attribute = match &word_segments[..] { - // Primary Attributes - ["foreign", name] => { - validate(name)?; - Attribute::Function(FunctionAttribute::Foreign(name.to_string())) - } - ["builtin", name] => { - validate(name)?; - Attribute::Function(FunctionAttribute::Builtin(name.to_string())) - } - ["oracle", name] => { - validate(name)?; - Attribute::Function(FunctionAttribute::Oracle(name.to_string())) - } - ["test"] => Attribute::Function(FunctionAttribute::Test(TestScope::None)), - ["fold"] => Attribute::Function(FunctionAttribute::Fold), - ["no_predicates"] => Attribute::Function(FunctionAttribute::NoPredicates), - ["inline_always"] => Attribute::Function(FunctionAttribute::InlineAlways), - ["test", name] => { - validate(name)?; - match TestScope::lookup_str(name) { - Some(scope) => Attribute::Function(FunctionAttribute::Test(scope)), - None => return Err(LexerErrorKind::MalformedTestAttribute { span }), - } - } - ["field", name] => { - validate(name)?; - Attribute::Secondary(SecondaryAttribute::Field(name.to_string())) - } - // Secondary attributes - ["deprecated"] => Attribute::Secondary(SecondaryAttribute::Deprecated(None)), - ["contract_library_method"] => { - Attribute::Secondary(SecondaryAttribute::ContractLibraryMethod) - } - ["abi", tag] => Attribute::Secondary(SecondaryAttribute::Abi(tag.to_string())), - ["export"] => Attribute::Secondary(SecondaryAttribute::Export), - ["deprecated", name] => { - if !name.starts_with('"') && !name.ends_with('"') { - return Err(LexerErrorKind::MalformedFuncAttribute { - span, - found: word.to_owned(), - }); - } - - Attribute::Secondary(SecondaryAttribute::Deprecated( - name.trim_matches('"').to_string().into(), - )) - } - ["varargs"] => Attribute::Secondary(SecondaryAttribute::Varargs), - ["use_callers_scope"] => Attribute::Secondary(SecondaryAttribute::UseCallersScope), - ["allow", tag] => Attribute::Secondary(SecondaryAttribute::Allow(tag.to_string())), - tokens => { - tokens.iter().try_for_each(|token| validate(token))?; - Attribute::Secondary(SecondaryAttribute::Meta(CustomAttribute { - contents: word.to_owned(), - span, - contents_span, - })) - } - }; - - Ok(attribute) - } -} - /// Primary Attributes are those which a function can only have one of. /// They change the FunctionKind and thus have direct impact on the IR output #[derive(PartialEq, Eq, Hash, Debug, Clone, PartialOrd, Ord)] @@ -946,7 +839,7 @@ impl fmt::Display for FunctionAttribute { /// Secondary attributes are those which a function can have many of. /// They are not able to change the `FunctionKind` and thus do not have direct impact on the IR output /// They are often consumed by libraries or used as notices for the developer -#[derive(PartialEq, Eq, Hash, Debug, Clone, PartialOrd, Ord)] +#[derive(PartialEq, Eq, Debug, Clone)] pub enum SecondaryAttribute { Deprecated(Option), // This is an attribute to specify that a function @@ -960,7 +853,7 @@ pub enum SecondaryAttribute { Tag(CustomAttribute), /// An attribute expected to run a comptime function of the same name: #[foo] - Meta(CustomAttribute), + Meta(MetaAttribute), Abi(String), @@ -977,14 +870,6 @@ pub enum SecondaryAttribute { } impl SecondaryAttribute { - pub(crate) fn as_custom(&self) -> Option<&CustomAttribute> { - if let Self::Tag(attribute) = self { - Some(attribute) - } else { - None - } - } - pub(crate) fn name(&self) -> Option { match self { SecondaryAttribute::Deprecated(_) => Some("deprecated".to_string()), @@ -994,7 +879,7 @@ impl SecondaryAttribute { SecondaryAttribute::Export => Some("export".to_string()), SecondaryAttribute::Field(_) => Some("field".to_string()), SecondaryAttribute::Tag(custom) => custom.name(), - SecondaryAttribute::Meta(custom) => custom.name(), + SecondaryAttribute::Meta(meta) => Some(meta.name.last_name().to_string()), SecondaryAttribute::Abi(_) => Some("abi".to_string()), SecondaryAttribute::Varargs => Some("varargs".to_string()), SecondaryAttribute::UseCallersScope => Some("use_callers_scope".to_string()), @@ -1020,7 +905,7 @@ impl SecondaryAttribute { format!("deprecated({note:?})") } SecondaryAttribute::Tag(ref attribute) => format!("'{}", attribute.contents), - SecondaryAttribute::Meta(ref attribute) => attribute.contents.to_string(), + SecondaryAttribute::Meta(ref meta) => meta.to_string(), SecondaryAttribute::ContractLibraryMethod => "contract_library_method".to_string(), SecondaryAttribute::Export => "export".to_string(), SecondaryAttribute::Field(ref k) => format!("field({k})"), @@ -1038,6 +923,25 @@ impl fmt::Display for SecondaryAttribute { } } +#[derive(PartialEq, Eq, Debug, Clone)] +pub struct MetaAttribute { + pub name: Path, + pub arguments: Vec, + pub span: Span, +} + +impl Display for MetaAttribute { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if self.arguments.is_empty() { + write!(f, "{}", self.name) + } else { + let args = + self.arguments.iter().map(ToString::to_string).collect::>().join(", "); + write!(f, "{}({})", self.name, args) + } + } +} + #[derive(PartialEq, Eq, Hash, Debug, Clone, PartialOrd, Ord)] pub struct CustomAttribute { pub contents: String, @@ -1059,38 +963,6 @@ impl CustomAttribute { } } -impl AsRef for FunctionAttribute { - fn as_ref(&self) -> &str { - match self { - FunctionAttribute::Foreign(string) => string, - FunctionAttribute::Builtin(string) => string, - FunctionAttribute::Oracle(string) => string, - FunctionAttribute::Test { .. } => "", - FunctionAttribute::Fold => "", - FunctionAttribute::NoPredicates => "", - FunctionAttribute::InlineAlways => "", - } - } -} - -impl AsRef for SecondaryAttribute { - fn as_ref(&self) -> &str { - match self { - SecondaryAttribute::Deprecated(Some(string)) => string, - SecondaryAttribute::Deprecated(None) => "", - SecondaryAttribute::Tag(attribute) => &attribute.contents, - SecondaryAttribute::Meta(attribute) => &attribute.contents, - SecondaryAttribute::Field(string) - | SecondaryAttribute::Abi(string) - | SecondaryAttribute::Allow(string) => string, - SecondaryAttribute::ContractLibraryMethod => "", - SecondaryAttribute::Export => "", - SecondaryAttribute::Varargs => "", - SecondaryAttribute::UseCallersScope => "", - } - } -} - /// Note that `self` is not present - it is a contextual keyword rather than a true one as it is /// only special within `impl`s. Otherwise `self` functions as a normal identifier. #[derive(PartialEq, Eq, Hash, Debug, Copy, Clone, PartialOrd, Ord, strum_macros::EnumIter)] diff --git a/noir/noir-repo/compiler/noirc_frontend/src/locations.rs b/noir/noir-repo/compiler/noirc_frontend/src/locations.rs index 4dd699251a6..ecae5b19a95 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/locations.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/locations.rs @@ -1,14 +1,11 @@ use fm::FileId; -use noirc_errors::Location; +use noirc_errors::{Location, Span}; use rangemap::RangeMap; use rustc_hash::FxHashMap as HashMap; use crate::{ ast::{FunctionDefinition, ItemVisibility}, - hir::{ - def_map::{ModuleDefId, ModuleId}, - resolution::import::PathResolutionItem, - }, + hir::def_map::{ModuleDefId, ModuleId}, node_interner::{ DefinitionId, FuncId, GlobalId, NodeInterner, ReferenceId, StructId, TraitId, TypeAliasId, }, @@ -37,6 +34,27 @@ impl LocationIndices { } } +pub struct ReferencesTracker<'a> { + interner: &'a mut NodeInterner, + file_id: FileId, +} + +impl<'a> ReferencesTracker<'a> { + pub fn new(interner: &'a mut NodeInterner, file_id: FileId) -> Self { + Self { interner, file_id } + } + + pub(crate) fn add_reference( + &mut self, + module_def_id: ModuleDefId, + span: Span, + is_self_type: bool, + ) { + let location = Location::new(span, self.file_id); + self.interner.add_module_def_id_reference(module_def_id, location, is_self_type); + } +} + impl NodeInterner { pub fn reference_location(&self, reference: ReferenceId) -> Location { match reference { @@ -102,37 +120,6 @@ impl NodeInterner { }; } - pub(crate) fn add_path_resolution_kind_reference( - &mut self, - kind: PathResolutionItem, - location: Location, - is_self_type: bool, - ) { - match kind { - PathResolutionItem::Module(module_id) => { - self.add_module_reference(module_id, location); - } - PathResolutionItem::Struct(struct_id) => { - self.add_struct_reference(struct_id, location, is_self_type); - } - PathResolutionItem::TypeAlias(type_alias_id) => { - self.add_alias_reference(type_alias_id, location); - } - PathResolutionItem::Trait(trait_id) => { - self.add_trait_reference(trait_id, location, is_self_type); - } - PathResolutionItem::Global(global_id) => { - self.add_global_reference(global_id, location); - } - PathResolutionItem::ModuleFunction(func_id) - | PathResolutionItem::StructFunction(_, _, func_id) - | PathResolutionItem::TypeAliasFunction(_, _, func_id) - | PathResolutionItem::TraitFunction(_, _, func_id) => { - self.add_function_reference(func_id, location); - } - } - } - pub(crate) fn add_module_reference(&mut self, id: ModuleId, location: Location) { self.add_reference(ReferenceId::Module(id), location, false); } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/ast.rs b/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/ast.rs index 7c7f87b7b8b..8f6817dc15d 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/ast.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/ast.rs @@ -237,13 +237,11 @@ pub enum InlineType { impl From<&Attributes> for InlineType { fn from(attributes: &Attributes) -> Self { - attributes.function.as_ref().map_or(InlineType::default(), |func_attribute| { - match func_attribute { - FunctionAttribute::Fold => InlineType::Fold, - FunctionAttribute::NoPredicates => InlineType::NoPredicates, - FunctionAttribute::InlineAlways => InlineType::InlineAlways, - _ => InlineType::default(), - } + attributes.function().map_or(InlineType::default(), |func_attribute| match func_attribute { + FunctionAttribute::Fold => InlineType::Fold, + FunctionAttribute::NoPredicates => InlineType::NoPredicates, + FunctionAttribute::InlineAlways => InlineType::InlineAlways, + _ => InlineType::default(), }) } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/errors.rs b/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/errors.rs index e2ff94f521b..c137a6fc90a 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/errors.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/errors.rs @@ -1,10 +1,13 @@ use noirc_errors::{CustomDiagnostic, FileDiagnostic, Location}; -use crate::{hir::comptime::InterpreterError, Type}; +use crate::{ + hir::{comptime::InterpreterError, type_check::TypeCheckError}, + Type, +}; #[derive(Debug)] pub enum MonomorphizationError { - UnknownArrayLength { length: Type, location: Location }, + UnknownArrayLength { length: Type, err: TypeCheckError, location: Location }, UnknownConstant { location: Location }, NoDefaultType { location: Location }, InternalError { message: &'static str, location: Location }, @@ -12,6 +15,7 @@ pub enum MonomorphizationError { ComptimeFnInRuntimeCode { name: String, location: Location }, ComptimeTypeInRuntimeCode { typ: String, location: Location }, CheckedTransmuteFailed { actual: Type, expected: Type, location: Location }, + CheckedCastFailed { actual: Type, expected: Type, location: Location }, } impl MonomorphizationError { @@ -23,6 +27,7 @@ impl MonomorphizationError { | MonomorphizationError::ComptimeFnInRuntimeCode { location, .. } | MonomorphizationError::ComptimeTypeInRuntimeCode { location, .. } | MonomorphizationError::CheckedTransmuteFailed { location, .. } + | MonomorphizationError::CheckedCastFailed { location, .. } | MonomorphizationError::NoDefaultType { location, .. } => *location, MonomorphizationError::InterpreterError(error) => error.get_location(), } @@ -41,14 +46,17 @@ impl From for FileDiagnostic { impl MonomorphizationError { fn into_diagnostic(self) -> CustomDiagnostic { let message = match &self { - MonomorphizationError::UnknownArrayLength { length, .. } => { - format!("Could not determine array length `{length}`") + MonomorphizationError::UnknownArrayLength { length, err, .. } => { + format!("Could not determine array length `{length}`, encountered error: `{err}`") } MonomorphizationError::UnknownConstant { .. } => { "Could not resolve constant".to_string() } MonomorphizationError::CheckedTransmuteFailed { actual, expected, .. } => { - format!("checked_transmute failed: `{actual}` != `{expected}`") + format!("checked_transmute failed: `{actual:?}` != `{expected:?}`") + } + MonomorphizationError::CheckedCastFailed { actual, expected, .. } => { + format!("Arithmetic generics simplification failed: `{actual:?}` != `{expected:?}`") } MonomorphizationError::NoDefaultType { location } => { let message = "Type annotation needed".into(); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/mod.rs index 21a2d533354..0ec26a5ca83 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -10,7 +10,7 @@ //! function, will monomorphize the entire reachable program. use crate::ast::{FunctionKind, IntegerBitSize, Signedness, UnaryOp, Visibility}; use crate::hir::comptime::InterpreterError; -use crate::hir::type_check::NoMatchingImplFoundError; +use crate::hir::type_check::{NoMatchingImplFoundError, TypeCheckError}; use crate::node_interner::{ExprId, ImplSearchErrorKind}; use crate::{ debug::DebugInstrumenter, @@ -221,14 +221,14 @@ impl<'interner> Monomorphizer<'interner> { let attributes = self.interner.function_attributes(&id); match self.interner.function_meta(&id).kind { FunctionKind::LowLevel => { - let attribute = attributes.function.as_ref().expect("all low level functions must contain a function attribute which contains the opcode which it links to"); + let attribute = attributes.function().expect("all low level functions must contain a function attribute which contains the opcode which it links to"); let opcode = attribute.foreign().expect( "ice: function marked as foreign, but attribute kind does not match this", ); Definition::LowLevel(opcode.to_string()) } FunctionKind::Builtin => { - let attribute = attributes.function.as_ref().expect("all builtin functions must contain a function attribute which contains the opcode which it links to"); + let attribute = attributes.function().expect("all builtin functions must contain a function attribute which contains the opcode which it links to"); let opcode = attribute.builtin().expect( "ice: function marked as builtin, but attribute kind does not match this", ); @@ -240,7 +240,7 @@ impl<'interner> Monomorphizer<'interner> { Definition::Function(id) } FunctionKind::Oracle => { - let attribute = attributes.function.as_ref().expect("all oracle functions must contain a function attribute which contains the opcode which it links to"); + let attribute = attributes.function().expect("all oracle functions must contain a function attribute which contains the opcode which it links to"); let opcode = attribute.oracle().expect( "ice: function marked as builtin, but attribute kind does not match this", ); @@ -570,9 +570,9 @@ impl<'interner> Monomorphizer<'interner> { let location = self.interner.expr_location(&array); let typ = Self::convert_type(&self.interner.id_type(array), location)?; - let length = length.evaluate_to_u32().ok_or_else(|| { + let length = length.evaluate_to_u32(location.span).map_err(|err| { let location = self.interner.expr_location(&array); - MonomorphizationError::UnknownArrayLength { location, length } + MonomorphizationError::UnknownArrayLength { location, err, length } })?; let contents = try_vecmap(0..length, |_| self.expr(repeated_element))?; @@ -922,17 +922,23 @@ impl<'interner> Monomorphizer<'interner> { TypeBinding::Unbound(_, _) => { unreachable!("Unbound type variable used in expression") } - TypeBinding::Bound(binding) => binding - .evaluate_to_field_element(&Kind::Numeric(numeric_typ.clone())) - .unwrap_or_else(|| { - panic!("Non-numeric type variable used in expression expecting a value") - }), + TypeBinding::Bound(binding) => { + let location = self.interner.id_location(expr_id); + binding + .evaluate_to_field_element( + &Kind::Numeric(numeric_typ.clone()), + location.span, + ) + .map_err(|err| MonomorphizationError::UnknownArrayLength { + length: binding.clone(), + err, + location, + })? + } }; let location = self.interner.id_location(expr_id); - if !Kind::Numeric(numeric_typ.clone()) - .unifies(&Kind::Numeric(Box::new(typ.clone()))) - { + if !Kind::Numeric(numeric_typ.clone()).unifies(&Kind::numeric(typ.clone())) { let message = "ICE: Generic's kind does not match expected type"; return Err(MonomorphizationError::InternalError { location, message }); } @@ -951,20 +957,51 @@ impl<'interner> Monomorphizer<'interner> { HirType::FieldElement => ast::Type::Field, HirType::Integer(sign, bits) => ast::Type::Integer(*sign, *bits), HirType::Bool => ast::Type::Bool, - HirType::String(size) => ast::Type::String(size.evaluate_to_u32().unwrap_or(0)), + HirType::String(size) => { + let size = match size.evaluate_to_u32(location.span) { + Ok(size) => size, + // only default variable sizes to size 0 + Err(TypeCheckError::NonConstantEvaluated { .. }) => 0, + Err(err) => { + let length = size.as_ref().clone(); + return Err(MonomorphizationError::UnknownArrayLength { + location, + err, + length, + }); + } + }; + ast::Type::String(size) + } HirType::FmtString(size, fields) => { - let size = size.evaluate_to_u32().unwrap_or(0); + let size = match size.evaluate_to_u32(location.span) { + Ok(size) => size, + // only default variable sizes to size 0 + Err(TypeCheckError::NonConstantEvaluated { .. }) => 0, + Err(err) => { + let length = size.as_ref().clone(); + return Err(MonomorphizationError::UnknownArrayLength { + location, + err, + length, + }); + } + }; let fields = Box::new(Self::convert_type(fields.as_ref(), location)?); ast::Type::FmtString(size, fields) } HirType::Unit => ast::Type::Unit, HirType::Array(length, element) => { let element = Box::new(Self::convert_type(element.as_ref(), location)?); - let length = match length.evaluate_to_u32() { - Some(length) => length, - None => { + let length = match length.evaluate_to_u32(location.span) { + Ok(length) => length, + Err(err) => { let length = length.as_ref().clone(); - return Err(MonomorphizationError::UnknownArrayLength { location, length }); + return Err(MonomorphizationError::UnknownArrayLength { + location, + err, + length, + }); } }; ast::Type::Array(length, element) @@ -988,6 +1025,11 @@ impl<'interner> Monomorphizer<'interner> { ast::Type::Field } + HirType::CheckedCast { from, to } => { + Self::check_checked_cast(from, to, location)?; + Self::convert_type(to, location)? + } + HirType::TypeVariable(ref binding) => { let type_var_kind = match &*binding.borrow() { TypeBinding::Bound(ref binding) => { @@ -1094,6 +1136,11 @@ impl<'interner> Monomorphizer<'interner> { Ok(()) } } + HirType::CheckedCast { from, to } => { + Self::check_checked_cast(from, to, location)?; + Self::check_type(to, location) + } + HirType::FmtString(_size, fields) => Self::check_type(fields.as_ref(), location), HirType::Array(_length, element) => Self::check_type(element.as_ref(), location), HirType::Slice(element) => Self::check_type(element.as_ref(), location), @@ -1165,6 +1212,40 @@ impl<'interner> Monomorphizer<'interner> { } } + /// Check that the 'from' and to' sides of a CheckedCast unify and + /// that if the 'to' side evaluates to a field element, that the 'from' side + /// evaluates to the same field element + fn check_checked_cast( + from: &Type, + to: &Type, + location: Location, + ) -> Result<(), MonomorphizationError> { + if from.unify(to).is_err() { + return Err(MonomorphizationError::CheckedCastFailed { + actual: to.clone(), + expected: from.clone(), + location, + }); + } + let to_value = to.evaluate_to_field_element(&to.kind(), location.span); + if to_value.is_ok() { + let skip_simplifications = false; + let from_value = from.evaluate_to_field_element_helper( + &to.kind(), + location.span, + skip_simplifications, + ); + if from_value.is_err() || from_value.unwrap() != to_value.clone().unwrap() { + return Err(MonomorphizationError::CheckedCastFailed { + actual: HirType::Constant(to_value.unwrap(), to.kind()), + expected: from.clone(), + location, + }); + } + } + Ok(()) + } + fn is_function_closure(&self, t: ast::Type) -> bool { if self.is_function_closure_type(&t) { true diff --git a/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs b/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs index c743d564339..736d37fe83f 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs @@ -2439,6 +2439,7 @@ fn get_type_method_key(typ: &Type) -> Option { | Type::Error | Type::Struct(_, _) | Type::InfixExpr(..) + | Type::CheckedCast { .. } | Type::TraitAsType(..) => None, } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/errors.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/errors.rs index 3315b38a351..63471efac43 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/errors.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/errors.rs @@ -95,6 +95,15 @@ pub enum ParserErrorReason { AssociatedTypesNotAllowedInPaths, #[error("Associated types are not allowed on a method call")] AssociatedTypesNotAllowedInMethodCalls, + #[error( + "Wrong number of arguments for attribute `{}`. Expected {}, found {}", + name, + if min == max { min.to_string() } else { format!("between {} and {}", min, max) }, + found + )] + WrongNumberOfAttributeArguments { name: String, min: usize, max: usize, found: usize }, + #[error("The `deprecated` attribute expects a string argument")] + DeprecatedAttributeExpectsAStringArgument, } /// Represents a parsing error, or a parsing error in the making. diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser.rs index 0030144b5e1..f369839ddd4 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser.rs @@ -318,6 +318,30 @@ impl<'a> Parser<'a> { } } + fn eat_attribute_start(&mut self) -> Option { + if matches!(self.token.token(), Token::AttributeStart { is_inner: false, .. }) { + let token = self.bump(); + match token.into_token() { + Token::AttributeStart { is_tag, .. } => Some(is_tag), + _ => unreachable!(), + } + } else { + None + } + } + + fn eat_inner_attribute_start(&mut self) -> Option { + if matches!(self.token.token(), Token::AttributeStart { is_inner: true, .. }) { + let token = self.bump(); + match token.into_token() { + Token::AttributeStart { is_tag, .. } => Some(is_tag), + _ => unreachable!(), + } + } else { + None + } + } + fn eat_comma(&mut self) -> bool { self.eat(Token::Comma) } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/attributes.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/attributes.rs index ffba74003b7..12cb37edb4b 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/attributes.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/attributes.rs @@ -1,32 +1,87 @@ use noirc_errors::Span; +use crate::ast::{Expression, ExpressionKind, Ident, Literal, Path}; +use crate::lexer::errors::LexerErrorKind; +use crate::parser::labels::ParsingRuleLabel; use crate::parser::ParserErrorReason; -use crate::token::SecondaryAttribute; -use crate::token::{Attribute, Token, TokenKind}; +use crate::token::{Attribute, FunctionAttribute, MetaAttribute, TestScope, Token}; +use crate::token::{CustomAttribute, SecondaryAttribute}; use super::parse_many::without_separator; use super::Parser; impl<'a> Parser<'a> { - /// InnerAttribute = inner_attribute + /// InnerAttribute = '#![' SecondaryAttribute ']' pub(super) fn parse_inner_attribute(&mut self) -> Option { - let token = self.eat_kind(TokenKind::InnerAttribute)?; - match token.into_token() { - Token::InnerAttribute(attribute) => Some(attribute), - _ => unreachable!(), + let start_span = self.current_token_span; + let is_tag = self.eat_inner_attribute_start()?; + let attribute = if is_tag { + self.parse_tag_attribute(start_span) + } else { + self.parse_non_tag_attribute(start_span) + }; + + match attribute { + Attribute::Function(function_attribute) => { + self.errors.push( + LexerErrorKind::InvalidInnerAttribute { + span: self.span_since(start_span), + found: function_attribute.to_string(), + } + .into(), + ); + None + } + Attribute::Secondary(secondary_attribute) => Some(secondary_attribute), } } - /// Attributes = attribute* + /// Attributes = Attribute* pub(super) fn parse_attributes(&mut self) -> Vec<(Attribute, Span)> { self.parse_many("attributes", without_separator(), Self::parse_attribute) } - fn parse_attribute(&mut self) -> Option<(Attribute, Span)> { - self.eat_kind(TokenKind::Attribute).map(|token| match token.into_token() { - Token::Attribute(attribute) => (attribute, self.previous_token_span), - _ => unreachable!(), - }) + /// Attribute = '#[' (FunctionAttribute | SecondaryAttribute) ']' + /// + /// FunctionAttribute + /// = 'builtin' '(' AttributeValue ')' + /// | 'fold' + /// | 'foreign' '(' AttributeValue ')' + /// | 'inline_always' + /// | 'no_predicates' + /// | 'oracle' '(' AttributeValue ')' + /// | 'recursive' + /// | 'test' + /// | 'test' '(' 'should_fail' ')' + /// | 'test' '(' 'should_fail_with' '=' string ')' + /// + /// SecondaryAttribute + /// = 'abi' '(' AttributeValue ')' + /// | 'allow' '(' AttributeValue ')' + /// | 'deprecated' + /// | 'deprecated' '(' string ')' + /// | 'contract_library_method' + /// | 'export' + /// | 'field' '(' AttributeValue ')' + /// | 'use_callers_scope' + /// | 'varargs' + /// | MetaAttribute + /// + /// MetaAttribute + /// = Path Arguments? + /// + /// AttributeValue + /// = Path + /// | integer + pub(crate) fn parse_attribute(&mut self) -> Option<(Attribute, Span)> { + let start_span = self.current_token_span; + let is_tag = self.eat_attribute_start()?; + let attribute = if is_tag { + self.parse_tag_attribute(start_span) + } else { + self.parse_non_tag_attribute(start_span) + }; + Some((attribute, self.span_since(start_span))) } pub(super) fn validate_secondary_attributes( @@ -44,17 +99,315 @@ impl<'a> Parser<'a> { }) .collect() } + + fn parse_tag_attribute(&mut self, start_span: Span) -> Attribute { + let contents_start_span = self.current_token_span; + let mut contents_span = contents_start_span; + let mut contents = String::new(); + + let mut brackets_count = 1; // 1 because of the starting `#[` + + while !self.at_eof() { + if self.at(Token::LeftBracket) { + brackets_count += 1; + } else if self.at(Token::RightBracket) { + brackets_count -= 1; + if brackets_count == 0 { + contents_span = self.span_since(contents_start_span); + self.bump(); + break; + } + } + + contents.push_str(&self.token.to_string()); + self.bump(); + } + + Attribute::Secondary(SecondaryAttribute::Tag(CustomAttribute { + contents, + span: self.span_since(start_span), + contents_span, + })) + } + + fn parse_non_tag_attribute(&mut self, start_span: Span) -> Attribute { + if matches!(&self.token.token(), Token::Keyword(..)) + && (self.next_is(Token::LeftParen) || self.next_is(Token::RightBracket)) + { + // This is a Meta attribute with the syntax `keyword(arg1, arg2, .., argN)` + let path = Path::from_single(self.token.to_string(), self.current_token_span); + self.bump(); + self.parse_meta_attribute(path, start_span) + } else if let Some(path) = self.parse_path_no_turbofish() { + if let Some(ident) = path.as_ident() { + if ident.0.contents == "test" { + // The test attribute is the only secondary attribute that has `a = b` in its syntax + // (`should_fail_with = "..."``) so we parse it differently. + self.parse_test_attribute(start_span) + } else { + // Every other attribute has the form `name(arg1, arg2, .., argN)` + self.parse_ident_attribute_other_than_test(ident, start_span) + } + } else { + // This is a Meta attribute with the syntax `path(arg1, arg2, .., argN)` + self.parse_meta_attribute(path, start_span) + } + } else { + self.expected_label(ParsingRuleLabel::Path); + self.parse_tag_attribute(start_span) + } + } + + fn parse_meta_attribute(&mut self, name: Path, start_span: Span) -> Attribute { + let arguments = self.parse_arguments().unwrap_or_default(); + self.skip_until_right_bracket(); + Attribute::Secondary(SecondaryAttribute::Meta(MetaAttribute { + name, + arguments, + span: self.span_since(start_span), + })) + } + + fn parse_ident_attribute_other_than_test( + &mut self, + ident: &Ident, + start_span: Span, + ) -> Attribute { + let arguments = self.parse_arguments().unwrap_or_default(); + self.skip_until_right_bracket(); + match ident.0.contents.as_str() { + "abi" => self.parse_single_name_attribute(ident, arguments, start_span, |name| { + Attribute::Secondary(SecondaryAttribute::Abi(name)) + }), + "allow" => self.parse_single_name_attribute(ident, arguments, start_span, |name| { + Attribute::Secondary(SecondaryAttribute::Allow(name)) + }), + "builtin" => self.parse_single_name_attribute(ident, arguments, start_span, |name| { + Attribute::Function(FunctionAttribute::Builtin(name)) + }), + "deprecated" => self.parse_deprecated_attribute(ident, arguments), + "contract_library_method" => { + let attr = Attribute::Secondary(SecondaryAttribute::ContractLibraryMethod); + self.parse_no_args_attribute(ident, arguments, attr) + } + "export" => { + let attr = Attribute::Secondary(SecondaryAttribute::Export); + self.parse_no_args_attribute(ident, arguments, attr) + } + "field" => self.parse_single_name_attribute(ident, arguments, start_span, |name| { + Attribute::Secondary(SecondaryAttribute::Field(name)) + }), + "fold" => { + let attr = Attribute::Function(FunctionAttribute::Fold); + self.parse_no_args_attribute(ident, arguments, attr) + } + "foreign" => self.parse_single_name_attribute(ident, arguments, start_span, |name| { + Attribute::Function(FunctionAttribute::Foreign(name)) + }), + "inline_always" => { + let attr = Attribute::Function(FunctionAttribute::InlineAlways); + self.parse_no_args_attribute(ident, arguments, attr) + } + "no_predicates" => { + let attr = Attribute::Function(FunctionAttribute::NoPredicates); + self.parse_no_args_attribute(ident, arguments, attr) + } + "oracle" => self.parse_single_name_attribute(ident, arguments, start_span, |name| { + Attribute::Function(FunctionAttribute::Oracle(name)) + }), + "use_callers_scope" => { + let attr = Attribute::Secondary(SecondaryAttribute::UseCallersScope); + self.parse_no_args_attribute(ident, arguments, attr) + } + "varargs" => { + let attr = Attribute::Secondary(SecondaryAttribute::Varargs); + self.parse_no_args_attribute(ident, arguments, attr) + } + _ => Attribute::Secondary(SecondaryAttribute::Meta(MetaAttribute { + name: Path::from_ident(ident.clone()), + arguments, + span: self.span_since(start_span), + })), + } + } + + fn parse_deprecated_attribute( + &mut self, + ident: &Ident, + mut arguments: Vec, + ) -> Attribute { + if arguments.is_empty() { + return Attribute::Secondary(SecondaryAttribute::Deprecated(None)); + } + + if arguments.len() > 1 { + self.push_error( + ParserErrorReason::WrongNumberOfAttributeArguments { + name: ident.to_string(), + min: 0, + max: 1, + found: arguments.len(), + }, + ident.span(), + ); + return Attribute::Secondary(SecondaryAttribute::Deprecated(None)); + } + + let argument = arguments.remove(0); + let ExpressionKind::Literal(Literal::Str(message)) = argument.kind else { + self.push_error( + ParserErrorReason::DeprecatedAttributeExpectsAStringArgument, + argument.span, + ); + return Attribute::Secondary(SecondaryAttribute::Deprecated(None)); + }; + + Attribute::Secondary(SecondaryAttribute::Deprecated(Some(message))) + } + + fn parse_test_attribute(&mut self, start_span: Span) -> Attribute { + let scope = if self.eat_left_paren() { + let scope = if let Some(ident) = self.eat_ident() { + match ident.0.contents.as_str() { + "should_fail" => Some(TestScope::ShouldFailWith { reason: None }), + "should_fail_with" => { + self.eat_or_error(Token::Assign); + if let Some(reason) = self.eat_str() { + Some(TestScope::ShouldFailWith { reason: Some(reason) }) + } else { + Some(TestScope::ShouldFailWith { reason: None }) + } + } + _ => None, + } + } else { + None + }; + self.eat_or_error(Token::RightParen); + scope + } else { + Some(TestScope::None) + }; + + self.skip_until_right_bracket(); + + let scope = if let Some(scope) = scope { + scope + } else { + self.errors.push( + LexerErrorKind::MalformedTestAttribute { span: self.span_since(start_span) }.into(), + ); + TestScope::None + }; + + Attribute::Function(FunctionAttribute::Test(scope)) + } + + fn parse_single_name_attribute( + &mut self, + ident: &Ident, + mut arguments: Vec, + start_span: Span, + f: F, + ) -> Attribute + where + F: FnOnce(String) -> Attribute, + { + if arguments.len() != 1 { + self.push_error( + ParserErrorReason::WrongNumberOfAttributeArguments { + name: ident.to_string(), + min: 1, + max: 1, + found: arguments.len(), + }, + self.current_token_span, + ); + return f(String::new()); + } + + let argument = arguments.remove(0); + match argument.kind { + ExpressionKind::Variable(..) | ExpressionKind::Literal(Literal::Integer(..)) => { + f(argument.to_string()) + } + _ => { + let span = self.span_since(start_span); + self.errors.push( + LexerErrorKind::MalformedFuncAttribute { span, found: argument.to_string() } + .into(), + ); + f(String::new()) + } + } + } + + fn parse_no_args_attribute( + &mut self, + ident: &Ident, + arguments: Vec, + attribute: Attribute, + ) -> Attribute { + if !arguments.is_empty() { + self.push_error( + ParserErrorReason::WrongNumberOfAttributeArguments { + name: ident.to_string(), + min: 0, + max: 0, + found: arguments.len(), + }, + ident.span(), + ); + } + + attribute + } + + fn skip_until_right_bracket(&mut self) { + let mut brackets_count = 1; // 1 because of the starting `#[` + + while !self.at_eof() { + if self.at(Token::LeftBracket) { + brackets_count += 1; + } else if self.at(Token::RightBracket) { + brackets_count -= 1; + if brackets_count == 0 { + self.bump(); + break; + } + } + + self.expected_token(Token::RightBracket); + self.bump(); + } + } } #[cfg(test)] mod tests { + use noirc_errors::Span; + use crate::{ parser::{parser::tests::expect_no_errors, Parser}, token::{Attribute, FunctionAttribute, SecondaryAttribute, TestScope}, }; + fn parse_inner_secondary_attribute_no_errors(src: &str, expected: SecondaryAttribute) { + let mut parser = Parser::for_str(src); + let attribute = parser.parse_inner_attribute(); + expect_no_errors(&parser.errors); + assert_eq!(attribute.unwrap(), expected); + } + + fn parse_attribute_no_errors(src: &str, expected: Attribute) { + let mut parser = Parser::for_str(src); + let (attribute, _span) = parser.parse_attribute().unwrap(); + expect_no_errors(&parser.errors); + assert_eq!(attribute, expected); + } + #[test] - fn parses_inner_attribute() { + fn parses_inner_attribute_as_tag() { let src = "#!['hello]"; let mut parser = Parser::for_str(src); let Some(SecondaryAttribute::Tag(custom)) = parser.parse_inner_attribute() else { @@ -62,6 +415,210 @@ mod tests { }; expect_no_errors(&parser.errors); assert_eq!(custom.contents, "hello"); + assert_eq!(custom.span, Span::from(0..src.len() as u32)); + assert_eq!(custom.contents_span, Span::from(4..src.len() as u32 - 1)); + } + + #[test] + fn parses_inner_attribute_as_tag_with_nested_brackets() { + let src = "#!['hello[1]]"; + let mut parser = Parser::for_str(src); + let Some(SecondaryAttribute::Tag(custom)) = parser.parse_inner_attribute() else { + panic!("Expected inner tag attribute"); + }; + expect_no_errors(&parser.errors); + assert_eq!(custom.contents, "hello[1]"); + } + + #[test] + fn parses_inner_attribute_deprecated() { + let src = "#![deprecated]"; + let expected = SecondaryAttribute::Deprecated(None); + parse_inner_secondary_attribute_no_errors(src, expected); + } + + #[test] + fn parses_inner_attribute_deprecated_with_message() { + let src = "#![deprecated(\"use something else\")]"; + let expected = SecondaryAttribute::Deprecated(Some("use something else".to_string())); + parse_inner_secondary_attribute_no_errors(src, expected); + } + + #[test] + fn parses_inner_attribute_contract_library_method() { + let src = "#![contract_library_method]"; + let expected = SecondaryAttribute::ContractLibraryMethod; + parse_inner_secondary_attribute_no_errors(src, expected); + } + + #[test] + fn parses_inner_attribute_export() { + let src = "#![export]"; + let expected = SecondaryAttribute::Export; + parse_inner_secondary_attribute_no_errors(src, expected); + } + + #[test] + fn parses_inner_attribute_varargs() { + let src = "#![varargs]"; + let expected = SecondaryAttribute::Varargs; + parse_inner_secondary_attribute_no_errors(src, expected); + } + + #[test] + fn parses_inner_attribute_use_callers_scope() { + let src = "#![use_callers_scope]"; + let expected = SecondaryAttribute::UseCallersScope; + parse_inner_secondary_attribute_no_errors(src, expected); + } + + #[test] + fn parses_attribute_abi() { + let src = "#[abi(foo)]"; + let expected = Attribute::Secondary(SecondaryAttribute::Abi("foo".to_string())); + parse_attribute_no_errors(src, expected); + } + + #[test] + fn parses_attribute_foreign() { + let src = "#[foreign(foo)]"; + let expected = Attribute::Function(FunctionAttribute::Foreign("foo".to_string())); + parse_attribute_no_errors(src, expected); + } + + #[test] + fn parses_attribute_builtin() { + let src = "#[builtin(foo)]"; + let expected = Attribute::Function(FunctionAttribute::Builtin("foo".to_string())); + parse_attribute_no_errors(src, expected); + } + + #[test] + fn parses_attribute_oracle() { + let src = "#[oracle(foo)]"; + let expected = Attribute::Function(FunctionAttribute::Oracle("foo".to_string())); + parse_attribute_no_errors(src, expected); + } + + #[test] + fn parses_attribute_fold() { + let src = "#[fold]"; + let expected = Attribute::Function(FunctionAttribute::Fold); + parse_attribute_no_errors(src, expected); + } + + #[test] + fn parses_attribute_no_predicates() { + let src = "#[no_predicates]"; + let expected = Attribute::Function(FunctionAttribute::NoPredicates); + parse_attribute_no_errors(src, expected); + } + + #[test] + fn parses_attribute_inline_always() { + let src = "#[inline_always]"; + let expected = Attribute::Function(FunctionAttribute::InlineAlways); + parse_attribute_no_errors(src, expected); + } + + #[test] + fn parses_attribute_field() { + let src = "#[field(bn254)]"; + let expected = Attribute::Secondary(SecondaryAttribute::Field("bn254".to_string())); + parse_attribute_no_errors(src, expected); + } + + #[test] + fn parses_attribute_field_with_integer() { + let src = "#[field(23)]"; + let expected = Attribute::Secondary(SecondaryAttribute::Field("23".to_string())); + parse_attribute_no_errors(src, expected); + } + + #[test] + fn parses_attribute_allow() { + let src = "#[allow(unused_vars)]"; + let expected = Attribute::Secondary(SecondaryAttribute::Allow("unused_vars".to_string())); + parse_attribute_no_errors(src, expected); + } + + #[test] + fn parses_attribute_test_no_scope() { + let src = "#[test]"; + let expected = Attribute::Function(FunctionAttribute::Test(TestScope::None)); + parse_attribute_no_errors(src, expected); + } + + #[test] + fn parses_attribute_test_should_fail() { + let src = "#[test(should_fail)]"; + let expected = Attribute::Function(FunctionAttribute::Test(TestScope::ShouldFailWith { + reason: None, + })); + parse_attribute_no_errors(src, expected); + } + + #[test] + fn parses_attribute_test_should_fail_with() { + let src = "#[test(should_fail_with = \"reason\")]"; + let expected = Attribute::Function(FunctionAttribute::Test(TestScope::ShouldFailWith { + reason: Some("reason".to_string()), + })); + parse_attribute_no_errors(src, expected); + } + + #[test] + fn parses_meta_attribute_single_identifier_no_arguments() { + let src = "#[foo]"; + let mut parser = Parser::for_str(src); + let (attribute, _span) = parser.parse_attribute().unwrap(); + expect_no_errors(&parser.errors); + let Attribute::Secondary(SecondaryAttribute::Meta(meta)) = attribute else { + panic!("Expected meta attribute"); + }; + assert_eq!(meta.name.to_string(), "foo"); + assert!(meta.arguments.is_empty()); + } + + #[test] + fn parses_meta_attribute_single_identifier_as_keyword() { + let src = "#[dep]"; + let mut parser = Parser::for_str(src); + let (attribute, _span) = parser.parse_attribute().unwrap(); + expect_no_errors(&parser.errors); + let Attribute::Secondary(SecondaryAttribute::Meta(meta)) = attribute else { + panic!("Expected meta attribute"); + }; + assert_eq!(meta.name.to_string(), "dep"); + assert!(meta.arguments.is_empty()); + } + + #[test] + fn parses_meta_attribute_single_identifier_with_arguments() { + let src = "#[foo(1, 2, 3)]"; + let mut parser = Parser::for_str(src); + let (attribute, _span) = parser.parse_attribute().unwrap(); + expect_no_errors(&parser.errors); + let Attribute::Secondary(SecondaryAttribute::Meta(meta)) = attribute else { + panic!("Expected meta attribute"); + }; + assert_eq!(meta.name.to_string(), "foo"); + assert_eq!(meta.arguments.len(), 3); + assert_eq!(meta.arguments[0].to_string(), "1"); + } + + #[test] + fn parses_meta_attribute_path_with_arguments() { + let src = "#[foo::bar(1, 2, 3)]"; + let mut parser = Parser::for_str(src); + let (attribute, _span) = parser.parse_attribute().unwrap(); + expect_no_errors(&parser.errors); + let Attribute::Secondary(SecondaryAttribute::Meta(meta)) = attribute else { + panic!("Expected meta attribute"); + }; + assert_eq!(meta.name.to_string(), "foo::bar"); + assert_eq!(meta.arguments.len(), 3); + assert_eq!(meta.arguments[0].to_string(), "1"); } #[test] diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/function.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/function.rs index a60bc6e7c1d..438757b5d79 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/function.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/function.rs @@ -246,22 +246,23 @@ impl<'a> Parser<'a> { } fn validate_attributes(&mut self, attributes: Vec<(Attribute, Span)>) -> Attributes { - let mut primary = None; + let mut function = None; let mut secondary = Vec::new(); - for (attribute, span) in attributes { + for (index, (attribute, span)) in attributes.into_iter().enumerate() { match attribute { Attribute::Function(attr) => { - if primary.is_some() { + if function.is_none() { + function = Some((attr, index)); + } else { self.push_error(ParserErrorReason::MultipleFunctionAttributesFound, span); } - primary = Some(attr); } Attribute::Secondary(attr) => secondary.push(attr), } } - Attributes { function: primary, secondary } + Attributes { function, secondary } } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs index 17accbd8366..f72741721d8 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs @@ -1,6 +1,7 @@ #![cfg(test)] mod aliases; +mod arithmetic_generics; mod bound_checks; mod imports; mod metaprogramming; @@ -36,6 +37,8 @@ use crate::hir::def_collector::dc_crate::DefCollector; use crate::hir::def_map::{CrateDefMap, LocalModuleId}; use crate::hir_def::expr::HirExpression; use crate::hir_def::stmt::HirStatement; +use crate::monomorphization::ast::Program; +use crate::monomorphization::errors::MonomorphizationError; use crate::monomorphization::monomorphize; use crate::parser::{ItemKind, ParserErrorReason}; use crate::token::SecondaryAttribute; @@ -57,6 +60,15 @@ pub(crate) fn remove_experimental_warnings(errors: &mut Vec<(CompilationError, F } pub(crate) fn get_program(src: &str) -> (ParsedModule, Context, Vec<(CompilationError, FileId)>) { + get_program_with_maybe_parser_errors( + src, false, // allow parser errors + ) +} + +pub(crate) fn get_program_with_maybe_parser_errors( + src: &str, + allow_parser_errors: bool, +) -> (ParsedModule, Context, Vec<(CompilationError, FileId)>) { let root = std::path::Path::new("/"); let fm = FileManager::new(root); @@ -69,7 +81,7 @@ pub(crate) fn get_program(src: &str) -> (ParsedModule, Context, Vec<(Compilation let mut errors = vecmap(parser_errors, |e| (e.into(), root_file_id)); remove_experimental_warnings(&mut errors); - if !has_parser_error(&errors) { + if allow_parser_errors || !has_parser_error(&errors) { let inner_attributes: Vec = program .items .iter() @@ -1239,10 +1251,18 @@ fn resolve_fmt_strings() { } } -fn check_rewrite(src: &str, expected: &str) { +fn monomorphize_program(src: &str) -> Result { let (_program, mut context, _errors) = get_program(src); let main_func_id = context.def_interner.find_function("main").unwrap(); - let program = monomorphize(main_func_id, &mut context.def_interner).unwrap(); + monomorphize(main_func_id, &mut context.def_interner) +} + +fn get_monomorphization_error(src: &str) -> Option { + monomorphize_program(src).err() +} + +fn check_rewrite(src: &str, expected: &str) { + let program = monomorphize_program(src).unwrap(); assert!(format!("{}", program) == expected); } @@ -1982,9 +2002,9 @@ fn numeric_generic_u16_array_size() { fn numeric_generic_field_larger_than_u32() { let src = r#" global A: Field = 4294967297; - + fn foo() { } - + fn main() { let _ = foo::(); } @@ -2014,14 +2034,14 @@ fn numeric_generic_field_arithmetic_larger_than_u32() { F } } - + // 2^32 - 1 global A: Field = 4294967295; - + fn foo() -> Foo { Foo {} } - + fn main() { let _ = foo::().size(); } @@ -3189,25 +3209,6 @@ fn as_trait_path_syntax_no_impl() { assert!(matches!(&errors[0].0, TypeError(TypeCheckError::NoMatchingImplFound { .. }))); } -#[test] -fn arithmetic_generics_canonicalization_deduplication_regression() { - let source = r#" - struct ArrData { - a: [Field; N], - b: [Field; N + N - 1], - } - - fn main() { - let _f: ArrData<5> = ArrData { - a: [0; 5], - b: [0; 9], - }; - } - "#; - let errors = get_program_errors(source); - assert_eq!(errors.len(), 0); -} - #[test] fn infer_globals_to_u32_from_type_use() { let src = r#" @@ -3393,6 +3394,36 @@ fn arithmetic_generics_rounding_fail() { let errors = get_program_errors(src); assert_eq!(errors.len(), 1); + assert!(matches!( + errors[0].0, + CompilationError::TypeError(TypeCheckError::TypeMismatch { .. }) + )); +} + +#[test] +fn arithmetic_generics_rounding_fail_on_struct() { + let src = r#" + struct W {} + + fn foo(_x: W, _y: W) -> W { + W {} + } + + fn main() { + let w_2: W<2> = W {}; + let w_3: W<3> = W {}; + // Do not simplify N/M*M to just N + // This should be 3/2*2 = 2, not 3 + let _: W<3> = foo(w_3, w_2); + } + "#; + + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + assert!(matches!( + errors[0].0, + CompilationError::TypeError(TypeCheckError::TypeMismatch { .. }) + )); } #[test] @@ -3438,7 +3469,7 @@ fn unconditional_recursion_fail() { fn main() -> pub u64 { foo(1, main()) } - fn foo(a: u64, b: u64) -> u64 { + fn foo(a: u64, b: u64) -> u64 { a + b } "#, @@ -3623,3 +3654,41 @@ fn use_type_alias_to_generic_concrete_type_in_method_call() { "#; assert_no_errors(src); } + +#[test] +fn allows_struct_with_generic_infix_type_as_main_input_1() { + let src = r#" + struct Foo { + x: [u64; N * 2], + } + + fn main(_x: Foo<18>) {} + "#; + assert_no_errors(src); +} + +#[test] +fn allows_struct_with_generic_infix_type_as_main_input_2() { + let src = r#" + struct Foo { + x: [u64; N * 2], + } + + fn main(_x: Foo<2 * 9>) {} + "#; + assert_no_errors(src); +} + +#[test] +fn allows_struct_with_generic_infix_type_as_main_input_3() { + let src = r#" + struct Foo { + x: [u64; N * 2], + } + + global N = 9; + + fn main(_x: Foo) {} + "#; + assert_no_errors(src); +} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/arithmetic_generics.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/arithmetic_generics.rs new file mode 100644 index 00000000000..3328fe439ae --- /dev/null +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/arithmetic_generics.rs @@ -0,0 +1,155 @@ +#![cfg(test)] + +use acvm::{AcirField, FieldElement}; + +use super::get_program_errors; +use crate::hir::type_check::TypeCheckError; +use crate::hir_def::types::{BinaryTypeOperator, Type}; +use crate::monomorphization::errors::MonomorphizationError; +use crate::tests::get_monomorphization_error; + +#[test] +fn arithmetic_generics_canonicalization_deduplication_regression() { + let source = r#" + struct ArrData { + a: [Field; N], + b: [Field; N + N - 1], + } + + fn main() { + let _f: ArrData<5> = ArrData { + a: [0; 5], + b: [0; 9], + }; + } + "#; + let errors = get_program_errors(source); + assert_eq!(errors.len(), 0); +} + +#[test] +fn checked_casts_do_not_prevent_canonicalization() { + // Regression test for https://github.com/noir-lang/noir/issues/6495 + let source = r#" + pub trait Serialize { + fn serialize(self) -> [Field; N]; + } + + pub struct Counted { + pub inner: T, + } + + pub fn append(array1: [T; N]) -> [T; N + 1] { + [array1[0]; N + 1] + } + + impl Serialize for Counted + where + T: Serialize, + { + fn serialize(self) -> [Field; N] { + append(self.inner.serialize()) + } + } + "#; + let errors = get_program_errors(source); + println!("{:?}", errors); + assert_eq!(errors.len(), 0); +} + +#[test] +fn arithmetic_generics_checked_cast_zeros() { + let source = r#" + struct W {} + + fn foo(_x: W) -> W<(0 * N) / (N % N)> { + W {} + } + + fn bar(_x: W) -> u1 { + N + } + + fn main() -> pub u1 { + let w_0: W<0> = W {}; + let w: W<_> = foo(w_0); + bar(w) + } + "#; + + let errors = get_program_errors(source); + assert_eq!(errors.len(), 0); + + let monomorphization_error = get_monomorphization_error(source); + assert!(monomorphization_error.is_some()); + + // Expect a CheckedCast (0 % 0) failure + let monomorphization_error = monomorphization_error.unwrap(); + if let MonomorphizationError::UnknownArrayLength { ref length, ref err, location: _ } = + monomorphization_error + { + match length { + Type::CheckedCast { from, to } => { + assert!(matches!(*from.clone(), Type::InfixExpr { .. })); + assert!(matches!(*to.clone(), Type::InfixExpr { .. })); + } + _ => panic!("unexpected length: {:?}", length), + } + assert!(matches!( + err, + TypeCheckError::FailingBinaryOp { op: BinaryTypeOperator::Modulo, lhs: 0, rhs: 0, .. } + )); + } else { + panic!("unexpected error: {:?}", monomorphization_error); + } +} + +#[test] +fn arithmetic_generics_checked_cast_indirect_zeros() { + let source = r#" + struct W {} + + fn foo(_x: W) -> W<(N - N) % (N - N)> { + W {} + } + + fn bar(_x: W) -> Field { + N + } + + fn main() { + let w_0: W<0> = W {}; + let w = foo(w_0); + let _ = bar(w); + } + "#; + + let errors = get_program_errors(source); + assert_eq!(errors.len(), 0); + + let monomorphization_error = get_monomorphization_error(source); + assert!(monomorphization_error.is_some()); + + // Expect a CheckedCast (0 % 0) failure + let monomorphization_error = monomorphization_error.unwrap(); + if let MonomorphizationError::UnknownArrayLength { ref length, ref err, location: _ } = + monomorphization_error + { + match length { + Type::CheckedCast { from, to } => { + assert!(matches!(*from.clone(), Type::InfixExpr { .. })); + assert!(matches!(*to.clone(), Type::InfixExpr { .. })); + } + _ => panic!("unexpected length: {:?}", length), + } + match err { + TypeCheckError::ModuloOnFields { lhs, rhs, .. } => { + assert_eq!(lhs.clone(), FieldElement::zero()); + assert_eq!(rhs.clone(), FieldElement::zero()); + } + _ => panic!("expected ModuloOnFields, but found: {:?}", err), + } + } else { + panic!("unexpected error: {:?}", monomorphization_error); + } +} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/imports.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/imports.rs index 1da7f5ce0ce..8598d8296e5 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests/imports.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/imports.rs @@ -127,3 +127,30 @@ fn warns_on_re_export_of_item_with_less_visibility() { ) )); } + +#[test] +fn errors_if_using_alias_in_import() { + let src = r#" + mod foo { + pub type bar = i32; + } + + use foo::bar::baz; + + fn main() { + } + "#; + + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::DefinitionError(DefCollectorErrorKind::PathResolutionError( + PathResolutionError::NotAModule { ident, kind }, + )) = &errors[0].0 + else { + panic!("Expected a 'not a module' error, got {:?}", errors[0].0); + }; + + assert_eq!(ident.to_string(), "bar"); + assert_eq!(*kind, "type alias"); +} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/traits.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/traits.rs index 42900d094b8..0adf5c90bea 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests/traits.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/traits.rs @@ -1,6 +1,6 @@ use crate::hir::def_collector::dc_crate::CompilationError; use crate::hir::resolution::errors::ResolverError; -use crate::tests::get_program_errors; +use crate::tests::{get_program_errors, get_program_with_maybe_parser_errors}; use super::assert_no_errors; @@ -390,3 +390,19 @@ fn trait_bounds_which_are_dependent_on_generic_types_are_resolved_correctly() { "#; assert_no_errors(src); } + +#[test] +fn does_not_crash_on_as_trait_path_with_empty_path() { + let src = r#" + struct Foo { + x: , + } + + fn main() {} + "#; + + let (_, _, errors) = get_program_with_maybe_parser_errors( + src, true, // allow parser errors + ); + assert!(!errors.is_empty()); +} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/unused_items.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/unused_items.rs index 86f77fc397a..5f9fc887b27 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests/unused_items.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/unused_items.rs @@ -294,3 +294,23 @@ fn no_warning_on_self_in_trait_impl() { "#; assert_no_errors(src); } + +#[test] +fn resolves_trait_where_clause_in_the_correct_module() { + // This is a regression test for https://github.com/noir-lang/noir/issues/6479 + let src = r#" + mod foo { + pub trait Foo {} + } + + use foo::Foo; + + pub trait Bar + where + T: Foo, + {} + + fn main() {} + "#; + assert_no_errors(src); +} diff --git a/noir/noir-repo/compiler/wasm/src/compile.rs b/noir/noir-repo/compiler/wasm/src/compile.rs index 2e178739bc5..e823f90add5 100644 --- a/noir/noir-repo/compiler/wasm/src/compile.rs +++ b/noir/noir-repo/compiler/wasm/src/compile.rs @@ -180,13 +180,13 @@ pub fn compile_program( .0; let optimized_program = nargo::ops::transform_program(compiled_program, expression_width); - // nargo::ops::check_program(&optimized_program).map_err(|errs| { - // CompileError::with_file_diagnostics( - // "Compiled program is not solvable", - // errs, - // &context.file_manager, - // ) - // })?; + nargo::ops::check_program(&optimized_program).map_err(|errs| { + CompileError::with_file_diagnostics( + "Compiled program is not solvable", + errs, + &context.file_manager, + ) + })?; let warnings = optimized_program.warnings.clone(); Ok(JsCompileProgramResult::new(optimized_program.into(), warnings)) diff --git a/noir/noir-repo/compiler/wasm/src/compile_new.rs b/noir/noir-repo/compiler/wasm/src/compile_new.rs index 3def12d9bfe..ac2f79147b3 100644 --- a/noir/noir-repo/compiler/wasm/src/compile_new.rs +++ b/noir/noir-repo/compiler/wasm/src/compile_new.rs @@ -118,13 +118,13 @@ impl CompilerContext { .0; let optimized_program = nargo::ops::transform_program(compiled_program, expression_width); - // nargo::ops::check_program(&optimized_program).map_err(|errs| { - // CompileError::with_file_diagnostics( - // "Compiled program is not solvable", - // errs, - // &self.context.file_manager, - // ) - // })?; + nargo::ops::check_program(&optimized_program).map_err(|errs| { + CompileError::with_file_diagnostics( + "Compiled program is not solvable", + errs, + &self.context.file_manager, + ) + })?; let warnings = optimized_program.warnings.clone(); Ok(JsCompileProgramResult::new(optimized_program.into(), warnings)) diff --git a/noir/noir-repo/noir_stdlib/src/ec/tecurve.nr b/noir/noir-repo/noir_stdlib/src/ec/tecurve.nr index 8512413c831..08f017c4f91 100644 --- a/noir/noir-repo/noir_stdlib/src/ec/tecurve.nr +++ b/noir/noir-repo/noir_stdlib/src/ec/tecurve.nr @@ -27,7 +27,7 @@ pub mod affine { impl Point { // Point constructor - #[deprecated = "It's recommmended to use the external noir-edwards library (https://github.com/noir-lang/noir-edwards)"] + //#[deprecated = "It's recommmended to use the external noir-edwards library (https://github.com/noir-lang/noir-edwards)"] pub fn new(x: Field, y: Field) -> Self { Self { x, y } } diff --git a/noir/noir-repo/test_programs/compile_success_empty/attribute_args/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/attribute_args/src/main.nr index 492afd9e2f1..5fc193150db 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/attribute_args/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/attribute_args/src/main.nr @@ -1,6 +1,6 @@ #[attr_with_args(1, 2)] -#[varargs(1, 2)] -#[varargs(1, 2, 3, 4)] +#[attr_with_varargs(1, 2)] +#[attr_with_varargs(1, 2, 3, 4)] pub struct Foo {} comptime fn attr_with_args(s: StructDefinition, a: Field, b: Field) { @@ -12,8 +12,8 @@ comptime fn attr_with_args(s: StructDefinition, a: Field, b: Field) { } #[varargs] -comptime fn varargs(s: StructDefinition, t: [Field]) { - let _ = s; +comptime fn attr_with_varargs(s: StructDefinition, t: [Field]) { + let _: StructDefinition = s; for _ in t {} assert(t.len() < 5); } diff --git a/noir/noir-repo/tooling/lsp/src/attribute_reference_finder.rs b/noir/noir-repo/tooling/lsp/src/attribute_reference_finder.rs index 660ee8ffdfb..e3f31b65b46 100644 --- a/noir/noir-repo/tooling/lsp/src/attribute_reference_finder.rs +++ b/noir/noir-repo/tooling/lsp/src/attribute_reference_finder.rs @@ -13,24 +13,22 @@ use noirc_frontend::{ graph::CrateId, hir::{ def_map::{CrateDefMap, LocalModuleId, ModuleId}, - resolution::{ - import::PathResolutionItem, - path_resolver::{PathResolver, StandardPathResolver}, - }, + resolution::import::resolve_import, }, - node_interner::{NodeInterner, ReferenceId}, - parser::{ParsedSubModule, Parser}, - token::CustomAttribute, + node_interner::ReferenceId, + parser::ParsedSubModule, + token::MetaAttribute, usage_tracker::UsageTracker, ParsedModule, }; +use crate::modules::module_def_id_to_reference_id; + pub(crate) struct AttributeReferenceFinder<'a> { byte_index: usize, /// The module ID in scope. This might change as we traverse the AST /// if we are analyzing something inside an inline module declaration. module_id: ModuleId, - interner: &'a NodeInterner, def_maps: &'a BTreeMap, reference_id: Option, } @@ -41,7 +39,6 @@ impl<'a> AttributeReferenceFinder<'a> { file: FileId, byte_index: usize, krate: CrateId, - interner: &'a NodeInterner, def_maps: &'a BTreeMap, ) -> Self { // Find the module the current file belongs to @@ -54,7 +51,7 @@ impl<'a> AttributeReferenceFinder<'a> { def_map.root() }; let module_id = ModuleId { krate, local_id }; - Self { byte_index, module_id, interner, def_maps, reference_id: None } + Self { byte_index, module_id, def_maps, reference_id: None } } pub(crate) fn find(&mut self, parsed_module: &ParsedModule) -> Option { @@ -88,42 +85,31 @@ impl<'a> Visitor for AttributeReferenceFinder<'a> { false } - fn visit_custom_attribute(&mut self, attribute: &CustomAttribute, _target: AttributeTarget) { - if !self.includes_span(attribute.contents_span) { - return; + fn visit_meta_attribute( + &mut self, + attribute: &MetaAttribute, + _target: AttributeTarget, + ) -> bool { + if !self.includes_span(attribute.span) { + return false; } - let name = match attribute.contents.split_once('(') { - Some((left, _right)) => left.to_string(), - None => attribute.contents.to_string(), - }; - let mut parser = Parser::for_str(&name); - let Some(path) = parser.parse_path_no_turbofish() else { - return; - }; - - let resolver = StandardPathResolver::new(self.module_id, None); - let mut usage_tracker = UsageTracker::default(); - let Ok(result) = - resolver.resolve(self.interner, self.def_maps, path, &mut usage_tracker, &mut None) - else { - return; + let path = attribute.name.clone(); + // The path here must resolve to a function and it's a simple path (can't have turbofish) + // so it can (and must) be solved as an import. + let Ok(Some((module_def_id, _, _))) = resolve_import( + path, + self.module_id, + self.def_maps, + &mut UsageTracker::default(), + None, // references tracker + ) + .map(|result| result.namespace.values) else { + return true; }; - self.reference_id = Some(path_resolution_item_to_reference_id(result.item)); - } -} + self.reference_id = Some(module_def_id_to_reference_id(module_def_id)); -fn path_resolution_item_to_reference_id(item: PathResolutionItem) -> ReferenceId { - match item { - PathResolutionItem::Module(module_id) => ReferenceId::Module(module_id), - PathResolutionItem::Struct(struct_id) => ReferenceId::Struct(struct_id), - PathResolutionItem::TypeAlias(type_alias_id) => ReferenceId::Alias(type_alias_id), - PathResolutionItem::Trait(trait_id) => ReferenceId::Trait(trait_id), - PathResolutionItem::Global(global_id) => ReferenceId::Global(global_id), - PathResolutionItem::ModuleFunction(func_id) - | PathResolutionItem::StructFunction(_, _, func_id) - | PathResolutionItem::TypeAliasFunction(_, _, func_id) - | PathResolutionItem::TraitFunction(_, _, func_id) => ReferenceId::Function(func_id), + true } } diff --git a/noir/noir-repo/tooling/lsp/src/requests/completion.rs b/noir/noir-repo/tooling/lsp/src/requests/completion.rs index 2dd42c740cd..d71f0414adc 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/completion.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/completion.rs @@ -26,20 +26,21 @@ use noirc_frontend::{ graph::{CrateId, Dependency}, hir::{ def_map::{CrateDefMap, LocalModuleId, ModuleDefId, ModuleId}, - resolution::visibility::{method_call_is_visible, struct_member_is_visible}, + resolution::visibility::{ + item_in_module_is_visible, method_call_is_visible, struct_member_is_visible, + }, }, hir_def::traits::Trait, node_interner::{NodeInterner, ReferenceId, StructId}, parser::{Item, ItemKind, ParsedSubModule}, - token::{CustomAttribute, Token, Tokens}, + token::{MetaAttribute, Token, Tokens}, Kind, ParsedModule, StructType, Type, TypeBinding, }; use sort_text::underscore_sort_text; use crate::{ requests::to_lsp_location, trait_impl_method_stub_generator::TraitImplMethodStubGenerator, - use_segment_positions::UseSegmentPositions, utils, visibility::item_in_module_is_visible, - LspState, + use_segment_positions::UseSegmentPositions, utils, LspState, }; use super::process_request; @@ -583,6 +584,14 @@ impl<'a> NodeFinder<'a> { self_prefix, ); } + Type::CheckedCast { to, .. } => { + return self.complete_type_fields_and_methods( + to, + prefix, + function_completion_kind, + self_prefix, + ); + } Type::Tuple(types) => { self.complete_tuple_fields(types, self_prefix); } @@ -799,10 +808,10 @@ impl<'a> NodeFinder<'a> { let per_ns = module_data.find_name(ident); if let Some((module_def_id, visibility, _)) = per_ns.types { if item_in_module_is_visible( - module_id, + self.def_maps, self.module_id, + module_id, visibility, - self.def_maps, ) { let completion_items = self.module_def_id_completion_items( module_def_id, @@ -820,10 +829,10 @@ impl<'a> NodeFinder<'a> { if let Some((module_def_id, visibility, _)) = per_ns.values { if item_in_module_is_visible( - module_id, + self.def_maps, self.module_id, + module_id, visibility, - self.def_maps, ) { let completion_items = self.module_def_id_completion_items( module_def_id, @@ -892,24 +901,6 @@ impl<'a> NodeFinder<'a> { None } - fn suggest_attributes(&mut self, prefix: &str, target: AttributeTarget) { - self.suggest_builtin_attributes(prefix, target); - - let function_completion_kind = FunctionCompletionKind::NameAndParameters; - let requested_items = RequestedItems::OnlyAttributeFunctions(target); - - self.complete_in_module( - self.module_id, - prefix, - PathKind::Plain, - true, - function_completion_kind, - requested_items, - ); - - self.complete_auto_imports(prefix, requested_items, function_completion_kind); - } - fn suggest_no_arguments_attributes(&mut self, prefix: &str, attributes: &[&str]) { for name in attributes { if name_matches(name, prefix) { @@ -1666,12 +1657,14 @@ impl<'a> Visitor for NodeFinder<'a> { false } - fn visit_custom_attribute(&mut self, attribute: &CustomAttribute, target: AttributeTarget) { - if self.byte_index != attribute.contents_span.end() as usize { - return; + fn visit_meta_attribute(&mut self, attribute: &MetaAttribute, target: AttributeTarget) -> bool { + if self.byte_index == attribute.name.span.end() as usize { + self.suggest_builtin_attributes(&attribute.name.to_string(), target); } - self.suggest_attributes(&attribute.contents, target); + self.find_in_path(&attribute.name, RequestedItems::OnlyAttributeFunctions(target)); + + true } fn visit_quote(&mut self, tokens: &Tokens) { diff --git a/noir/noir-repo/tooling/lsp/src/requests/completion/builtins.rs b/noir/noir-repo/tooling/lsp/src/requests/completion/builtins.rs index c2c561ced32..b4c7d8b6e01 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/completion/builtins.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/completion/builtins.rs @@ -107,6 +107,15 @@ impl<'a> NodeFinder<'a> { let one_argument_attributes = &["abi", "field", "foreign", "oracle"]; self.suggest_one_argument_attributes(prefix, one_argument_attributes); + if name_matches("deprecated", prefix) { + self.completion_items.push(snippet_completion_item( + "deprecated(\"...\")", + CompletionItemKind::METHOD, + "deprecated(\"${1:message}\")", + None, + )); + } + if name_matches("test", prefix) || name_matches("should_fail", prefix) { self.completion_items.push(snippet_completion_item( "test(should_fail)", diff --git a/noir/noir-repo/tooling/lsp/src/requests/completion/tests.rs b/noir/noir-repo/tooling/lsp/src/requests/completion/tests.rs index 745dacfc152..8cfb2a4b5ee 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/completion/tests.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/completion/tests.rs @@ -2350,13 +2350,13 @@ fn main() { #[test] async fn test_suggests_built_in_function_attribute() { let src = r#" - #[dep>|<] + #[no_pred>|<] fn foo() {} "#; assert_completion_excluding_auto_import( src, - vec![simple_completion_item("deprecated", CompletionItemKind::METHOD, None)], + vec![simple_completion_item("no_predicates", CompletionItemKind::METHOD, None)], ) .await; } diff --git a/noir/noir-repo/tooling/lsp/src/requests/goto_definition.rs b/noir/noir-repo/tooling/lsp/src/requests/goto_definition.rs index 9380209da5c..a2443ea165d 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/goto_definition.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/goto_definition.rs @@ -46,7 +46,6 @@ fn on_goto_definition_inner( file_id, byte_index, args.crate_id, - args.interner, args.def_maps, ); finder.find(&parsed_module) diff --git a/noir/noir-repo/tooling/lsp/src/requests/hover.rs b/noir/noir-repo/tooling/lsp/src/requests/hover.rs index eb066b53b78..bb1ea661719 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/hover.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/hover.rs @@ -47,7 +47,6 @@ pub(crate) fn on_hover_request( file_id, byte_index, args.crate_id, - args.interner, args.def_maps, ); finder.find(&parsed_module) @@ -680,6 +679,7 @@ impl<'a> TypeLinksGatherer<'a> { self.gather_type_links(lhs); self.gather_type_links(rhs); } + Type::CheckedCast { to, .. } => self.gather_type_links(to), Type::FieldElement | Type::Integer(..) | Type::Bool diff --git a/noir/noir-repo/tooling/lsp/src/requests/inlay_hint.rs b/noir/noir-repo/tooling/lsp/src/requests/inlay_hint.rs index f7b3e6a748d..c6415acb545 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/inlay_hint.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/inlay_hint.rs @@ -471,6 +471,7 @@ fn push_type_parts(typ: &Type, parts: &mut Vec, files: &File push_type_variable_parts(binding, parts, files); } } + Type::CheckedCast { to, .. } => push_type_parts(to, parts, files), Type::FieldElement | Type::Integer(..) diff --git a/noir/noir-repo/tooling/lsp/src/trait_impl_method_stub_generator.rs b/noir/noir-repo/tooling/lsp/src/trait_impl_method_stub_generator.rs index b433ee2ec88..2ae0d526f3e 100644 --- a/noir/noir-repo/tooling/lsp/src/trait_impl_method_stub_generator.rs +++ b/noir/noir-repo/tooling/lsp/src/trait_impl_method_stub_generator.rs @@ -368,6 +368,7 @@ impl<'a> TraitImplMethodStubGenerator<'a> { self.string.push(' '); self.append_type(right); } + Type::CheckedCast { to, .. } => self.append_type(to), Type::Constant(..) | Type::Integer(_, _) | Type::Bool diff --git a/noir/noir-repo/tooling/lsp/src/visibility.rs b/noir/noir-repo/tooling/lsp/src/visibility.rs index 6a21525f069..6724a0ba505 100644 --- a/noir/noir-repo/tooling/lsp/src/visibility.rs +++ b/noir/noir-repo/tooling/lsp/src/visibility.rs @@ -5,36 +5,13 @@ use noirc_frontend::{ graph::CrateId, hir::{ def_map::{CrateDefMap, ModuleDefId, ModuleId}, - resolution::visibility::can_reference_module_id, + resolution::visibility::item_in_module_is_visible, }, node_interner::NodeInterner, }; use crate::modules::get_parent_module; -/// Returns true if an item with the given visibility in the target module -/// is visible from the current module. For example: -/// -/// mod foo { -/// ^^^ <-- target module -/// pub(crate) fn bar() {} -/// ^^^^^^^^^^ <- visibility -/// } -pub(super) fn item_in_module_is_visible( - target_module_id: ModuleId, - current_module_id: ModuleId, - visibility: ItemVisibility, - def_maps: &BTreeMap, -) -> bool { - can_reference_module_id( - def_maps, - current_module_id.krate, - current_module_id.local_id, - target_module_id, - visibility, - ) -} - /// Returns true if the given ModuleDefId is visible from the current module, given its visibility. /// This will in turn check if the ModuleDefId parent modules are visible from the current module. /// If `defining_module` is Some, it will be considered as the parent of the item to check @@ -57,7 +34,7 @@ pub(super) fn module_def_id_is_visible( // Then check if it's visible, and upwards while let Some(module_id) = target_module_id { - if !item_in_module_is_visible(module_id, current_module_id, visibility, def_maps) { + if !item_in_module_is_visible(def_maps, current_module_id, module_id, visibility) { return false; } diff --git a/noir/noir-repo/tooling/nargo_cli/build.rs b/noir/noir-repo/tooling/nargo_cli/build.rs index 94f74a06149..438eef687b8 100644 --- a/noir/noir-repo/tooling/nargo_cli/build.rs +++ b/noir/noir-repo/tooling/nargo_cli/build.rs @@ -130,7 +130,7 @@ fn generate_execution_success_tests(test_file: &mut File, test_data_dir: &Path) &test_dir, r#" nargo.arg("execute").arg("--force"); - + nargo.assert().success();"#, ); @@ -141,7 +141,7 @@ fn generate_execution_success_tests(test_file: &mut File, test_data_dir: &Path) &test_dir, r#" nargo.arg("execute").arg("--force").arg("--force-brillig"); - + nargo.assert().success();"#, ); } @@ -169,7 +169,7 @@ fn generate_execution_failure_tests(test_file: &mut File, test_data_dir: &Path) &test_dir, r#" nargo.arg("execute").arg("--force"); - + nargo.assert().failure().stderr(predicate::str::contains("The application panicked (crashed).").not());"#, ); } @@ -196,7 +196,7 @@ fn generate_noir_test_success_tests(test_file: &mut File, test_data_dir: &Path) &test_dir, r#" nargo.arg("test"); - + nargo.assert().success();"#, ); } @@ -222,7 +222,7 @@ fn generate_noir_test_failure_tests(test_file: &mut File, test_data_dir: &Path) &test_dir, r#" nargo.arg("test"); - + nargo.assert().failure();"#, ); } @@ -273,7 +273,7 @@ fn generate_compile_success_empty_tests(test_file: &mut File, test_data_dir: &Pa &format!( r#" nargo.arg("info").arg("--json").arg("--force"); - + {assert_zero_opcodes}"#, ), ); @@ -353,7 +353,7 @@ fn generate_compile_failure_tests(test_file: &mut File, test_data_dir: &Path) { &test_name, &test_dir, r#"nargo.arg("compile").arg("--force"); - + nargo.assert().failure().stderr(predicate::str::contains("The application panicked (crashed).").not());"#, ); } diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/compile_cmd.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/compile_cmd.rs index df3a4370cb0..304988ed516 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -192,7 +192,7 @@ fn compile_programs( let target_width = get_target_width(package.expression_width, compile_options.expression_width); let program = nargo::ops::transform_program(program, target_width); - // nargo::ops::check_program(&program)?; + nargo::ops::check_program(&program)?; save_program_to_file(&program.into(), &package.name, workspace.target_directory_path()); Ok(((), warnings)) diff --git a/noir/noir-repo/tooling/nargo_fmt/src/chunks.rs b/noir/noir-repo/tooling/nargo_fmt/src/chunks.rs index 0e55dfad3b7..fcef261284d 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/chunks.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/chunks.rs @@ -915,11 +915,18 @@ impl<'a> Formatter<'a> { self.write_indentation(); } Chunk::LeadingComment(text_chunk) => { - let ends_with_newline = text_chunk.string.ends_with('\n'); + let ends_with_multiple_newlines = text_chunk.string.ends_with("\n\n"); + let ends_with_newline = + ends_with_multiple_newlines || text_chunk.string.ends_with('\n'); self.write_chunk_lines(text_chunk.string.trim()); // Respect whether the leading comment had a newline before what comes next or not - if ends_with_newline { + if ends_with_multiple_newlines { + // Remove any indentation that could exist (we'll add it later) + self.buffer.trim_spaces(); + self.write_multiple_lines_without_skipping_whitespace_and_comments(); + self.write_indentation(); + } else if ends_with_newline { self.write_line_without_skipping_whitespace_and_comments(); self.write_indentation(); } else { diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter.rs index 4ae5443a2cc..9a9386e1911 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter.rs @@ -188,8 +188,7 @@ impl<'a> Formatter<'a> { pub(crate) fn write_token(&mut self, token: Token) { self.skip_comments_and_whitespace(); if self.token == token { - self.write_current_token(); - self.bump(); + self.write_current_token_and_bump(); } else { panic!("Expected token {:?}, got: {:?}", token, self.token); } @@ -200,6 +199,12 @@ impl<'a> Formatter<'a> { self.write(&self.token.to_string()); } + /// Writes the current token and advances to the next one + pub(crate) fn write_current_token_and_bump(&mut self) { + self.write(&self.token.to_string()); + self.bump(); + } + /// Writes the current token trimming its end but doesn't advance to the next one. /// Mainly used when writing comment lines, because we never want trailing spaces /// inside comments. diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/attribute.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/attribute.rs index c13ba2a8c4c..19d5730a546 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/attribute.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/attribute.rs @@ -1,30 +1,342 @@ -use noirc_frontend::token::Token; +use noirc_frontend::token::{ + Attribute, Attributes, FunctionAttribute, MetaAttribute, SecondaryAttribute, TestScope, Token, +}; + +use crate::chunks::ChunkGroup; use super::Formatter; impl<'a> Formatter<'a> { - pub(super) fn format_attributes(&mut self) { - loop { - self.skip_comments_and_whitespace(); + pub(super) fn format_attributes(&mut self, attributes: Attributes) { + let mut all_attributes = Vec::new(); + for attribute in attributes.secondary { + all_attributes.push(Attribute::Secondary(attribute)); + } + if let Some((function_attribute, index)) = attributes.function { + all_attributes.insert(index, Attribute::Function(function_attribute)); + } + for attribute in all_attributes { + self.format_attribute(attribute); + } + } - if let Token::Attribute(_) = self.token { - self.write_indentation(); - self.write_current_token(); - self.bump(); - self.write_line(); - } else { - break; + pub(super) fn format_secondary_attributes(&mut self, attributes: Vec) { + for attribute in attributes { + self.format_secondary_attribute(attribute); + } + } + + fn format_attribute(&mut self, attribute: Attribute) { + match attribute { + Attribute::Function(function_attribute) => { + self.format_function_attribute(function_attribute); + } + Attribute::Secondary(secondary_attribute) => { + self.format_secondary_attribute(secondary_attribute); } } } - pub(super) fn format_inner_attribute(&mut self) { + fn format_function_attribute(&mut self, attribute: FunctionAttribute) { self.skip_comments_and_whitespace(); - let Token::InnerAttribute(..) = self.token else { - panic!("Expected inner attribute, got {:?}", self.token); - }; self.write_indentation(); - self.write_current_token(); - self.bump(); + + if !matches!(self.token, Token::AttributeStart { .. }) { + panic!("Expected attribute start, got: {:?}", self.token); + } + + match attribute { + FunctionAttribute::Foreign(_) + | FunctionAttribute::Builtin(_) + | FunctionAttribute::Oracle(_) => self.format_one_arg_attribute(), + FunctionAttribute::Test(test_scope) => self.format_test_attribute(test_scope), + FunctionAttribute::Fold + | FunctionAttribute::NoPredicates + | FunctionAttribute::InlineAlways => self.format_no_args_attribute(), + } + + self.write_line(); + } + + pub(super) fn format_secondary_attribute(&mut self, attribute: SecondaryAttribute) { + self.skip_comments_and_whitespace(); + self.write_indentation(); + + if !matches!(self.token, Token::AttributeStart { .. }) { + panic!("Expected attribute start, got: {:?}", self.token); + } + + match attribute { + SecondaryAttribute::Deprecated(message) => { + self.format_deprecated_attribute(message); + } + SecondaryAttribute::ContractLibraryMethod + | SecondaryAttribute::Export + | SecondaryAttribute::Varargs + | SecondaryAttribute::UseCallersScope => { + self.format_no_args_attribute(); + } + SecondaryAttribute::Field(_) + | SecondaryAttribute::Abi(_) + | SecondaryAttribute::Allow(_) => { + self.format_one_arg_attribute(); + } + SecondaryAttribute::Tag(custom_attribute) => { + self.write_and_skip_span_without_formatting(custom_attribute.span); + } + SecondaryAttribute::Meta(meta_attribute) => { + self.format_meta_attribute(meta_attribute); + } + } + + self.write_line(); + } + + fn format_deprecated_attribute(&mut self, message: Option) { + self.write_current_token_and_bump(); // #[ + self.skip_comments_and_whitespace(); + if message.is_some() { + self.write_current_token_and_bump(); // deprecated + self.write_left_paren(); // ( + self.skip_comments_and_whitespace(); // message + self.write_current_token_and_bump(); // ) + self.write_right_paren(); + } else { + self.write_current_token_and_bump(); + } + self.write_right_bracket(); // ] + } + + fn format_test_attribute(&mut self, test_scope: TestScope) { + self.write_current_token_and_bump(); // #[ + self.skip_comments_and_whitespace(); + self.write_current_token_and_bump(); // test + + match test_scope { + TestScope::None => (), + TestScope::ShouldFailWith { reason: None } => { + self.write_left_paren(); // ( + self.skip_comments_and_whitespace(); + self.write_current_token_and_bump(); // should_fail + self.write_right_paren(); // ) + } + TestScope::ShouldFailWith { reason: Some(..) } => { + self.write_left_paren(); // ( + self.skip_comments_and_whitespace(); + self.write_current_token_and_bump(); // should_fail_with + self.write_space(); + self.write_token(Token::Assign); + self.write_space(); + self.skip_comments_and_whitespace(); + self.write_current_token_and_bump(); // "reason" + self.write_right_paren(); // ) + } + } + + self.write_right_bracket(); // ] + } + + fn format_meta_attribute(&mut self, meta_attribute: MetaAttribute) { + self.write_current_token_and_bump(); // #[ + self.skip_comments_and_whitespace(); + self.format_path(meta_attribute.name); + self.skip_comments_and_whitespace(); + if self.is_at(Token::LeftParen) { + let comments_count_before_arguments = self.written_comments_count; + let has_arguments = !meta_attribute.arguments.is_empty(); + + let mut chunk_formatter = self.chunk_formatter(); + let mut group = ChunkGroup::new(); + group.text(chunk_formatter.chunk(|formatter| { + formatter.write_left_paren(); + })); + chunk_formatter.format_expressions_separated_by_comma( + meta_attribute.arguments, + false, // force trailing comma + &mut group, + ); + group.text(chunk_formatter.chunk(|formatter| { + formatter.write_right_paren(); + })); + if has_arguments || self.written_comments_count > comments_count_before_arguments { + self.format_chunk_group(group); + } + } + self.write_right_bracket(); + } + + fn format_no_args_attribute(&mut self) { + self.write_current_token_and_bump(); // #[ + self.skip_comments_and_whitespace(); + self.write_current_token_and_bump(); // name + self.write_right_bracket(); // ] + } + + fn format_one_arg_attribute(&mut self) { + self.write_current_token_and_bump(); // #[ + self.skip_comments_and_whitespace(); + self.write_current_token_and_bump(); // name + self.write_left_paren(); // ( + loop { + self.skip_comments_and_whitespace(); + if self.is_at(Token::RightParen) { + self.write_right_paren(); + break; + } else { + self.write_current_token_and_bump(); + } + } + self.write_right_bracket(); // ] + } +} + +#[cfg(test)] +mod tests { + use crate::assert_format; + + fn assert_format_attribute(src: &str, expected: &str) { + let src = format!(" {src} fn foo() {{}}"); + let expected = format!("{expected}\nfn foo() {{}}\n"); + assert_format(&src, &expected); + } + + #[test] + fn format_inner_tag_attribute() { + let src = " #!['foo] "; + let expected = "#!['foo]\n"; + assert_format(src, expected); + } + + #[test] + fn format_deprecated_attribute() { + let src = " #[ deprecated ] "; + let expected = "#[deprecated]"; + assert_format_attribute(src, expected); + } + + #[test] + fn format_deprecated_attribute_with_message() { + let src = " #[ deprecated ( \"use something else\" ) ] "; + let expected = "#[deprecated(\"use something else\")]"; + assert_format_attribute(src, expected); + } + + #[test] + fn format_contract_library_method() { + let src = " #[ contract_library_method ] "; + let expected = "#[contract_library_method]"; + assert_format_attribute(src, expected); + } + + #[test] + fn format_export() { + let src = " #[ export ] "; + let expected = "#[export]"; + assert_format_attribute(src, expected); + } + + #[test] + fn format_varargs() { + let src = " #[ varargs ] "; + let expected = "#[varargs]"; + assert_format_attribute(src, expected); + } + + #[test] + fn format_use_callers_scope() { + let src = " #[ use_callers_scope ] "; + let expected = "#[use_callers_scope]"; + assert_format_attribute(src, expected); + } + + #[test] + fn format_field_attribute() { + let src = " #[ field ( bn256 ) ] "; + let expected = "#[field(bn256)]"; + assert_format_attribute(src, expected); + } + + #[test] + fn format_abi_attribute() { + let src = " #[ abi ( foo ) ] "; + let expected = "#[abi(foo)]"; + assert_format_attribute(src, expected); + } + + #[test] + fn format_allow_attribute() { + let src = " #[ allow ( unused_vars ) ] "; + let expected = "#[allow(unused_vars)]"; + assert_format_attribute(src, expected); + } + + #[test] + fn format_meta_attribute_without_arguments() { + let src = " #[ custom ] "; + let expected = "#[custom]"; + assert_format_attribute(src, expected); + } + + #[test] + fn format_meta_attribute_without_arguments_removes_parentheses() { + let src = " #[ custom ( ) ] "; + let expected = "#[custom]"; + assert_format_attribute(src, expected); + } + + #[test] + fn format_meta_attribute_without_arguments_but_comment() { + let src = " #[ custom ( /* nothing */ ) ] "; + let expected = "#[custom( /* nothing */ )]"; + assert_format_attribute(src, expected); + } + + #[test] + fn format_meta_attribute_with_arguments() { + let src = " #[ custom ( 1 , 2, [ 3, 4 ], ) ] "; + let expected = "#[custom(1, 2, [3, 4])]"; + assert_format_attribute(src, expected); + } + + #[test] + fn format_foreign_attribute() { + let src = " #[ foreign ( foo ) ] "; + let expected = "#[foreign(foo)]"; + assert_format_attribute(src, expected); + } + + #[test] + fn format_recursive_attribute() { + let src = " #[ recursive ] "; + let expected = "#[recursive]"; + assert_format_attribute(src, expected); + } + + #[test] + fn format_test_attribute() { + let src = " #[ test ] "; + let expected = "#[test]"; + assert_format_attribute(src, expected); + } + + #[test] + fn format_test_should_fail_attribute() { + let src = " #[ test ( should_fail )] "; + let expected = "#[test(should_fail)]"; + assert_format_attribute(src, expected); + } + + #[test] + fn format_test_should_fail_with_reason_attribute() { + let src = " #[ test ( should_fail_with=\"reason\" )] "; + let expected = "#[test(should_fail_with = \"reason\")]"; + assert_format_attribute(src, expected); + } + + #[test] + fn format_multiple_function_attributes() { + let src = " #[foo] #[test] #[bar] "; + let expected = "#[foo]\n#[test]\n#[bar]"; + assert_format_attribute(src, expected); } } diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/buffer.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/buffer.rs index e4740311bf6..3e4bebef608 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/buffer.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/buffer.rs @@ -38,9 +38,10 @@ impl Buffer { } /// Trim spaces from the end of the buffer. - pub(super) fn trim_spaces(&mut self) { + pub(crate) fn trim_spaces(&mut self) { while self.buffer.ends_with(' ') { self.buffer.truncate(self.buffer.len() - 1); + self.current_line_width -= 1; } } @@ -48,6 +49,7 @@ impl Buffer { pub(super) fn trim_comma(&mut self) -> bool { if self.buffer.ends_with(',') { self.buffer.truncate(self.buffer.len() - 1); + self.current_line_width -= 1; true } else { false diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs index 547d33348b8..e20eb4291d1 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs @@ -162,8 +162,7 @@ impl<'a> Formatter<'a> { // will never write two consecutive spaces. self.write_space_without_skipping_whitespace_and_comments(); } - self.write_current_token(); - self.bump(); + self.write_current_token_and_bump(); passed_whitespace = false; last_was_block_comment = true; self.written_comments_count += 1; diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/expression.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/expression.rs index bff1799a298..0ac4c98bb95 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/expression.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/expression.rs @@ -1133,11 +1133,15 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { if count > 0 { // If newlines follow, we first add a line, then add the comment chunk group.lines(count > 1); - group.leading_comment(self.skip_comments_and_whitespace_chunk()); + group.leading_comment(self.chunk(|formatter| { + formatter.skip_comments_and_whitespace_writing_multiple_lines_if_found(); + })); ignore_next = self.ignore_next; } else { // Otherwise, add the comment first as it's a trailing comment - group.trailing_comment(self.skip_comments_and_whitespace_chunk()); + group.trailing_comment(self.chunk(|formatter| { + formatter.skip_comments_and_whitespace_writing_multiple_lines_if_found(); + })); ignore_next = self.ignore_next; group.line(); } @@ -1146,6 +1150,20 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { self.format_statement(statement, group, ignore_next); } + // See how many newlines follow the last statement + let count = self.following_newlines_count(); + + group.text(self.chunk(|formatter| { + formatter.skip_whitespace(); + })); + + // After skipping whitespace we check if there's a comment. If so, we respect + // how many lines were before that comment. + if count > 0 && matches!(self.token, Token::LineComment(..) | Token::BlockComment(..)) { + group.lines(count > 1); + } + + // Finally format the comment, if any group.text(self.chunk(|formatter| { formatter.skip_comments_and_whitespace(); })); diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/function.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/function.rs index 1dc9ead42e0..fd6977df613 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/function.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/function.rs @@ -3,13 +3,14 @@ use noirc_frontend::{ BlockExpression, FunctionReturnType, Ident, ItemVisibility, NoirFunction, Param, UnresolvedGenerics, UnresolvedTraitConstraint, Visibility, }, - token::{Keyword, Token}, + token::{Attributes, Keyword, Token}, }; use super::Formatter; use crate::chunks::{ChunkGroup, TextChunk}; pub(super) struct FunctionToFormat { + pub(super) attributes: Attributes, pub(super) visibility: ItemVisibility, pub(super) name: Ident, pub(super) generics: UnresolvedGenerics, @@ -23,6 +24,7 @@ pub(super) struct FunctionToFormat { impl<'a> Formatter<'a> { pub(super) fn format_function(&mut self, func: NoirFunction) { self.format_function_impl(FunctionToFormat { + attributes: func.def.attributes, visibility: func.def.visibility, name: func.def.name, generics: func.def.generics, @@ -37,7 +39,7 @@ impl<'a> Formatter<'a> { pub(super) fn format_function_impl(&mut self, func: FunctionToFormat) { let has_where_clause = !func.where_clause.is_empty(); - self.format_attributes(); + self.format_attributes(func.attributes); self.write_indentation(); self.format_function_modifiers(func.visibility); self.write_keyword(Keyword::Fn); @@ -278,16 +280,12 @@ impl<'a> Formatter<'a> { } pub(super) fn format_function_body(&mut self, body: BlockExpression) { - if body.is_empty() { - self.format_empty_block_contents(); - } else { - let mut group = ChunkGroup::new(); - self.chunk_formatter().format_non_empty_block_expression_contents( - body, true, // force multiple lines - &mut group, - ); - self.format_chunk_group(group); - } + let mut group = ChunkGroup::new(); + self.chunk_formatter().format_block_expression_contents( + body, true, // force multiple newlines + &mut group, + ); + self.format_chunk_group(group); } } @@ -535,4 +533,46 @@ fn baz() { let z = 3 ; "; assert_format(src, expected); } + + #[test] + fn comment_in_body_respects_newlines() { + let src = "fn foo() { + let x = 1; + + // comment + + let y = 2; +} +"; + let expected = src; + assert_format(src, expected); + } + + #[test] + fn final_comment_in_body_respects_newlines() { + let src = "fn foo() { + let x = 1; + + let y = 2; + + // comment +} +"; + let expected = src; + assert_format(src, expected); + } + + #[test] + fn initial_comment_in_body_respects_newlines() { + let src = "fn foo() { + // comment + + let x = 1; + + let y = 2; +} +"; + let expected = src; + assert_format(src, expected); + } } diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/item.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/item.rs index 77f1cd10cbc..521e476fe71 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/item.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/item.rs @@ -73,7 +73,7 @@ impl<'a> Formatter<'a> { ItemKind::Submodules(parsed_sub_module) => { self.format_submodule(parsed_sub_module); } - ItemKind::InnerAttribute(..) => self.format_inner_attribute(), + ItemKind::InnerAttribute(attribute) => self.format_secondary_attribute(attribute), } } diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/module.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/module.rs index 3df9b7e0c90..e07d22c7586 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/module.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/module.rs @@ -6,9 +6,7 @@ use super::Formatter; impl<'a> Formatter<'a> { pub(super) fn format_module_declaration(&mut self, module_declaration: ModuleDeclaration) { - if !module_declaration.outer_attributes.is_empty() { - self.format_attributes(); - } + self.format_secondary_attributes(module_declaration.outer_attributes); self.write_indentation(); self.format_item_visibility(module_declaration.visibility); self.write_keyword(Keyword::Mod); @@ -18,9 +16,7 @@ impl<'a> Formatter<'a> { } pub(super) fn format_submodule(&mut self, submodule: ParsedSubModule) { - if !submodule.outer_attributes.is_empty() { - self.format_attributes(); - } + self.format_secondary_attributes(submodule.outer_attributes); self.write_indentation(); self.format_item_visibility(submodule.visibility); if submodule.is_contract { diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/statement.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/statement.rs index 28107294909..50c286ff161 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/statement.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/statement.rs @@ -16,7 +16,15 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { group: &mut ChunkGroup, mut ignore_next: bool, ) { - group.leading_comment(self.skip_comments_and_whitespace_chunk()); + // First skip any whitespace to avoid writing multiple lines + group.text(self.chunk(|formatter| { + formatter.skip_whitespace(); + })); + + // Now write any leading comment respecting multiple newlines after them + group.leading_comment(self.chunk(|formatter| { + formatter.skip_comments_and_whitespace_writing_multiple_lines_if_found(); + })); ignore_next |= self.ignore_next; @@ -102,9 +110,7 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { let mut group = ChunkGroup::new(); group.text(self.chunk(|formatter| { - if !attributes.is_empty() { - formatter.format_attributes(); - } + formatter.format_secondary_attributes(attributes); formatter.write_keyword(keyword); formatter.write_space(); formatter.format_pattern(pattern); @@ -692,4 +698,16 @@ mod tests { "; assert_format_with_max_width(src, expected, " a_long_variable = foo(1, 2);".len() - 1); } + + #[test] + fn long_let_preceded_by_two_newlines() { + let src = "fn foo() { + let y = 0; + + let x = 123456; +} +"; + let expected = src; + assert_format_with_max_width(src, expected, " let x = 123456;".len()); + } } diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/structs.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/structs.rs index 54bfc88264d..c26ab552f30 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/structs.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/structs.rs @@ -8,9 +8,7 @@ use crate::chunks::ChunkGroup; impl<'a> Formatter<'a> { pub(super) fn format_struct(&mut self, noir_struct: NoirStruct) { - if !noir_struct.attributes.is_empty() { - self.format_attributes(); - } + self.format_secondary_attributes(noir_struct.attributes); self.write_indentation(); self.format_item_visibility(noir_struct.visibility); self.write_keyword(Keyword::Struct); diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/traits.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/traits.rs index 1379596b483..9a6b84c6537 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/traits.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/traits.rs @@ -1,15 +1,13 @@ use noirc_frontend::{ ast::{NoirTrait, Param, Pattern, TraitItem, Visibility}, - token::{Keyword, Token}, + token::{Attributes, Keyword, Token}, }; use super::{function::FunctionToFormat, Formatter}; impl<'a> Formatter<'a> { pub(super) fn format_trait(&mut self, noir_trait: NoirTrait) { - if !noir_trait.attributes.is_empty() { - self.format_attributes(); - } + self.format_secondary_attributes(noir_trait.attributes); self.write_indentation(); self.format_item_visibility(noir_trait.visibility); self.write_keyword(Keyword::Trait); @@ -91,6 +89,7 @@ impl<'a> Formatter<'a> { .collect(); let func = FunctionToFormat { + attributes: Attributes::empty(), visibility, name, generics, diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/type_expression.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/type_expression.rs index 87ba1430f10..95b0c045156 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/type_expression.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/type_expression.rs @@ -14,14 +14,12 @@ impl<'a> Formatter<'a> { match type_expr { UnresolvedTypeExpression::Variable(path) => self.format_path(path), UnresolvedTypeExpression::Constant(..) => { - self.write_current_token(); - self.bump(); + self.write_current_token_and_bump(); } UnresolvedTypeExpression::BinaryOperation(lhs, _operator, rhs, _span) => { self.format_type_expression(*lhs); self.write_space(); - self.write_current_token(); - self.bump(); + self.write_current_token_and_bump(); self.write_space(); self.format_type_expression(*rhs); } diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/types.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/types.rs index d2b5c4ab793..e52704ddaa7 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/types.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/types.rs @@ -18,8 +18,7 @@ impl<'a> Formatter<'a> { self.write_keyword(Keyword::Bool); } UnresolvedTypeData::Integer(..) | UnresolvedTypeData::FieldElement => { - self.write_current_token(); - self.bump(); + self.write_current_token_and_bump(); } UnresolvedTypeData::Array(type_expr, typ) => { self.write_left_bracket(); @@ -145,8 +144,7 @@ impl<'a> Formatter<'a> { } } UnresolvedTypeData::Quoted(..) => { - self.write_current_token(); - self.bump(); + self.write_current_token_and_bump(); } UnresolvedTypeData::AsTraitPath(as_trait_path) => { self.format_as_trait_path(*as_trait_path); diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/visibility.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/visibility.rs index d1068aa4d05..27441b977bb 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/visibility.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/visibility.rs @@ -37,8 +37,7 @@ impl<'a> Formatter<'a> { self.write_keyword(Keyword::CallData); self.write_left_paren(); self.skip_comments_and_whitespace(); - self.write_current_token(); - self.bump(); + self.write_current_token_and_bump(); self.skip_comments_and_whitespace(); self.write_right_paren(); self.write_space(); diff --git a/noir/noir-repo/tooling/nargo_fmt/tests/expected/parens.nr b/noir/noir-repo/tooling/nargo_fmt/tests/expected/parens.nr index 5ff0acf7330..2eaf3838ed6 100644 --- a/noir/noir-repo/tooling/nargo_fmt/tests/expected/parens.nr +++ b/noir/noir-repo/tooling/nargo_fmt/tests/expected/parens.nr @@ -25,6 +25,7 @@ fn main(x: u64, y: pub u64) { ( /*a*/ + ( // test 1