Skip to content

Commit

Permalink
chore: Move turbofish changes to the elaborator (noir-lang#5094)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## Summary\*

Moving noir-lang#3542,
noir-lang#5041, and
noir-lang#5087 to the elaborator

## Additional Context



## Documentation\*

Check one:
- [ ] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [ ] I have tested the changes locally.
- [ ] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: jfecher <[email protected]>
  • Loading branch information
vezenovm and jfecher authored May 23, 2024
1 parent 2b4755c commit 86fd0ac
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 18 deletions.
33 changes: 22 additions & 11 deletions compiler/noirc_frontend/src/elaborator/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ impl<'context> Elaborator<'context> {
let expr_id = self.interner.push_expr(ident);
self.interner.push_expr_location(expr_id, call_expr_span, self.file);
let ident = old_value.ident.clone();
let typ = self.type_check_variable(ident, expr_id);
let typ = self.type_check_variable(ident, expr_id, None);
self.interner.push_expr_type(expr_id, typ.clone());
capture_types.push(typ);
fmt_str_idents.push(expr_id);
Expand Down Expand Up @@ -291,16 +291,25 @@ impl<'context> Elaborator<'context> {
Some(method_ref) => {
// Automatically add `&mut` if the method expects a mutable reference and
// the object is not already one.
if let HirMethodReference::FuncId(func_id) = &method_ref {
if *func_id != FuncId::dummy_id() {
let function_type = self.interner.function_meta(func_id).typ.clone();

self.try_add_mutable_reference_to_object(
&function_type,
&mut object_type,
&mut object,
);
let func_id = match &method_ref {
HirMethodReference::FuncId(func_id) => *func_id,
HirMethodReference::TraitMethodId(method_id, _) => {
let id = self.interner.trait_method_id(*method_id);
let definition = self.interner.definition(id);
let DefinitionKind::Function(func_id) = definition.kind else {
unreachable!("Expected trait function to be a DefinitionKind::Function")
};
func_id
}
};

if func_id != FuncId::dummy_id() {
let function_type = self.interner.function_meta(&func_id).typ.clone();
self.try_add_mutable_reference_to_object(
&function_type,
&mut object_type,
&mut object,
);
}

// These arguments will be given to the desugared function call.
Expand All @@ -322,6 +331,7 @@ impl<'context> Elaborator<'context> {
let generics = method_call.generics.map(|option_inner| {
option_inner.into_iter().map(|generic| self.resolve_type(generic)).collect()
});
let turbofish_generics = generics.clone();
let method_call =
HirMethodCallExpression { method, object, arguments, location, generics };

Expand All @@ -335,7 +345,8 @@ impl<'context> Elaborator<'context> {
self.interner,
);

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

// Type check the new call now that it has been changed from a method call
// to a function call. This way we avoid duplicating code.
Expand Down
70 changes: 65 additions & 5 deletions compiler/noirc_frontend/src/elaborator/patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use rustc_hash::FxHashSet as HashSet;
use crate::{
ast::ERROR_IDENT,
hir::{
def_collector::dc_crate::CompilationError,
resolution::errors::ResolverError,
type_check::{Source, TypeCheckError},
},
Expand Down Expand Up @@ -330,9 +331,9 @@ impl<'context> Elaborator<'context> {
) -> (ExprId, Type) {
let span = variable.span;
let expr = self.resolve_variable(variable);
let id = self.interner.push_expr(HirExpression::Ident(expr.clone(), generics));
let id = self.interner.push_expr(HirExpression::Ident(expr.clone(), generics.clone()));
self.interner.push_expr_location(id, span, self.file);
let typ = self.type_check_variable(expr, id);
let typ = self.type_check_variable(expr, id, generics);
self.interner.push_expr_type(id, typ.clone());
(id, typ)
}
Expand Down Expand Up @@ -384,14 +385,19 @@ impl<'context> Elaborator<'context> {
}
}

pub(super) fn type_check_variable(&mut self, ident: HirIdent, expr_id: ExprId) -> Type {
pub(super) fn type_check_variable(
&mut self,
ident: HirIdent,
expr_id: ExprId,
generics: Option<Vec<Type>>,
) -> Type {
let mut bindings = TypeBindings::new();

// Add type bindings from any constraints that were used.
// We need to do this first since otherwise instantiating the type below
// will replace each trait generic with a fresh type variable, rather than
// the type used in the trait constraint (if it exists). See #4088.
if let ImplKind::TraitMethod(_, constraint, _) = &ident.impl_kind {
if let ImplKind::TraitMethod(_, constraint, assumed) = &ident.impl_kind {
let the_trait = self.interner.get_trait(constraint.trait_id);
assert_eq!(the_trait.generics.len(), constraint.trait_generics.len());

Expand All @@ -401,6 +407,16 @@ impl<'context> Elaborator<'context> {
bindings.insert(param.id(), (param.clone(), arg.clone()));
}
}

// If the trait impl is already assumed to exist we should add any type bindings for `Self`.
// Otherwise `self` will be replaced with a fresh type variable, which will require the user
// to specify a redundant type annotation.
if *assumed {
bindings.insert(
the_trait.self_type_typevar_id,
(the_trait.self_type_typevar.clone(), constraint.typ.clone()),
);
}
}

// An identifiers type may be forall-quantified in the case of generic functions.
Expand All @@ -409,10 +425,21 @@ impl<'context> Elaborator<'context> {
// variable to handle generic functions.
let t = self.interner.id_type_substitute_trait_as_type(ident.id);

let definition = self.interner.try_definition(ident.id);
let function_generic_count = definition.map_or(0, |definition| match &definition.kind {
DefinitionKind::Function(function) => {
self.interner.function_modifiers(function).generic_count
}
_ => 0,
});

let span = self.interner.expr_span(&expr_id);
let location = self.interner.expr_location(&expr_id);
// 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) = t.instantiate_with_bindings(bindings, self.interner);
let (typ, bindings) =
self.instantiate(t, bindings, generics, function_generic_count, span, location);

// 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 @@ -447,6 +474,39 @@ impl<'context> Elaborator<'context> {
typ
}

fn instantiate(
&mut self,
typ: Type,
bindings: TypeBindings,
turbofish_generics: Option<Vec<Type>>,
function_generic_count: usize,
span: Span,
location: Location,
) -> (Type, TypeBindings) {
match turbofish_generics {
Some(turbofish_generics) => {
if turbofish_generics.len() != function_generic_count {
let type_check_err = TypeCheckError::IncorrectTurbofishGenericCount {
expected_count: function_generic_count,
actual_count: turbofish_generics.len(),
span,
};
self.errors.push((CompilationError::TypeError(type_check_err), location.file));
typ.instantiate_with_bindings(bindings, self.interner)
} else {
// 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(turbofish_generics, self.interner, implicit_generic_count)
}
}
None => typ.instantiate_with_bindings(bindings, self.interner),
}
}

fn get_ident_from_path(&mut self, path: Path) -> (HirIdent, usize) {
let location = Location::new(path.span(), self.file);

Expand Down
3 changes: 1 addition & 2 deletions compiler/noirc_frontend/src/hir/type_check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,8 +399,6 @@ impl<'interner> TypeChecker<'interner> {
// variable to handle generic functions.
let t = self.interner.id_type_substitute_trait_as_type(ident.id);

let span = self.interner.expr_span(expr_id);

let definition = self.interner.try_definition(ident.id);
let function_generic_count = definition.map_or(0, |definition| match &definition.kind {
DefinitionKind::Function(function) => {
Expand All @@ -409,6 +407,7 @@ impl<'interner> TypeChecker<'interner> {
_ => 0,
});

let span = self.interner.expr_span(expr_id);
// 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?
Expand Down

0 comments on commit 86fd0ac

Please sign in to comment.