From 71e9bb9cff6a97a81665df65b4fdd0d9f0e323f6 Mon Sep 17 00:00:00 2001 From: Tom Date: Tue, 20 Aug 2024 18:48:10 +0100 Subject: [PATCH 1/2] chore: make spans non-optional on `UnresolvedType` --- Co-authored-by: Jake Fecher --- aztec_macros/src/transforms/functions.rs | 6 +- aztec_macros/src/transforms/note_interface.rs | 59 ++++++++++--------- aztec_macros/src/transforms/storage.rs | 2 +- aztec_macros/src/utils/ast_utils.rs | 9 +-- compiler/noirc_frontend/src/ast/expression.rs | 8 +-- compiler/noirc_frontend/src/ast/function.rs | 4 +- compiler/noirc_frontend/src/ast/mod.rs | 18 +----- compiler/noirc_frontend/src/ast/statement.rs | 6 +- compiler/noirc_frontend/src/debug/mod.rs | 6 +- compiler/noirc_frontend/src/elaborator/mod.rs | 16 +---- .../src/elaborator/trait_impls.rs | 7 +-- .../noirc_frontend/src/elaborator/types.rs | 56 ++++++++---------- .../src/hir/comptime/hir_to_display_ast.rs | 2 +- .../src/hir/comptime/interpreter/builtin.rs | 2 +- .../src/hir/type_check/errors.rs | 2 +- compiler/noirc_frontend/src/hir_def/traits.rs | 3 +- compiler/noirc_frontend/src/parser/parser.rs | 6 +- .../src/parser/parser/lambdas.rs | 10 ++-- .../noirc_frontend/src/parser/parser/types.rs | 2 +- tooling/lsp/src/requests/completion.rs | 6 +- tooling/lsp/src/requests/document_symbol.rs | 18 ++---- tooling/nargo_fmt/src/rewrite/typ.rs | 4 +- tooling/nargo_fmt/src/visitor/item.rs | 13 ++-- 23 files changed, 112 insertions(+), 153 deletions(-) diff --git a/aztec_macros/src/transforms/functions.rs b/aztec_macros/src/transforms/functions.rs index cd3fdd1fc62..dcd4fdc76f6 100644 --- a/aztec_macros/src/transforms/functions.rs +++ b/aztec_macros/src/transforms/functions.rs @@ -267,7 +267,7 @@ fn create_inputs(ty: &str) -> Param { let path_snippet = ty.to_case(Case::Snake); // e.g. private_context_inputs let type_path = chained_dep!("aztec", "context", "inputs", &path_snippet, ty); - let context_type = make_type(UnresolvedTypeData::Named(type_path, vec![], true)); + let context_type = make_type(UnresolvedTypeData::Named(type_path, Default::default(), true)); let visibility = Visibility::Private; Param { pattern: context_pattern, typ: context_type, visibility, span: Span::default() } @@ -396,7 +396,7 @@ fn serialize_to_hasher( Signedness::Unsigned, ast::IntegerBitSize::ThirtyTwo, ), - span: None, + span: Span::default(), }, hasher_name, )) @@ -595,7 +595,7 @@ fn abstract_return_values(func: &NoirFunction) -> Result>, serialize_to_hasher(&ident(return_value_name), ¤t_return_type, hasher_name) .ok_or_else(|| AztecMacroError::UnsupportedFunctionReturnType { typ: current_return_type.clone(), - span: func.return_type().span.unwrap_or_default(), + span: func.return_type().span, })?; replacement_statements.extend(serialization_statements); diff --git a/aztec_macros/src/transforms/note_interface.rs b/aztec_macros/src/transforms/note_interface.rs index 7f5c0e8b48b..46ed75620a7 100644 --- a/aztec_macros/src/transforms/note_interface.rs +++ b/aztec_macros/src/transforms/note_interface.rs @@ -62,8 +62,9 @@ pub fn generate_note_interface_impl( note_struct.name.0.contents )), })?; - let note_interface_impl_span: Option = - if empty_spans { None } else { trait_impl.object_type.span }; + let note_interface_impl_span = + if empty_spans { Span::default() } else { trait_impl.object_type.span }; + // Look for the note struct implementation, generate a default one if it doesn't exist (in order to append methods to it) let existing_impl = module.impls.iter_mut().find(|r#impl| match &r#impl.object_type.typ { UnresolvedTypeData::Named(path, _, _) => path.last_ident().eq(¬e_struct.name), @@ -94,7 +95,7 @@ pub fn generate_note_interface_impl( Ok(val.to_string()) } _ => Err(AztecMacroError::CouldNotImplementNoteInterface { - span: trait_impl.object_type.span, + span: Some(trait_impl.object_type.span), secondary_message: Some(format!( "NoteInterface must be generic over NOTE_LEN and NOTE_BYTES_LEN: {}", note_type @@ -231,7 +232,7 @@ fn generate_note_to_be_bytes( note_type: &String, byte_length: &str, serialized_length: &str, - impl_span: Option, + impl_span: Span, empty_spans: bool, ) -> Result { let function_source = format!( @@ -268,13 +269,13 @@ fn generate_note_to_be_bytes( dbg!(errors); return Err(AztecMacroError::CouldNotImplementNoteInterface { secondary_message: Some("Failed to parse Noir macro code (fn to_be_bytes). This is either a bug in the compiler or the Noir macro code".to_string()), - span: impl_span + span: Some(impl_span) }); } let mut function_ast = function_ast.into_sorted(); let mut noir_fn = function_ast.functions.remove(0); - noir_fn.def.span = impl_span.unwrap_or_default(); + noir_fn.def.span = impl_span; noir_fn.def.visibility = ItemVisibility::Public; Ok(noir_fn) } @@ -282,7 +283,7 @@ fn generate_note_to_be_bytes( fn generate_note_get_header( note_type: &String, note_header_field_name: &String, - impl_span: Option, + impl_span: Span, empty_spans: bool, ) -> Result { let function_source = format!( @@ -300,13 +301,13 @@ fn generate_note_get_header( dbg!(errors); return Err(AztecMacroError::CouldNotImplementNoteInterface { secondary_message: Some("Failed to parse Noir macro code (fn get_header). This is either a bug in the compiler or the Noir macro code".to_string()), - span: impl_span + span: Some(impl_span) }); } let mut function_ast = function_ast.into_sorted(); let mut noir_fn = function_ast.functions.remove(0); - noir_fn.def.span = impl_span.unwrap_or_default(); + noir_fn.def.span = impl_span; noir_fn.def.visibility = ItemVisibility::Public; Ok(noir_fn) } @@ -314,7 +315,7 @@ fn generate_note_get_header( fn generate_note_set_header( note_type: &String, note_header_field_name: &String, - impl_span: Option, + impl_span: Span, empty_spans: bool, ) -> Result { let function_source = format!( @@ -331,13 +332,13 @@ fn generate_note_set_header( dbg!(errors); return Err(AztecMacroError::CouldNotImplementNoteInterface { secondary_message: Some("Failed to parse Noir macro code (fn set_header). This is either a bug in the compiler or the Noir macro code".to_string()), - span: impl_span + span: Some(impl_span) }); } let mut function_ast = function_ast.into_sorted(); let mut noir_fn = function_ast.functions.remove(0); - noir_fn.def.span = impl_span.unwrap_or_default(); + noir_fn.def.span = impl_span; noir_fn.def.visibility = ItemVisibility::Public; Ok(noir_fn) } @@ -346,7 +347,7 @@ fn generate_note_set_header( // of the conversion of the characters in the note's struct name to unsigned integers. fn generate_get_note_type_id( note_type_id: u32, - impl_span: Option, + impl_span: Span, empty_spans: bool, ) -> Result { // TODO(#7165): replace {} with dep::aztec::protocol_types::abis::note_selector::compute_note_selector(\"{}\") in the function source below @@ -365,13 +366,13 @@ fn generate_get_note_type_id( dbg!(errors); return Err(AztecMacroError::CouldNotImplementNoteInterface { secondary_message: Some("Failed to parse Noir macro code (fn get_note_type_id). This is either a bug in the compiler or the Noir macro code".to_string()), - span: impl_span + span: Some(impl_span) }); } let mut function_ast = function_ast.into_sorted(); let mut noir_fn = function_ast.functions.remove(0); - noir_fn.def.span = impl_span.unwrap_or_default(); + noir_fn.def.span = impl_span; noir_fn.def.visibility = ItemVisibility::Public; Ok(noir_fn) } @@ -389,7 +390,7 @@ fn generate_note_properties_struct( note_type: &str, note_fields: &[(String, String)], note_header_field_name: &String, - impl_span: Option, + impl_span: Span, empty_spans: bool, ) -> Result { let struct_source = @@ -400,7 +401,7 @@ fn generate_note_properties_struct( dbg!(errors); return Err(AztecMacroError::CouldNotImplementNoteInterface { secondary_message: Some(format!("Failed to parse Noir macro code (struct {}Properties). This is either a bug in the compiler or the Noir macro code", note_type)), - span: impl_span + span: Some(impl_span) }); } @@ -423,7 +424,7 @@ fn generate_note_deserialize_content( note_fields: &[(String, String)], note_serialize_len: &String, note_header_field_name: &String, - impl_span: Option, + impl_span: Span, empty_spans: bool, ) -> Result { let function_source = generate_note_deserialize_content_source( @@ -438,13 +439,13 @@ fn generate_note_deserialize_content( dbg!(errors); return Err(AztecMacroError::CouldNotImplementNoteInterface { secondary_message: Some("Failed to parse Noir macro code (fn deserialize_content). This is either a bug in the compiler or the Noir macro code".to_string()), - span: impl_span + span: Some(impl_span) }); } let mut function_ast = function_ast.into_sorted(); let mut noir_fn = function_ast.functions.remove(0); - noir_fn.def.span = impl_span.unwrap_or_default(); + noir_fn.def.span = impl_span; noir_fn.def.visibility = ItemVisibility::Public; Ok(noir_fn) } @@ -461,7 +462,7 @@ fn generate_note_serialize_content( note_fields: &[(String, String)], note_serialize_len: &String, note_header_field_name: &String, - impl_span: Option, + impl_span: Span, empty_spans: bool, ) -> Result { let function_source = generate_note_serialize_content_source( @@ -476,13 +477,13 @@ fn generate_note_serialize_content( dbg!(errors); return Err(AztecMacroError::CouldNotImplementNoteInterface { secondary_message: Some("Failed to parse Noir macro code (fn serialize_content). This is either a bug in the compiler or the Noir macro code".to_string()), - span: impl_span + span: Some(impl_span) }); } let mut function_ast = function_ast.into_sorted(); let mut noir_fn = function_ast.functions.remove(0); - noir_fn.def.span = impl_span.unwrap_or_default(); + noir_fn.def.span = impl_span; noir_fn.def.visibility = ItemVisibility::Public; Ok(noir_fn) } @@ -492,7 +493,7 @@ fn generate_note_properties_fn( note_type: &str, note_fields: &[(String, String)], note_header_field_name: &String, - impl_span: Option, + impl_span: Span, empty_spans: bool, ) -> Result { let function_source = @@ -502,12 +503,12 @@ fn generate_note_properties_fn( dbg!(errors); return Err(AztecMacroError::CouldNotImplementNoteInterface { secondary_message: Some("Failed to parse Noir macro code (fn properties). This is either a bug in the compiler or the Noir macro code".to_string()), - span: impl_span + span: Some(impl_span) }); } let mut function_ast = function_ast.into_sorted(); let mut noir_fn = function_ast.functions.remove(0); - noir_fn.def.span = impl_span.unwrap_or_default(); + noir_fn.def.span = impl_span; noir_fn.def.visibility = ItemVisibility::Public; Ok(noir_fn) } @@ -519,7 +520,7 @@ fn generate_note_properties_fn( // fn generate_compute_note_hiding_point( note_type: &String, - impl_span: Option, + impl_span: Span, empty_spans: bool, ) -> Result { // TODO(#7771): update this to do only 1 MSM call @@ -541,12 +542,12 @@ fn generate_compute_note_hiding_point( dbg!(errors); return Err(AztecMacroError::CouldNotImplementNoteInterface { secondary_message: Some("Failed to parse Noir macro code (fn compute_note_hiding_point). This is either a bug in the compiler or the Noir macro code".to_string()), - span: impl_span + span: Some(impl_span) }); } let mut function_ast = function_ast.into_sorted(); let mut noir_fn = function_ast.functions.remove(0); - noir_fn.def.span = impl_span.unwrap_or_default(); + noir_fn.def.span = impl_span; noir_fn.def.visibility = ItemVisibility::Public; Ok(noir_fn) } diff --git a/aztec_macros/src/transforms/storage.rs b/aztec_macros/src/transforms/storage.rs index dacea1a95e3..ce82b4d4b6d 100644 --- a/aztec_macros/src/transforms/storage.rs +++ b/aztec_macros/src/transforms/storage.rs @@ -238,7 +238,7 @@ pub fn generate_storage_implementation( vec![generic_context_type.clone()], true, ), - span: Some(Span::default()), + span: Span::default(), }, type_span: Span::default(), generics: vec![generic_context_ident.into()], diff --git a/aztec_macros/src/utils/ast_utils.rs b/aztec_macros/src/utils/ast_utils.rs index a74ec5b777a..955e4111bb3 100644 --- a/aztec_macros/src/utils/ast_utils.rs +++ b/aztec_macros/src/utils/ast_utils.rs @@ -108,17 +108,14 @@ pub fn assignment_with_type( } pub fn return_type(path: Path) -> FunctionReturnType { - let ty = make_type(UnresolvedTypeData::Named(path, vec![], true)); + let ty = make_type(UnresolvedTypeData::Named(path, Default::default(), true)); FunctionReturnType::Ty(ty) } pub fn lambda(parameters: Vec<(Pattern, UnresolvedType)>, body: Expression) -> Expression { expression(ExpressionKind::Lambda(Box::new(Lambda { parameters, - return_type: UnresolvedType { - typ: UnresolvedTypeData::Unspecified, - span: Some(Span::default()), - }, + return_type: UnresolvedType { typ: UnresolvedTypeData::Unspecified, span: Span::default() }, body, }))) } @@ -179,7 +176,7 @@ pub fn cast(lhs: Expression, ty: UnresolvedTypeData) -> Expression { } pub fn make_type(typ: UnresolvedTypeData) -> UnresolvedType { - UnresolvedType { typ, span: Some(Span::default()) } + UnresolvedType { typ, span: Span::default() } } pub fn index_array(array: Ident, index: &str) -> Expression { diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs index 1fd1e313c44..dc07f55ee33 100644 --- a/compiler/noirc_frontend/src/ast/expression.rs +++ b/compiler/noirc_frontend/src/ast/expression.rs @@ -69,9 +69,7 @@ impl UnresolvedGeneric { pub fn span(&self) -> Span { match self { UnresolvedGeneric::Variable(ident) => ident.0.span(), - UnresolvedGeneric::Numeric { ident, typ } => { - ident.0.span().merge(typ.span.unwrap_or_default()) - } + UnresolvedGeneric::Numeric { ident, typ } => ident.0.span().merge(typ.span), UnresolvedGeneric::Resolved(_, span) => *span, } } @@ -791,7 +789,7 @@ impl FunctionDefinition { visibility: Visibility::Private, pattern: Pattern::Identifier(ident.clone()), typ: unresolved_type.clone(), - span: ident.span().merge(unresolved_type.span.unwrap()), + span: ident.span().merge(unresolved_type.span), }) .collect(); @@ -848,7 +846,7 @@ impl FunctionReturnType { pub fn get_type(&self) -> Cow { match self { FunctionReturnType::Default(span) => { - Cow::Owned(UnresolvedType { typ: UnresolvedTypeData::Unit, span: Some(*span) }) + Cow::Owned(UnresolvedType { typ: UnresolvedTypeData::Unit, span: *span }) } FunctionReturnType::Ty(typ) => Cow::Borrowed(typ), } diff --git a/compiler/noirc_frontend/src/ast/function.rs b/compiler/noirc_frontend/src/ast/function.rs index 8acc068d86a..4f55e4c2c76 100644 --- a/compiler/noirc_frontend/src/ast/function.rs +++ b/compiler/noirc_frontend/src/ast/function.rs @@ -61,9 +61,7 @@ impl NoirFunction { pub fn return_type(&self) -> UnresolvedType { match &self.def.return_type { - FunctionReturnType::Default(_) => { - UnresolvedType::without_span(UnresolvedTypeData::Unit) - } + FunctionReturnType::Default(span) => UnresolvedTypeData::Unit.with_span(*span), FunctionReturnType::Ty(ty) => ty.clone(), } } diff --git a/compiler/noirc_frontend/src/ast/mod.rs b/compiler/noirc_frontend/src/ast/mod.rs index 7b5b64e7089..781630571ef 100644 --- a/compiler/noirc_frontend/src/ast/mod.rs +++ b/compiler/noirc_frontend/src/ast/mod.rs @@ -148,11 +148,7 @@ pub enum UnresolvedTypeData { #[derive(Debug, PartialEq, Eq, Clone, Hash)] pub struct UnresolvedType { pub typ: UnresolvedTypeData, - - // The span is None in the cases where the User omitted a type: - // fn Foo() {} --- return type is UnresolvedType::Unit without a span - // let x = 100; --- type is UnresolvedType::Unspecified without a span - pub span: Option, + pub span: Span, } /// Type wrapper for a member access @@ -184,7 +180,7 @@ pub enum UnresolvedTypeExpression { impl Recoverable for UnresolvedType { fn error(span: Span) -> Self { - UnresolvedType { typ: UnresolvedTypeData::Error, span: Some(span) } + UnresolvedType { typ: UnresolvedTypeData::Error, span } } } @@ -280,14 +276,6 @@ impl UnresolvedType { } } - pub fn without_span(typ: UnresolvedTypeData) -> UnresolvedType { - UnresolvedType { typ, span: None } - } - - pub fn unspecified() -> UnresolvedType { - UnresolvedType { typ: UnresolvedTypeData::Unspecified, span: None } - } - pub(crate) fn is_type_expression(&self) -> bool { matches!(&self.typ, UnresolvedTypeData::Expression(_)) } @@ -309,7 +297,7 @@ impl UnresolvedTypeData { } pub fn with_span(&self, span: Span) -> UnresolvedType { - UnresolvedType { typ: self.clone(), span: Some(span) } + UnresolvedType { typ: self.clone(), span } } } diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index 10ae7a1e62f..18033197079 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -12,7 +12,7 @@ use super::{ }; use crate::elaborator::types::SELF_TYPE_NAME; use crate::lexer::token::SpannedToken; -use crate::macros_api::SecondaryAttribute; +use crate::macros_api::{SecondaryAttribute, UnresolvedTypeData}; use crate::parser::{ParserError, ParserErrorReason}; use crate::token::Token; @@ -672,7 +672,7 @@ impl ForRange { let let_array = Statement { kind: StatementKind::Let(LetStatement { pattern: Pattern::Identifier(array_ident.clone()), - r#type: UnresolvedType::unspecified(), + r#type: UnresolvedTypeData::Unspecified.with_span(Default::default()), expression: array, comptime: false, attributes: vec![], @@ -718,7 +718,7 @@ impl ForRange { let let_elem = Statement { kind: StatementKind::Let(LetStatement { pattern: Pattern::Identifier(identifier), - r#type: UnresolvedType::unspecified(), + r#type: UnresolvedTypeData::Unspecified.with_span(Default::default()), expression: Expression::new(loop_element, array_span), comptime: false, attributes: vec![], diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index 2856bb55276..935acc4e6d0 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -144,7 +144,7 @@ impl DebugInstrumenter { let save_ret_expr = ast::Statement { kind: ast::StatementKind::Let(ast::LetStatement { pattern: ast::Pattern::Identifier(ident("__debug_expr", ret_expr.span)), - r#type: ast::UnresolvedType::unspecified(), + r#type: ast::UnresolvedTypeData::Unspecified.with_span(Default::default()), expression: ret_expr.clone(), comptime: false, attributes: vec![], @@ -244,7 +244,7 @@ impl DebugInstrumenter { ast::Statement { kind: ast::StatementKind::Let(ast::LetStatement { pattern: ast::Pattern::Tuple(vars_pattern, let_stmt.pattern.span()), - r#type: ast::UnresolvedType::unspecified(), + r#type: ast::UnresolvedTypeData::Unspecified.with_span(Default::default()), comptime: false, expression: ast::Expression { kind: ast::ExpressionKind::Block(ast::BlockExpression { @@ -276,7 +276,7 @@ impl DebugInstrumenter { let let_kind = ast::StatementKind::Let(ast::LetStatement { pattern: ast::Pattern::Identifier(ident("__debug_expr", assign_stmt.expression.span)), - r#type: ast::UnresolvedType::unspecified(), + r#type: ast::UnresolvedTypeData::Unspecified.with_span(Default::default()), expression: assign_stmt.expression.clone(), comptime: false, attributes: vec![], diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 2e4a69c067f..bba87f9a934 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -774,8 +774,7 @@ impl<'context> Elaborator<'context> { lints::unnecessary_pub_argument(func, visibility, is_pub_allowed).map(Into::into) }); - let type_span = typ.span.unwrap_or_else(|| pattern.span()); - + let type_span = typ.span; let typ = match typ.typ { UnresolvedTypeData::TraitAsType(path, args) => { self.desugar_impl_trait_arg(path, args, &mut generics, &mut trait_constraints) @@ -1022,7 +1021,7 @@ impl<'context> Elaborator<'context> { let self_type_span = trait_impl.object_type.span; if matches!(self_type, Type::MutableReference(_)) { - let span = self_type_span.unwrap_or_else(|| trait_impl.trait_path.span()); + let span = self_type_span; self.push_err(DefCollectorErrorKind::MutableReferenceInTraitImpl { span }); } @@ -1038,15 +1037,6 @@ impl<'context> Elaborator<'context> { self.collect_trait_impl_methods(trait_id, trait_impl, &where_clause); let span = trait_impl.object_type.span; - - let span = if let Some(span) = span { - span - } else if self.interner.is_in_lsp_mode() { - // A span might not be set if this was generated by a macro - Default::default() - } else { - span.expect("All trait self types should have spans") - }; self.declare_methods_on_struct(true, &mut trait_impl.methods, span); let methods = trait_impl.methods.function_ids(); @@ -1078,7 +1068,7 @@ impl<'context> Elaborator<'context> { ) { self.push_err(DefCollectorErrorKind::OverlappingImpl { typ: self_type.clone(), - span: self_type_span.unwrap_or_else(|| trait_impl.trait_path.span()), + span: self_type_span, }); // The 'previous impl defined here' note must be a separate error currently diff --git a/compiler/noirc_frontend/src/elaborator/trait_impls.rs b/compiler/noirc_frontend/src/elaborator/trait_impls.rs index 853ee6389fd..20719b9f090 100644 --- a/compiler/noirc_frontend/src/elaborator/trait_impls.rs +++ b/compiler/noirc_frontend/src/elaborator/trait_impls.rs @@ -72,10 +72,7 @@ impl<'context> Elaborator<'context> { self.push_err(DefCollectorErrorKind::TraitMissingMethod { trait_name: self.interner.get_trait(trait_id).name.clone(), method_name: method.name.clone(), - trait_impl_span: trait_impl - .object_type - .span - .expect("type must have a span"), + trait_impl_span: trait_impl.object_type.span, }); } } else { @@ -221,7 +218,7 @@ impl<'context> Elaborator<'context> { let the_trait = self.interner.get_trait(trait_id); if self.crate_id != the_trait.crate_id && self.crate_id != object_crate { self.push_err(DefCollectorErrorKind::TraitImplOrphaned { - span: trait_impl.object_type.span.expect("object type must have a span"), + span: trait_impl.object_type.span, }); } } diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 80c5aad7fd2..a863a3e4e9b 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -45,9 +45,7 @@ impl<'context> Elaborator<'context> { let span = typ.span; let resolved_type = self.resolve_type_inner(typ, &Kind::Normal); if resolved_type.is_nested_slice() { - self.push_err(ResolverError::NestedSlices { - span: span.expect("Type should have span"), - }); + self.push_err(ResolverError::NestedSlices { span }); } resolved_type } @@ -117,7 +115,11 @@ impl<'context> Elaborator<'context> { } Quoted(quoted) => Type::Quoted(quoted), Unit => Type::Unit, - Unspecified => Type::Error, + Unspecified => { + let span = typ.span; + self.push_err(TypeCheckError::TypeAnnotationsNeeded { span }); + Type::Error + } Error => Type::Error, Named(path, args, _) => self.resolve_named_type(path, args), TraitAsType(path, args) => self.resolve_trait_as_type(path, args), @@ -128,12 +130,7 @@ impl<'context> Elaborator<'context> { Function(args, ret, env, unconstrained) => { let args = vecmap(args, |arg| self.resolve_type_inner(arg, kind)); let ret = Box::new(self.resolve_type_inner(*ret, kind)); - - // expect() here is valid, because the only places we don't have a span are omitted types - // e.g. a function without return type implicitly has a spanless UnresolvedType::Unit return type - // To get an invalid env type, the user must explicitly specify the type, which will have a span - let env_span = - env.span.expect("Unexpected missing span for closure environment type"); + let env_span = env.span; let env = Box::new(self.resolve_type_inner(*env, kind)); @@ -158,27 +155,26 @@ impl<'context> Elaborator<'context> { AsTraitPath(_) => todo!("Resolve AsTraitPath"), }; - if let Some(unresolved_span) = typ.span { - let location = Location::new(named_path_span.unwrap_or(unresolved_span), self.file); + let unresolved_span = typ.span; + let location = Location::new(named_path_span.unwrap_or(unresolved_span), self.file); - match resolved_type { - Type::Struct(ref struct_type, _) => { - // Record the location of the type reference - self.interner.push_type_ref_location(resolved_type.clone(), location); + match resolved_type { + Type::Struct(ref struct_type, _) => { + // Record the location of the type reference + self.interner.push_type_ref_location(resolved_type.clone(), location); - if !is_synthetic { - self.interner.add_struct_reference( - struct_type.borrow().id, - location, - is_self_type_name, - ); - } - } - Type::Alias(ref alias_type, _) => { - self.interner.add_alias_reference(alias_type.borrow().id, location); + if !is_synthetic { + self.interner.add_struct_reference( + struct_type.borrow().id, + location, + is_self_type_name, + ); } - _ => (), } + Type::Alias(ref alias_type, _) => { + self.interner.add_alias_reference(alias_type.borrow().id, location); + } + _ => (), } // Check that any types with a type kind match the expected type kind supplied to this function @@ -196,10 +192,8 @@ impl<'context> Elaborator<'context> { // } if let Type::NamedGeneric(_, name, resolved_kind) = &resolved_type { if matches!(resolved_kind, Kind::Numeric { .. }) && matches!(kind, Kind::Normal) { - let expected_typ_err = ResolverError::NumericGenericUsedForType { - name: name.to_string(), - span: span.expect("Type should have span"), - }; + let expected_typ_err = + ResolverError::NumericGenericUsedForType { name: name.to_string(), span }; self.push_err(expected_typ_err); return Type::Error; } diff --git a/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs b/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs index 853697220b3..07c5c1a0c77 100644 --- a/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs +++ b/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs @@ -370,7 +370,7 @@ impl Type { } }; - UnresolvedType { typ, span: None } + UnresolvedType { typ, span: Span::default() } } /// Convert to AST for display (some details lost) diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index f65c5ae8cdf..6ef4aee7531 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -1219,7 +1219,7 @@ fn function_def_set_return_type( mutate_func_meta_type(interpreter.elaborator.interner, func_id, |func_meta| { func_meta.return_type = FunctionReturnType::Ty(UnresolvedType { typ: UnresolvedTypeData::Resolved(quoted_type_id), - span: Some(location.span), + span: location.span, }); replace_func_meta_return_type(&mut func_meta.typ, return_type); }); diff --git a/compiler/noirc_frontend/src/hir/type_check/errors.rs b/compiler/noirc_frontend/src/hir/type_check/errors.rs index d51760f9d20..de5d146713f 100644 --- a/compiler/noirc_frontend/src/hir/type_check/errors.rs +++ b/compiler/noirc_frontend/src/hir/type_check/errors.rs @@ -298,7 +298,7 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic { Source::Return(ret_ty, expr_span) => { let ret_ty_span = match ret_ty.clone() { FunctionReturnType::Default(span) => span, - FunctionReturnType::Ty(ty) => ty.span.unwrap(), + FunctionReturnType::Ty(ty) => ty.span, }; let mut diagnostic = Diagnostic::simple_error(format!("expected type {expected}, found type {actual}"), format!("expected {expected} because of return type"), ret_ty_span); diff --git a/compiler/noirc_frontend/src/hir_def/traits.rs b/compiler/noirc_frontend/src/hir_def/traits.rs index df6ccc0492c..9d820b9443c 100644 --- a/compiler/noirc_frontend/src/hir_def/traits.rs +++ b/compiler/noirc_frontend/src/hir_def/traits.rs @@ -1,10 +1,11 @@ use rustc_hash::FxHashMap as HashMap; use crate::ast::{Ident, NoirFunction}; +use crate::TypeVariableId; use crate::{ graph::CrateId, node_interner::{FuncId, TraitId, TraitMethodId}, - Generics, Type, TypeBindings, TypeVariable, TypeVariableId, + Generics, Type, TypeBindings, TypeVariable, }; use fm::FileId; use noirc_errors::{Location, Span}; diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index eddf85becfe..b86c2c46c9b 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -422,9 +422,9 @@ fn check_statements_require_semicolon( /// Parse an optional ': type' fn optional_type_annotation<'a>() -> impl NoirParser + 'a { - ignore_then_commit(just(Token::Colon), parse_type()) - .or_not() - .map(|r#type| r#type.unwrap_or_else(UnresolvedType::unspecified)) + ignore_then_commit(just(Token::Colon), parse_type()).or_not().map_with_span(|r#type, span| { + r#type.unwrap_or(UnresolvedTypeData::Unspecified.with_span(span)) + }) } fn module_declaration() -> impl NoirParser { diff --git a/compiler/noirc_frontend/src/parser/parser/lambdas.rs b/compiler/noirc_frontend/src/parser/parser/lambdas.rs index 2b4a1d547c0..5ef0b918375 100644 --- a/compiler/noirc_frontend/src/parser/parser/lambdas.rs +++ b/compiler/noirc_frontend/src/parser/parser/lambdas.rs @@ -2,6 +2,7 @@ use chumsky::{primitive::just, Parser}; use super::{parse_type, pattern}; use crate::ast::{Expression, ExpressionKind, Lambda, Pattern, UnresolvedType}; +use crate::macros_api::UnresolvedTypeData; use crate::{ parser::{labels::ParsingRuleLabel, parameter_name_recovery, parameter_recovery, NoirParser}, token::Token, @@ -23,9 +24,10 @@ fn lambda_parameters() -> impl NoirParser> { let typ = parse_type().recover_via(parameter_recovery()); let typ = just(Token::Colon).ignore_then(typ); - let parameter = pattern() - .recover_via(parameter_name_recovery()) - .then(typ.or_not().map(|typ| typ.unwrap_or_else(UnresolvedType::unspecified))); + let parameter = + pattern().recover_via(parameter_name_recovery()).then(typ.or_not().map_with_span( + |typ, span| typ.unwrap_or(UnresolvedTypeData::Unspecified.with_span(span)), + )); parameter .separated_by(just(Token::Comma)) @@ -37,5 +39,5 @@ fn lambda_return_type() -> impl NoirParser { just(Token::Arrow) .ignore_then(parse_type()) .or_not() - .map(|ret| ret.unwrap_or_else(UnresolvedType::unspecified)) + .map_with_span(|ret, span| ret.unwrap_or(UnresolvedTypeData::Unspecified.with_span(span))) } diff --git a/compiler/noirc_frontend/src/parser/parser/types.rs b/compiler/noirc_frontend/src/parser/parser/types.rs index 3db2f24aa58..cb7271a416c 100644 --- a/compiler/noirc_frontend/src/parser/parser/types.rs +++ b/compiler/noirc_frontend/src/parser/parser/types.rs @@ -65,7 +65,7 @@ pub(super) fn parenthesized_type( .delimited_by(just(Token::LeftParen), just(Token::RightParen)) .map_with_span(|typ, span| UnresolvedType { typ: UnresolvedTypeData::Parenthesized(Box::new(typ)), - span: span.into(), + span, }) } diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 302152f0829..28388230e94 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -612,10 +612,8 @@ impl<'a> NodeFinder<'a> { } fn find_in_unresolved_type(&mut self, unresolved_type: &UnresolvedType) { - if let Some(span) = unresolved_type.span { - if !self.includes_span(span) { - return; - } + if !self.includes_span(unresolved_type.span) { + return; } match &unresolved_type.typ { diff --git a/tooling/lsp/src/requests/document_symbol.rs b/tooling/lsp/src/requests/document_symbol.rs index 20fdfb6ece7..5d2635b3549 100644 --- a/tooling/lsp/src/requests/document_symbol.rs +++ b/tooling/lsp/src/requests/document_symbol.rs @@ -140,11 +140,7 @@ impl<'a> DocumentSymbolCollector<'a> { let mut children = Vec::new(); for (field_name, typ) in &noir_struct.fields { - let span = if let Some(typ) = typ.span { - Span::from(field_name.span().start()..typ.end()) - } else { - field_name.span() - }; + let span = Span::from(field_name.span().start()..typ.span.end()); let Some(field_location) = self.to_lsp_location(span) else { continue; @@ -238,9 +234,7 @@ impl<'a> DocumentSymbolCollector<'a> { span = Span::from(span.start()..return_type_span.end()); } FunctionReturnType::Ty(typ) => { - if let Some(type_span) = typ.span { - span = Span::from(span.start()..type_span.end()); - } + span = Span::from(span.start()..typ.span.end()); } } @@ -290,9 +284,7 @@ impl<'a> DocumentSymbolCollector<'a> { let mut span = name.span(); // If there's a type span, extend the span to include it - if let Some(type_span) = typ.span { - span = Span::from(span.start()..type_span.end()); - } + span = Span::from(span.start()..typ.span.end()); // If there's a default value, extend the span to include it if let Some(default_value) = default_value { @@ -326,8 +318,8 @@ impl<'a> DocumentSymbolCollector<'a> { return; }; - let span = if let Some(type_span) = typ.and_then(|typ| typ.span) { - Span::from(name.span().start()..type_span.end()) + let span = if let Some(typ) = typ { + Span::from(name.span().start()..typ.span.end()) } else { name.span() }; diff --git a/tooling/nargo_fmt/src/rewrite/typ.rs b/tooling/nargo_fmt/src/rewrite/typ.rs index 86cc618eb33..8d1e27078a8 100644 --- a/tooling/nargo_fmt/src/rewrite/typ.rs +++ b/tooling/nargo_fmt/src/rewrite/typ.rs @@ -40,7 +40,7 @@ pub(crate) fn rewrite(visitor: &FmtVisitor, _shape: Shape, typ: UnresolvedType) UnresolvedTypeData::Function(args, return_type, env, unconstrained) => { let unconstrained = if unconstrained { "unconstrained " } else { "" }; - let env = if span_is_empty(env.span.unwrap()) { + let env = if span_is_empty(env.span) { "".into() } else { let ty = rewrite(visitor, _shape, *env); @@ -72,7 +72,7 @@ pub(crate) fn rewrite(visitor: &FmtVisitor, _shape: Shape, typ: UnresolvedType) | UnresolvedTypeData::String(_) | UnresolvedTypeData::FormatString(_, _) | UnresolvedTypeData::Quoted(_) - | UnresolvedTypeData::TraitAsType(_, _) => visitor.slice(typ.span.unwrap()).into(), + | UnresolvedTypeData::TraitAsType(_, _) => visitor.slice(typ.span).into(), UnresolvedTypeData::Error => unreachable!(), } } diff --git a/tooling/nargo_fmt/src/visitor/item.rs b/tooling/nargo_fmt/src/visitor/item.rs index 0c9f61a7d40..94a32449ebe 100644 --- a/tooling/nargo_fmt/src/visitor/item.rs +++ b/tooling/nargo_fmt/src/visitor/item.rs @@ -6,7 +6,10 @@ use crate::{ }, visitor::expr::{format_seq, NewlineMode}, }; -use noirc_frontend::ast::{NoirFunction, Visibility}; +use noirc_frontend::{ + ast::{NoirFunction, Visibility}, + macros_api::UnresolvedTypeData, +}; use noirc_frontend::{ hir::resolution::errors::Span, parser::{Item, ItemKind}, @@ -108,14 +111,16 @@ impl super::FmtVisitor<'_> { fn format_return_type( &self, - return_type_span: Option, + span: Span, func: &NoirFunction, func_span: Span, params_end: u32, ) -> String { let mut result = String::new(); - if let Some(span) = return_type_span { + if func.return_type().typ == UnresolvedTypeData::Unit { + result.push_str(self.slice(params_end..func_span.start())); + } else { result.push_str(" -> "); let visibility = match func.def.return_visibility { @@ -135,8 +140,6 @@ impl super::FmtVisitor<'_> { if !slice.trim().is_empty() { result.push_str(slice); } - } else { - result.push_str(self.slice(params_end..func_span.start())); } result From 811b93fb9eb2cd90004f0b57e7d575447c5e0808 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Tue, 20 Aug 2024 19:10:48 +0100 Subject: [PATCH 2/2] Update compiler/noirc_frontend/src/elaborator/types.rs --- compiler/noirc_frontend/src/elaborator/types.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index a863a3e4e9b..f74d7449cd3 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -115,11 +115,7 @@ impl<'context> Elaborator<'context> { } Quoted(quoted) => Type::Quoted(quoted), Unit => Type::Unit, - Unspecified => { - let span = typ.span; - self.push_err(TypeCheckError::TypeAnnotationsNeeded { span }); - Type::Error - } + Unspecified => Type::Error, Error => Type::Error, Named(path, args, _) => self.resolve_named_type(path, args), TraitAsType(path, args) => self.resolve_trait_as_type(path, args),