Skip to content

Commit

Permalink
fix: NoMatchingImplFound in comptime code only (#5617)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves #5615

## Summary\*

Fixes a difference in how the interpreter and monomorphizer handle
generics - further explained in the linked issue.

## Additional Context

To fix this I saved the generics on each link in the call stack and
restored them after the function finishes. This wasn't good enough
however - doing this too early mean losing intermediate bindings in
`instantiation_bindings`. So I needed to add some `follow_bindings`
there as well. The `impl_bindings` all bind the trait impl to the trait
itself so follow_bindings there is unneeded.

## Documentation\*

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

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
jfecher authored Jul 29, 2024
1 parent 1cddf42 commit 28211a3
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 3 deletions.
64 changes: 61 additions & 3 deletions compiler/noirc_frontend/src/hir/comptime/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use crate::monomorphization::{
undo_instantiation_bindings,
};
use crate::token::Tokens;
use crate::TypeVariable;
use crate::{
hir_def::{
expr::{
Expand Down Expand Up @@ -53,6 +54,12 @@ pub struct Interpreter<'local, 'interner> {
in_loop: bool,

current_function: Option<FuncId>,

/// Maps each bound generic to each binding it has in the current callstack.
/// Since the interpreter monomorphizes as it interprets, we can bind over the same generic
/// multiple times. Without this map, when one of these inner functions exits we would
/// unbind the generic completely instead of resetting it to its previous binding.
bound_generics: Vec<HashMap<TypeVariable, Type>>,
}

#[allow(unused)]
Expand All @@ -62,26 +69,41 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
crate_id: CrateId,
current_function: Option<FuncId>,
) -> Self {
Self { elaborator, crate_id, current_function, in_loop: false }
let bound_generics = Vec::new();
Self { elaborator, crate_id, current_function, bound_generics, in_loop: false }
}

pub(crate) fn call_function(
&mut self,
function: FuncId,
arguments: Vec<(Value, Location)>,
instantiation_bindings: TypeBindings,
mut instantiation_bindings: TypeBindings,
location: Location,
) -> IResult<Value> {
let trait_method = self.elaborator.interner.get_trait_method_id(function);

// To match the monomorphizer, we need to call follow_bindings on each of
// the instantiation bindings before we unbind the generics from the previous function.
// This is because the instantiation bindings refer to variables from the call site.
for (_, binding) in instantiation_bindings.values_mut() {
*binding = binding.follow_bindings();
}

self.unbind_generics_from_previous_function();
perform_instantiation_bindings(&instantiation_bindings);
let impl_bindings =
let mut impl_bindings =
perform_impl_bindings(self.elaborator.interner, trait_method, function, location)?;

for (_, binding) in impl_bindings.values_mut() {
*binding = binding.follow_bindings();
}

self.remember_bindings(&instantiation_bindings, &impl_bindings);
let result = self.call_function_inner(function, arguments, location);

undo_instantiation_bindings(impl_bindings);
undo_instantiation_bindings(instantiation_bindings);
self.rebind_generics_from_previous_function();
result
}

Expand Down Expand Up @@ -250,6 +272,42 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
self.elaborator.comptime_scopes.last_mut().unwrap()
}

fn unbind_generics_from_previous_function(&mut self) {
if let Some(bindings) = self.bound_generics.last() {
for var in bindings.keys() {
var.unbind(var.id());
}
}
// Push a new bindings list for the current function
self.bound_generics.push(HashMap::default());
}

fn rebind_generics_from_previous_function(&mut self) {
// Remove the currently bound generics first.
self.bound_generics.pop();

if let Some(bindings) = self.bound_generics.last() {
for (var, binding) in bindings {
var.force_bind(binding.clone());
}
}
}

fn remember_bindings(&mut self, main_bindings: &TypeBindings, impl_bindings: &TypeBindings) {
let bound_generics = self
.bound_generics
.last_mut()
.expect("remember_bindings called with no bound_generics on the stack");

for (var, binding) in main_bindings.values() {
bound_generics.insert(var.clone(), binding.follow_bindings());
}

for (var, binding) in impl_bindings.values() {
bound_generics.insert(var.clone(), binding.follow_bindings());
}
}

pub(super) fn define_pattern(
&mut self,
pattern: &HirPattern,
Expand Down
7 changes: 7 additions & 0 deletions test_programs/execution_success/regression_5615/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "regression_5615"
type = "bin"
authors = [""]
compiler_version = ">=0.32.0"

[dependencies]
12 changes: 12 additions & 0 deletions test_programs/execution_success/regression_5615/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
use std::collections::umap::UHashMap;
use std::hash::BuildHasherDefault;
use std::hash::poseidon2::Poseidon2Hasher;

unconstrained fn main() {
comptime
{
let mut map: UHashMap<u32, Field, BuildHasherDefault<Poseidon2Hasher>> = UHashMap::default();

map.insert(1, 2);
}
}

0 comments on commit 28211a3

Please sign in to comment.