Skip to content

Commit

Permalink
fix: Variables from trait constraints being permanently bound over wh…
Browse files Browse the repository at this point in the history
…en used within a trait impl (#4450)

# Description

## Problem\*

Resolves #4436 

## Summary\*

This issue stemmed from calling another trait method within an impl for
the trait. The trait method required a trait constraint of the same
trait to be valid and was using the trait with its original type
variables. These would later be bound over when an impl was searched for
for this trait.

I've added a somewhat hacky solution of avoiding inserting `t = t`
bindings from the trait constraint when checking identifiers. Avoiding
inserting this binding means `t` in this case will later be instantiated
to a fresh type variable later on in the `instantiate_with_bindings`
call.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** 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 committed Mar 1, 2024
1 parent 21fc4b8 commit ac60ef5
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 2 deletions.
5 changes: 4 additions & 1 deletion compiler/noirc_frontend/src/hir/type_check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,10 @@ impl<'interner> TypeChecker<'interner> {
assert_eq!(the_trait.generics.len(), constraint.trait_generics.len());

for (param, arg) in the_trait.generics.iter().zip(&constraint.trait_generics) {
bindings.insert(param.id(), (param.clone(), arg.clone()));
// Avoid binding t = t
if !arg.occurs(param.id()) {
bindings.insert(param.id(), (param.clone(), arg.clone()));
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/hir_def/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1558,7 +1558,7 @@ impl Type {
}

/// True if the given TypeVariableId is free anywhere within self
fn occurs(&self, target_id: TypeVariableId) -> bool {
pub fn occurs(&self, target_id: TypeVariableId) -> bool {
match self {
Type::Array(len, elem) => len.occurs(target_id) || elem.occurs(target_id),
Type::String(len) => len.occurs(target_id),
Expand Down
5 changes: 5 additions & 0 deletions test_programs/execution_success/regression_4436/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[package]
name = "regression_4436"
type = "bin"
authors = [""]
compiler_version = ">=0.22.0"
31 changes: 31 additions & 0 deletions test_programs/execution_success/regression_4436/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
trait LibTrait<N> {
fn broadcast();
fn get_constant() -> Field;
}

global STRUCT_A_LEN: Field = 3;
global STRUCT_B_LEN: Field = 5;

struct StructA;
struct StructB;

impl LibTrait<u32> for StructA {
fn broadcast() {
Self::get_constant();
}

fn get_constant() -> Field {
1
}
}
impl LibTrait<u64> for StructB {
fn broadcast() {
Self::get_constant();
}

fn get_constant() -> Field {
1
}
}

fn main() {}

0 comments on commit ac60ef5

Please sign in to comment.