Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Implement turbofish operator #3542

Merged
merged 41 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
04ea5d0
Add turbofish operator; add stack overflow crash
jfecher Nov 21, 2023
3a95229
Fix parsing error
jfecher Nov 22, 2023
427cb95
Add test
jfecher Nov 22, 2023
3ab58b8
Add compiler error for incorrect generic count
jfecher Nov 22, 2023
8fc453a
Edit example to have a more problematic case
jfecher Nov 22, 2023
4050fe5
Merge branch 'master' into jf/turbofish
TomAFrench Dec 11, 2023
b9673b2
resolved merge conflicts
vezenovm May 14, 2024
b289b87
fixup remaining fmt stuff
vezenovm May 14, 2024
6127baf
cleanup
vezenovm May 14, 2024
9ebb3b5
cargo fmt
vezenovm May 14, 2024
c652541
working initial turbofish tests
vezenovm May 14, 2024
9660360
cargo fmt
vezenovm May 14, 2024
4cee817
nargo fmt
vezenovm May 15, 2024
e89cde2
clippy
vezenovm May 15, 2024
a2da705
fmt
vezenovm May 15, 2024
da23029
nargo fmt
vezenovm May 15, 2024
da26bc3
fix nargo fmt for turbofish on method calls
vezenovm May 15, 2024
77dfc19
clippy
vezenovm May 15, 2024
c17e834
working turbofish with implicit generics now
vezenovm May 16, 2024
8f2be2e
separate out function and implicit generic counts
vezenovm May 16, 2024
5ad8fb4
cleanup
vezenovm May 16, 2024
e7000d7
Merge branch 'master' into jf/turbofish
vezenovm May 16, 2024
384e26a
chore: add test for specifying types on function with turbofish
TomAFrench May 17, 2024
98c5d89
chore: add test for using turbofish with generic methods
TomAFrench May 17, 2024
32d1714
chore: add turbofish to cspell
TomAFrench May 17, 2024
9ab7351
nargo fmt tests
vezenovm May 17, 2024
b73d263
fix noirc_frontend tests
vezenovm May 17, 2024
a886946
chore: update formatter test outputs
TomAFrench May 20, 2024
4125804
Revert "chore: update formatter test outputs"
TomAFrench May 20, 2024
bc7abf8
Update compiler/noirc_frontend/src/hir_def/expr.rs
vezenovm May 20, 2024
8b1bbc4
have atom_or_right_unary accept a type parser
vezenovm May 20, 2024
cd1f59e
Merge remote-tracking branch 'origin/jf/turbofish' into jf/turbofish
vezenovm May 20, 2024
bd075fa
clippy and fmt
vezenovm May 20, 2024
c6aed8c
fetch implicit generic count from node interner
vezenovm May 20, 2024
9bf4ac2
remove unused method implicit generics map
vezenovm May 20, 2024
22e3cda
add new line to expected formatter tests
vezenovm May 20, 2024
bf07ee8
move where implciit_generic_count is computed
vezenovm May 21, 2024
a9cb6c4
switch to single loop in instantiate_with
vezenovm May 21, 2024
daf151b
rename to turbofish_generics
vezenovm May 21, 2024
0d887de
Merge branch 'master' into jf/turbofish
vezenovm May 21, 2024
a44782b
Merge branch 'master' into jf/turbofish
vezenovm May 21, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion compiler/noirc_frontend/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ noirc_arena.workspace = true
noirc_errors.workspace = true
noirc_printable_type.workspace = true
fm.workspace = true
fxhash.workspace = true
iter-extended.workspace = true
chumsky.workspace = true
thiserror.workspace = true
Expand Down
8 changes: 6 additions & 2 deletions compiler/noirc_frontend/src/elaborator/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,12 @@ impl<'context> Elaborator<'context> {
// Desugar the method call into a normal, resolved function call
// so that the backend doesn't need to worry about methods
// TODO: update object_type here?
let ((function_id, function_name), function_call, _) = method_call
.into_function_call(&method_ref, object_type, location, self.interner);
let ((function_id, function_name), function_call) = method_call.into_function_call(
&method_ref,
object_type,
location,
self.interner,
);

let func_type = self.type_check_variable(function_name, function_id);

Expand Down
46 changes: 16 additions & 30 deletions compiler/noirc_frontend/src/hir/type_check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,18 +249,13 @@ impl<'interner> TypeChecker<'interner> {
}
}

// Need to borrow `generics` here as `method_call` is moved when calling `into_function_call`
// We don't save the expected generic count just yet though as `into_function_call` also accounts
// for any implicit generics such as from a generic impl.
let has_turbofish_generics = method_call.generics.is_some();
// TODO: update object_type here?
let (_, function_call, implicit_generics_count) = method_call
.into_function_call(&method_ref, object_type, location, self.interner);

if has_turbofish_generics {
self.method_call_implicit_generic_counts
.insert(function_call.func, implicit_generics_count);
}
let (_, function_call) = method_call.into_function_call(
&method_ref,
object_type,
location,
self.interner,
);

self.interner.replace_expr(expr_id, HirExpression::Call(function_call));

Expand Down Expand Up @@ -394,20 +389,10 @@ impl<'interner> TypeChecker<'interner> {
_ => 0,
});

let implicit_generic_count =
self.method_call_implicit_generic_counts.get(expr_id).copied().unwrap_or_default();

// This instantiates a trait's generics as well which need to be set
// when the constraint below is later solved for when the function is
// finished. How to link the two?
let (typ, bindings) = self.instantiate(
t,
bindings,
generics,
function_generic_count,
implicit_generic_count,
span,
);
let (typ, bindings) = self.instantiate(t, bindings, generics, function_generic_count, span);

// Push any trait constraints required by this definition to the context
// to be checked later when the type of this variable is further constrained.
Expand Down Expand Up @@ -448,24 +433,25 @@ impl<'interner> TypeChecker<'interner> {
bindings: TypeBindings,
generics: Option<Vec<Type>>,
function_generic_count: usize,
implicit_generic_count: usize,
span: Span,
) -> (Type, TypeBindings) {
match generics {
Some(generics) => {
if generics.len() != (function_generic_count + implicit_generic_count) {
// The list of `generics` represents the total number of generics on a function, including implicit generics.
// We calculate any implicit generics earlier when type checking a method call,
// In a user facing error we want to display only the number of function generics
// as this is what the user will actually be specifying.
if generics.len() != function_generic_count {
self.errors.push(TypeCheckError::IncorrectTurbofishGenericCount {
expected_count: function_generic_count,
actual_count: generics.len() - implicit_generic_count,
actual_count: generics.len(),
span,
});
typ.instantiate_with_bindings(bindings, self.interner)
} else {
typ.instantiate_with(generics)
// Fetch the count of any implicit generics on the function, such as
// for a method within a generic impl.
let implicit_generic_count = match &typ {
Type::Forall(generics, _) => generics.len() - function_generic_count,
_ => 0,
};
typ.instantiate_with(generics, self.interner, implicit_generic_count)
}
}
None => typ.instantiate_with_bindings(bindings, self.interner),
Expand Down
7 changes: 0 additions & 7 deletions compiler/noirc_frontend/src/hir/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ use crate::{
node_interner::{ExprId, FuncId, GlobalId, NodeInterner},
Type, TypeBindings,
};
use fxhash::FxHashMap as HashMap;

pub use self::errors::Source;

Expand All @@ -43,10 +42,6 @@ pub struct TypeChecker<'interner> {
/// This map is used to default any integer type variables at the end of
/// a function (before checking trait constraints) if a type wasn't already chosen.
type_variables: Vec<Type>,

/// Method calls can have implicit generics from generic impls.
/// These need to be tracked separately for accurate type checking.
method_call_implicit_generic_counts: HashMap<ExprId, usize>,
}

/// Type checks a function and assigns the
Expand Down Expand Up @@ -359,7 +354,6 @@ impl<'interner> TypeChecker<'interner> {
trait_constraints: Vec::new(),
type_variables: Vec::new(),
current_function: None,
method_call_implicit_generic_counts: HashMap::default(),
}
}

Expand All @@ -377,7 +371,6 @@ impl<'interner> TypeChecker<'interner> {
trait_constraints: Vec::new(),
type_variables: Vec::new(),
current_function: None,
method_call_implicit_generic_counts: HashMap::default(),
};
let statement = this.interner.get_global(id).let_statement;
this.check_statement(&statement);
Expand Down
27 changes: 3 additions & 24 deletions compiler/noirc_frontend/src/hir_def/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,28 +212,7 @@ impl HirMethodCallExpression {
object_type: Type,
location: Location,
interner: &mut NodeInterner,
) -> ((ExprId, HirIdent), HirCallExpression, usize) {
// Attach any generics which may not be a part of the function call's explicit generics
// This includes types such as implicit generics if the function is part of an impl
let (generics, generics_count) = if let Some(mut function_generics) = self.generics
{
let mut generics = Vec::new();
match object_type.clone() {
Type::Struct(_, mut struct_generics) => {
generics.append(&mut struct_generics);
}
_ => {
// TODO: Support other types that may contain generics such as `TraitAsType`
unreachable!("Only struct method calls are supported");
}
};
let implicit_generics_count = generics.len();
generics.append(&mut function_generics);
(Some(generics), implicit_generics_count)
} else {
(None, 0)
};

) -> ((ExprId, HirIdent), HirCallExpression) {
let mut arguments = vec![self.object];
arguments.append(&mut self.arguments);

Expand All @@ -252,10 +231,10 @@ impl HirMethodCallExpression {
}
};
let func_var = HirIdent { location, id, impl_kind };
let func = interner.push_expr(HirExpression::Ident(func_var.clone(), generics));
let func = interner.push_expr(HirExpression::Ident(func_var.clone(), self.generics));
interner.push_expr_location(func, location.span, location.file);
let expr = HirCallExpression { func, arguments, location };
((func, func_var), expr, generics_count)
((func, func_var), expr)
}
}

Expand Down
24 changes: 18 additions & 6 deletions compiler/noirc_frontend/src/hir_def/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1485,16 +1485,28 @@ impl Type {
/// is used and generic substitutions are provided manually by users.
///
/// Expects the given type vector to be the same length as the Forall type variables.
pub fn instantiate_with(&self, types: Vec<Type>) -> (Type, TypeBindings) {
pub fn instantiate_with(
&self,
types: Vec<Type>,
interner: &NodeInterner,
implicit_generic_count: usize,
) -> (Type, TypeBindings) {
match self {
Type::Forall(typevars, typ) => {
assert_eq!(types.len(), typevars.len(), "Turbofish operator used with incorrect generic count which was not caught by name resolution");
assert_eq!(types.len() + implicit_generic_count, typevars.len(), "Turbofish operator used with incorrect generic count which was not caught by name resolution");

let replacements = typevars
let mut replacements = typevars[..implicit_generic_count]
.iter()
.zip(types)
.map(|(var, binding)| (var.id(), (var.clone(), binding)))
.collect();
.map(|var| {
let new = interner.next_type_variable();
(var.id(), (var.clone(), new))
})
.collect::<HashMap<_, _>>();

// `zip` ends at the shorter iterator so we skip any implicit generics included in the `Forall` type
for (var, binding) in typevars.iter().skip(implicit_generic_count).zip(types) {
replacements.insert(var.id(), (var.clone(), binding));
}

let instantiated = typ.substitute(&replacements);
(instantiated, replacements)
Expand Down
10 changes: 7 additions & 3 deletions compiler/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1044,6 +1044,7 @@ where
expr_no_constructors,
statement,
allow_constructors,
parse_type(),
))
})
}
Expand All @@ -1064,6 +1065,7 @@ fn atom_or_right_unary<'a, P, P2, S>(
expr_no_constructors: P2,
statement: S,
allow_constructors: bool,
type_parser: impl NoirParser<UnresolvedType> + 'a,
) -> impl NoirParser<Expression> + 'a
where
P: ExprParser + 'a,
Expand All @@ -1088,12 +1090,12 @@ where

// `as Type` in `atom as Type`
let cast_rhs = keyword(Keyword::As)
.ignore_then(parse_type())
.ignore_then(type_parser.clone())
.map(UnaryRhs::Cast)
.labelled(ParsingRuleLabel::Cast);

// A turbofish operator is optional in a method call to specify generic types
let turbofish = primitives::turbofish(parse_type());
let turbofish = primitives::turbofish(type_parser);

// `.foo` or `.foo(args)` in `atom.foo` or `atom.foo(args)`
let member_rhs = just(Token::Dot)
Expand Down Expand Up @@ -1384,11 +1386,12 @@ mod test {
expression_no_constructors(expression()),
fresh_statement(),
true,
parse_type(),
),
vec!["x as u8", "x as u16", "0 as Field", "(x + 3) as [Field; 8]"],
);
parse_all_failing(
atom_or_right_unary(expression(), expression_nc, fresh_statement(), true),
atom_or_right_unary(expression(), expression_nc, fresh_statement(), true, parse_type()),
vec!["x as pub u8"],
);
}
Expand All @@ -1408,6 +1411,7 @@ mod test {
expression_no_constructors(expression()),
fresh_statement(),
true,
parse_type(),
),
valid,
);
Expand Down
48 changes: 30 additions & 18 deletions tooling/nargo_fmt/src/rewrite/expr.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use noirc_frontend::ast::{
ArrayLiteral, BlockExpression, Expression, ExpressionKind, Literal, UnaryOp,
ArrayLiteral, BlockExpression, Expression, ExpressionKind, Literal, UnaryOp, UnresolvedType,
};
use noirc_frontend::{macros_api::Span, token::Token};

Expand Down Expand Up @@ -73,6 +73,7 @@ pub(crate) fn rewrite(

let object = rewrite_sub_expr(visitor, shape, method_call_expr.object);
let method = method_call_expr.method_name.to_string();
let turbofish = rewrite_turbofish(visitor, shape, method_call_expr.generics);
let args = format_parens(
visitor.config.fn_call_width.into(),
visitor.fork(),
Expand All @@ -84,21 +85,7 @@ pub(crate) fn rewrite(
NewlineMode::IfContainsNewLineAndWidth,
);

if let Some(generics) = method_call_expr.generics {
let mut turbofish = "".to_owned();
for (i, generic) in generics.into_iter().enumerate() {
let generic = rewrite::typ(visitor, shape, generic);
turbofish = if i == 0 {
format!("<{}", generic)
} else {
format!("{turbofish}, {}", generic)
};
}
turbofish = format!("{turbofish}>");
format!("{object}.{method}::{turbofish}{args}")
} else {
format!("{object}.{method}{args}")
}
format!("{object}.{method}{turbofish}{args}")
}
ExpressionKind::MemberAccess(member_access_expr) => {
let lhs_str = rewrite_sub_expr(visitor, shape, member_access_expr.lhs);
Expand Down Expand Up @@ -172,9 +159,13 @@ pub(crate) fn rewrite(

visitor.format_if(*if_expr)
}
ExpressionKind::Lambda(_) | ExpressionKind::Variable(_, _) => {
visitor.slice(span).to_string()
ExpressionKind::Variable(path, generics) => {
let path_string = visitor.slice(path.span);

let turbofish = rewrite_turbofish(visitor, shape, generics);
format!("{path_string}{turbofish}")
}
ExpressionKind::Lambda(_) => visitor.slice(span).to_string(),
ExpressionKind::Quote(block) => format!("quote {}", rewrite_block(visitor, block, span)),
ExpressionKind::Comptime(block) => {
format!("comptime {}", rewrite_block(visitor, block, span))
Expand All @@ -188,3 +179,24 @@ fn rewrite_block(visitor: &FmtVisitor, block: BlockExpression, span: Span) -> St
visitor.visit_block(block, span);
visitor.finish()
}

fn rewrite_turbofish(
visitor: &FmtVisitor,
shape: Shape,
generics: Option<Vec<UnresolvedType>>,
) -> String {
if let Some(generics) = generics {
let mut turbofish = "".to_owned();
for (i, generic) in generics.into_iter().enumerate() {
let generic = rewrite::typ(visitor, shape, generic);
turbofish = if i == 0 {
format!("::<{}", generic)
} else {
format!("{turbofish}, {}", generic)
};
}
format!("{turbofish}>")
} else {
"".to_owned()
}
}
2 changes: 1 addition & 1 deletion tooling/nargo_fmt/tests/expected/turbofish_call.nr
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ fn foo() {
some_function(), // Original inner function call
another_function() // Original inner function call
);
}
}
2 changes: 1 addition & 1 deletion tooling/nargo_fmt/tests/expected/turbofish_method_call.nr
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ fn foo() {
)
)
);
}
}
Loading