From e3009e59fe98fad24d8284827aad1f2d4d113d89 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 30 Oct 2024 10:57:57 -0300 Subject: [PATCH 1/3] fix: distinguish TypePath with and without turbofish --- compiler/noirc_frontend/src/ast/expression.rs | 4 ++-- compiler/noirc_frontend/src/ast/statement.rs | 2 +- compiler/noirc_frontend/src/ast/visitor.rs | 4 +++- compiler/noirc_frontend/src/elaborator/patterns.rs | 8 ++++++-- compiler/noirc_frontend/src/hir/comptime/display.rs | 4 +++- .../noirc_frontend/src/parser/parser/expression.rs | 10 +++++----- .../compile_success_empty/type_path/src/main.nr | 5 +++++ tooling/nargo_fmt/src/formatter/expression.rs | 4 ++-- 8 files changed, 27 insertions(+), 14 deletions(-) diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs index 64edae8322f..67ddffe3277 100644 --- a/compiler/noirc_frontend/src/ast/expression.rs +++ b/compiler/noirc_frontend/src/ast/expression.rs @@ -802,8 +802,8 @@ impl Display for AsTraitPath { impl Display for TypePath { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "{}::{}", self.typ, self.item)?; - if !self.turbofish.is_empty() { - write!(f, "::{}", self.turbofish)?; + if let Some(turbofish) = &self.turbofish { + write!(f, "::{}", turbofish)?; } Ok(()) } diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index a25b87b7395..49f585894d9 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -408,7 +408,7 @@ pub struct AsTraitPath { pub struct TypePath { pub typ: UnresolvedType, pub item: Ident, - pub turbofish: GenericTypeArgs, + pub turbofish: Option, } // Note: Path deliberately doesn't implement Recoverable. diff --git a/compiler/noirc_frontend/src/ast/visitor.rs b/compiler/noirc_frontend/src/ast/visitor.rs index 16ee0fc4c02..9e12b29677c 100644 --- a/compiler/noirc_frontend/src/ast/visitor.rs +++ b/compiler/noirc_frontend/src/ast/visitor.rs @@ -1245,7 +1245,9 @@ impl TypePath { pub fn accept_children(&self, visitor: &mut impl Visitor) { self.typ.accept(visitor); - self.turbofish.accept(visitor); + if let Some(turbofish) = &self.turbofish { + turbofish.accept(visitor); + } } } diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index b45f455633b..32bd2f406b6 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -762,8 +762,12 @@ impl<'context> Elaborator<'context> { .func_id(self.interner) .expect("Expected trait function to be a DefinitionKind::Function"); - let generics = self.resolve_type_args(path.turbofish, func_id, span).0; - let generics = (!generics.is_empty()).then_some(generics); + let generics = if let Some(turbofish) = path.turbofish { + let generics = self.resolve_type_args(turbofish, func_id, span).0; + (!generics.is_empty()).then_some(generics) + } else { + None + }; let location = Location::new(span, self.file); let id = self.interner.function_definition_id(func_id); diff --git a/compiler/noirc_frontend/src/hir/comptime/display.rs b/compiler/noirc_frontend/src/hir/comptime/display.rs index 9f753f11e4b..bfbe39df241 100644 --- a/compiler/noirc_frontend/src/hir/comptime/display.rs +++ b/compiler/noirc_frontend/src/hir/comptime/display.rs @@ -620,7 +620,9 @@ fn remove_interned_in_expression_kind( } ExpressionKind::TypePath(mut path) => { path.typ = remove_interned_in_unresolved_type(interner, path.typ); - path.turbofish = remove_interned_in_generic_type_args(interner, path.turbofish); + path.turbofish = path + .turbofish + .map(|turbofish| remove_interned_in_generic_type_args(interner, turbofish)); ExpressionKind::TypePath(path) } ExpressionKind::Resolved(id) => { diff --git a/compiler/noirc_frontend/src/parser/parser/expression.rs b/compiler/noirc_frontend/src/parser/parser/expression.rs index 93bb4980801..949be9272fe 100644 --- a/compiler/noirc_frontend/src/parser/parser/expression.rs +++ b/compiler/noirc_frontend/src/parser/parser/expression.rs @@ -4,7 +4,7 @@ use noirc_errors::Span; use crate::{ ast::{ ArrayLiteral, BlockExpression, CallExpression, CastExpression, ConstructorExpression, - Expression, ExpressionKind, GenericTypeArgs, Ident, IfExpression, IndexExpression, Literal, + Expression, ExpressionKind, Ident, IfExpression, IndexExpression, Literal, MemberAccessExpression, MethodCallExpression, Statement, TypePath, UnaryOp, UnresolvedType, }, parser::{labels::ParsingRuleLabel, parser::parse_many::separated_by_comma, ParserErrorReason}, @@ -553,9 +553,9 @@ impl<'a> Parser<'a> { if generics.is_empty() { self.expected_token(Token::Less); } - generics + Some(generics) } else { - GenericTypeArgs::default() + None }; Some(ExpressionKind::TypePath(TypePath { typ, item, turbofish })) @@ -1587,7 +1587,7 @@ mod tests { }; assert_eq!(type_path.typ.to_string(), "Field"); assert_eq!(type_path.item.to_string(), "foo"); - assert!(type_path.turbofish.is_empty()); + assert!(type_path.turbofish.is_none()); } #[test] @@ -1599,7 +1599,7 @@ mod tests { }; assert_eq!(type_path.typ.to_string(), "Field"); assert_eq!(type_path.item.to_string(), "foo"); - assert!(!type_path.turbofish.is_empty()); + assert!(type_path.turbofish.is_some()); } #[test] diff --git a/test_programs/compile_success_empty/type_path/src/main.nr b/test_programs/compile_success_empty/type_path/src/main.nr index 92f04104868..ca9a9008f6c 100644 --- a/test_programs/compile_success_empty/type_path/src/main.nr +++ b/test_programs/compile_success_empty/type_path/src/main.nr @@ -5,6 +5,11 @@ fn main() { $foo::static() } } + + // Make sure this call works fine: in the past we used to not distinguish + // whether a TypePath had generics or not, always resolved them, filling them + // up with Type::Error, and eventually leading to an ICE. + let _ = Field::from_be_bytes([1]); } pub struct Foo {} diff --git a/tooling/nargo_fmt/src/formatter/expression.rs b/tooling/nargo_fmt/src/formatter/expression.rs index 239b52b7d04..bff1799a298 100644 --- a/tooling/nargo_fmt/src/formatter/expression.rs +++ b/tooling/nargo_fmt/src/formatter/expression.rs @@ -382,9 +382,9 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { formatter.format_type(type_path.typ); formatter.write_token(Token::DoubleColon); formatter.write_identifier(type_path.item); - if !type_path.turbofish.is_empty() { + if let Some(turbofish) = type_path.turbofish { formatter.write_token(Token::DoubleColon); - formatter.format_generic_type_args(type_path.turbofish); + formatter.format_generic_type_args(turbofish); } })); group From 42911a7f25523f31a333cb50770c7547a1dfb804 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 30 Oct 2024 11:05:12 -0300 Subject: [PATCH 2/3] Use `map`, and don't default empty generics to None --- compiler/noirc_frontend/src/elaborator/patterns.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 32bd2f406b6..8ef8ebcd87c 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -762,12 +762,8 @@ impl<'context> Elaborator<'context> { .func_id(self.interner) .expect("Expected trait function to be a DefinitionKind::Function"); - let generics = if let Some(turbofish) = path.turbofish { - let generics = self.resolve_type_args(turbofish, func_id, span).0; - (!generics.is_empty()).then_some(generics) - } else { - None - }; + let generics = + path.turbofish.map(|turbofish| self.resolve_type_args(turbofish, func_id, span).0); let location = Location::new(span, self.file); let id = self.interner.function_definition_id(func_id); From ab4c994d100587de9f1ce29db0fa21cc33776fcf Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 30 Oct 2024 11:58:23 -0300 Subject: [PATCH 3/3] use `bool::then` --- compiler/noirc_frontend/src/parser/parser/expression.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/compiler/noirc_frontend/src/parser/parser/expression.rs b/compiler/noirc_frontend/src/parser/parser/expression.rs index 949be9272fe..06f51b16842 100644 --- a/compiler/noirc_frontend/src/parser/parser/expression.rs +++ b/compiler/noirc_frontend/src/parser/parser/expression.rs @@ -548,15 +548,13 @@ impl<'a> Parser<'a> { Ident::new(String::new(), self.span_at_previous_token_end()) }; - let turbofish = if self.eat_double_colon() { + let turbofish = self.eat_double_colon().then(|| { let generics = self.parse_generic_type_args(); if generics.is_empty() { self.expected_token(Token::Less); } - Some(generics) - } else { - None - }; + generics + }); Some(ExpressionKind::TypePath(TypePath { typ, item, turbofish })) }