Skip to content

Commit

Permalink
[1 changes] feat: Sync from aztec-packages (noir-lang/noir#5387)
Browse files Browse the repository at this point in the history
feat: Implement trait dispatch in the comptime interpreter (noir-lang/noir#5376)
feat(lsp): allow function rename (noir-lang/noir#4294)
chore: use `mod.nr` files in stdlib (noir-lang/noir#5379)
fix: Replace std::HashMap with FxHashMap to fix frontend indeterminism (noir-lang/noir#5385)
fix: Remove panics in the interpreter when a builtin fails to type check (noir-lang/noir#5382)
fix: Replace expects in interpreter with errors (noir-lang/noir#5383)
  • Loading branch information
AztecBot committed Jul 2, 2024
1 parent df3b27b commit ce503f5
Show file tree
Hide file tree
Showing 54 changed files with 1,156 additions and 373 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
7b77bbfc19c51829814149e623257a3424d8e8c2
32029f91f6aae4d2f6b08b4ea40481f5837e50bc
7 changes: 7 additions & 0 deletions noir/noir-repo/Cargo.lock

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

2 changes: 1 addition & 1 deletion noir/noir-repo/acvm-repo/acvm_js/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ function run_if_available {
require_command jq
require_command cargo
require_command wasm-bindgen
#require_command wasm-opt
require_command wasm-opt

self_path=$(dirname "$(readlink -f "$0")")
pname=$(cargo read-manifest | jq -r '.name')
Expand Down
1 change: 1 addition & 0 deletions noir/noir-repo/compiler/noirc_frontend/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ regex = "1.9.1"
cfg-if = "1.0.0"
tracing.workspace = true
petgraph = "0.6"
rangemap = "1.4.0"
lalrpop-util = { version = "0.20.2", features = ["lexer"] }


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ impl<'context> Elaborator<'context> {
trait_id: trait_id.trait_id,
trait_generics: Vec::new(),
};
self.trait_constraints.push((constraint, expr_id));
self.push_trait_constraint(constraint, expr_id);
self.type_check_operator_method(expr_id, trait_id, &lhs_type, span);
}
typ
Expand Down Expand Up @@ -663,7 +663,14 @@ impl<'context> Elaborator<'context> {
}

fn elaborate_comptime_block(&mut self, block: BlockExpression, span: Span) -> (ExprId, Type) {
// We have to push a new FunctionContext so that we can resolve any constraints
// in this comptime block early before the function as a whole finishes elaborating.
// Otherwise the interpreter below may find expressions for which the underlying trait
// call is not yet solved for.
self.function_context.push(Default::default());
let (block, _typ) = self.elaborate_block_expression(block);
self.check_and_pop_function_context();

let mut interpreter =
Interpreter::new(self.interner, &mut self.comptime_scopes, self.crate_id);
let value = interpreter.evaluate_block(block);
Expand Down
103 changes: 59 additions & 44 deletions noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,16 +138,14 @@ pub struct Elaborator<'context> {

current_function: Option<FuncId>,

/// All type variables created in the current function.
/// 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>,

/// Trait constraints are collected during type checking until they are
/// verified at the end of a function. This is because constraints arise
/// on each variable, but it is only until function calls when the types
/// needed for the trait constraint may become known.
trait_constraints: Vec<(TraitConstraint, ExprId)>,
/// This is a stack of function contexts. Most of the time, for each function we
/// expect this to be of length one, containing each type variable and trait constraint
/// used in the function. This is also pushed to when a `comptime {}` block is used within
/// the function. Since it can force us to resolve that block's trait constraints earlier
/// so that they are resolved when the interpreter is run before the enclosing function
/// is finished elaborating. When this happens, we need to resolve any type variables
/// that were made within this block as well so that we can solve these traits.
function_context: Vec<FunctionContext>,

/// The current module this elaborator is in.
/// Initially empty, it is set whenever a new top-level item is resolved.
Expand All @@ -166,6 +164,20 @@ pub struct Elaborator<'context> {
unresolved_globals: BTreeMap<GlobalId, UnresolvedGlobal>,
}

#[derive(Default)]
struct FunctionContext {
/// All type variables created in the current function.
/// 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>,

/// Trait constraints are collected during type checking until they are
/// verified at the end of a function. This is because constraints arise
/// on each variable, but it is only until function calls when the types
/// needed for the trait constraint may become known.
trait_constraints: Vec<(TraitConstraint, ExprId)>,
}

impl<'context> Elaborator<'context> {
pub fn new(context: &'context mut Context, crate_id: CrateId) -> Self {
Self {
Expand All @@ -185,8 +197,7 @@ impl<'context> Elaborator<'context> {
resolving_ids: BTreeSet::new(),
trait_bounds: Vec::new(),
current_function: None,
type_variables: Vec::new(),
trait_constraints: Vec::new(),
function_context: vec![FunctionContext::default()],
current_trait_impl: None,
comptime_scopes: vec![HashMap::default()],
unresolved_globals: BTreeMap::new(),
Expand Down Expand Up @@ -326,6 +337,7 @@ impl<'context> Elaborator<'context> {
let func_meta = func_meta.clone();

self.trait_bounds = func_meta.trait_constraints.clone();
self.function_context.push(FunctionContext::default());

// Introduce all numeric generics into scope
for generic in &func_meta.all_generics {
Expand Down Expand Up @@ -367,34 +379,11 @@ impl<'context> Elaborator<'context> {
self.type_check_function_body(body_type, &func_meta, hir_func.as_expr());
}

// Default any type variables that still need defaulting.
// Default any type variables that still need defaulting and
// verify any remaining trait constraints arising from the function body.
// This is done before trait impl search since leaving them bindable can lead to errors
// when multiple impls are available. Instead we default first to choose the Field or u64 impl.
for typ in &self.type_variables {
if let Type::TypeVariable(variable, kind) = typ.follow_bindings() {
let msg = "TypeChecker should only track defaultable type vars";
variable.bind(kind.default_type().expect(msg));
}
}

// Verify any remaining trait constraints arising from the function body
for (mut constraint, expr_id) in std::mem::take(&mut self.trait_constraints) {
let span = self.interner.expr_span(&expr_id);

if matches!(&constraint.typ, Type::MutableReference(_)) {
let (_, dereferenced_typ) =
self.insert_auto_dereferences(expr_id, constraint.typ.clone());
constraint.typ = dereferenced_typ;
}

self.verify_trait_constraint(
&constraint.typ,
constraint.trait_id,
&constraint.trait_generics,
expr_id,
span,
);
}
self.check_and_pop_function_context();

// Now remove all the `where` clause constraints we added
for constraint in &func_meta.trait_constraints {
Expand All @@ -417,12 +406,42 @@ impl<'context> Elaborator<'context> {
meta.function_body = FunctionBody::Resolved;

self.trait_bounds.clear();
self.type_variables.clear();
self.interner.update_fn(id, hir_func);
self.current_function = old_function;
self.current_item = old_item;
}

/// Defaults all type variables used in this function context then solves
/// all still-unsolved trait constraints in this context.
fn check_and_pop_function_context(&mut self) {
let context = self.function_context.pop().expect("Imbalanced function_context pushes");

for typ in context.type_variables {
if let Type::TypeVariable(variable, kind) = typ.follow_bindings() {
let msg = "TypeChecker should only track defaultable type vars";
variable.bind(kind.default_type().expect(msg));
}
}

for (mut constraint, expr_id) in context.trait_constraints {
let span = self.interner.expr_span(&expr_id);

if matches!(&constraint.typ, Type::MutableReference(_)) {
let (_, dereferenced_typ) =
self.insert_auto_dereferences(expr_id, constraint.typ.clone());
constraint.typ = dereferenced_typ;
}

self.verify_trait_constraint(
&constraint.typ,
constraint.trait_id,
&constraint.trait_generics,
expr_id,
span,
);
}
}

/// This turns function parameters of the form:
/// `fn foo(x: impl Bar)`
///
Expand Down Expand Up @@ -1339,10 +1358,6 @@ impl<'context> Elaborator<'context> {
self.elaborate_comptime_global(global_id);
}

// Avoid defaulting the types of globals here since they may be used in any function.
// Otherwise we may prematurely default to a Field inside the next function if this
// global was unused there, even if it is consistently used as a u8 everywhere else.
self.type_variables.clear();
self.local_module = old_module;
self.file = old_file;
self.current_item = old_item;
Expand Down Expand Up @@ -1494,7 +1509,7 @@ impl<'context> Elaborator<'context> {
traits: BTreeMap::new(),
trait_impls: Vec::new(),
globals: Vec::new(),
impls: std::collections::HashMap::new(),
impls: rustc_hash::FxHashMap::default(),
};

items.functions = function_sets;
Expand Down
24 changes: 18 additions & 6 deletions noir/noir-repo/compiler/noirc_frontend/src/elaborator/patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::{
stmt::HirPattern,
},
macros_api::{HirExpression, Ident, Path, Pattern},
node_interner::{DefinitionId, DefinitionKind, ExprId, GlobalId, TraitImplKind},
node_interner::{DefinitionId, DefinitionKind, DependencyId, ExprId, GlobalId, TraitImplKind},
Shared, StructType, Type, TypeBindings,
};

Expand Down Expand Up @@ -396,6 +396,7 @@ impl<'context> Elaborator<'context> {
let expr = self.resolve_variable(variable);

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, generics);
self.interner.push_expr_type(id, typ.clone());
Expand All @@ -418,10 +419,14 @@ impl<'context> Elaborator<'context> {

if hir_ident.id != DefinitionId::dummy_id() {
match self.interner.definition(hir_ident.id).kind {
DefinitionKind::Function(id) => {
DefinitionKind::Function(func_id) => {
if let Some(current_item) = self.current_item {
self.interner.add_function_dependency(current_item, id);
self.interner.add_function_dependency(current_item, func_id);
}

let variable = DependencyId::Variable(hir_ident.location);
let function = DependencyId::Function(func_id);
self.interner.add_reference(function, variable);
}
DefinitionKind::Global(global_id) => {
if let Some(global) = self.unresolved_globals.remove(&global_id) {
Expand All @@ -430,6 +435,10 @@ impl<'context> Elaborator<'context> {
if let Some(current_item) = self.current_item {
self.interner.add_global_dependency(current_item, global_id);
}

let variable = DependencyId::Variable(hir_ident.location);
let global = DependencyId::Global(global_id);
self.interner.add_reference(global, variable);
}
DefinitionKind::GenericType(_) => {
// Initialize numeric generics to a polymorphic integer type in case
Expand Down Expand Up @@ -516,7 +525,7 @@ impl<'context> Elaborator<'context> {

for mut constraint in function.trait_constraints.clone() {
constraint.apply_bindings(&bindings);
self.trait_constraints.push((constraint, expr_id));
self.push_trait_constraint(constraint, expr_id);
}
}
}
Expand All @@ -533,7 +542,7 @@ impl<'context> Elaborator<'context> {
// Currently only one impl can be selected per expr_id, so this
// constraint needs to be pushed after any other constraints so
// that monomorphization can resolve this trait method to the correct impl.
self.trait_constraints.push((constraint, expr_id));
self.push_trait_constraint(constraint, expr_id);
}
}

Expand Down Expand Up @@ -575,7 +584,10 @@ impl<'context> Elaborator<'context> {
}

pub fn get_ident_from_path(&mut self, path: Path) -> (HirIdent, usize) {
let location = Location::new(path.span(), self.file);
let location = Location::new(
path.segments.last().expect("ice: path without segments").span(),
self.file,
);

let error = match path.as_ident().map(|ident| self.use_variable(ident)) {
Some(Ok(found)) => return found,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -433,8 +433,15 @@ impl<'context> Elaborator<'context> {
}

fn elaborate_comptime_statement(&mut self, statement: Statement) -> (HirStatement, Type) {
// We have to push a new FunctionContext so that we can resolve any constraints
// in this comptime block early before the function as a whole finishes elaborating.
// Otherwise the interpreter below may find expressions for which the underlying trait
// call is not yet solved for.
self.function_context.push(Default::default());
let span = statement.span;
let (hir_statement, _typ) = self.elaborate_statement(statement);
self.check_and_pop_function_context();

let mut interpreter =
Interpreter::new(self.interner, &mut self.comptime_scopes, self.crate_id);
let value = interpreter.evaluate_statement(hir_statement);
Expand Down
Loading

0 comments on commit ce503f5

Please sign in to comment.