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

fix(frontend): Call trait method with mut self from generic definition #5041

Merged
merged 63 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
63 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
0e5a579
initial work to get calling trait methods working, decided this would…
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
405683e
merge conflicts w/ jf/turbofish and working trait methods with mutabl…
vezenovm May 16, 2024
5ad8fb4
cleanup
vezenovm May 16, 2024
e7000d7
Merge branch 'master' into jf/turbofish
vezenovm May 16, 2024
3cbba0b
merge conflicts w/ parent jf/turbofish
vezenovm May 16, 2024
cc135e7
cargo fmt
vezenovm May 16, 2024
a0090c1
cleanup
vezenovm May 16, 2024
954aba7
nargo fmt
vezenovm May 16, 2024
1e22003
add todo comments
vezenovm May 16, 2024
44a4aee
specify type annotation for hasher
vezenovm May 17, 2024
8d5c072
rename
vezenovm May 17, 2024
c6f3057
brought back removed comment
vezenovm May 17, 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
b2100ad
Update compiler/noirc_frontend/src/hir/type_check/expr.rs
vezenovm May 20, 2024
0d7fb67
Update test_programs/execution_success/trait_method_mut_self/src/main.nr
vezenovm May 20, 2024
c045540
Update test_programs/execution_success/trait_method_mut_self/src/main.nr
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
af2c858
fixup after code review
vezenovm May 20, 2024
aa8e49e
fmy and clippy
vezenovm May 20, 2024
af3cc88
Merge branch 'jf/turbofish' into mv/trait-method-reference
vezenovm May 20, 2024
e6850e9
update comment
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
8d93a66
Merge branch 'jf/turbofish' into mv/trait-method-reference
vezenovm May 21, 2024
a44782b
Merge branch 'master' into jf/turbofish
vezenovm May 21, 2024
d174e13
Merge branch 'jf/turbofish' into mv/trait-method-reference
vezenovm May 21, 2024
c05e917
Merge branch 'master' into mv/trait-method-reference
vezenovm May 21, 2024
c0b9684
remove duplicated function count
vezenovm May 21, 2024
8a0d483
missing stdlib format
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
32 changes: 21 additions & 11 deletions compiler/noirc_frontend/src/hir/type_check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,17 +236,27 @@ impl<'interner> TypeChecker<'interner> {

// Automatically add `&mut` if the method expects a mutable reference and
// the object is not already one.
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
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(
&mut method_call,
&function_type,
&mut object_type,
);
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(
&mut method_call,
&function_type,
&mut object_type,
);
}

// TODO: update object_type here?
Expand Down Expand Up @@ -567,7 +577,7 @@ impl<'interner> TypeChecker<'interner> {

/// Insert as many dereference operations as necessary to automatically dereference a method
/// call object to its base value type T.
fn insert_auto_dereferences(&mut self, object: ExprId, typ: Type) -> (ExprId, Type) {
pub(crate) fn insert_auto_dereferences(&mut self, object: ExprId, typ: Type) -> (ExprId, Type) {
if let Type::MutableReference(element) = typ {
let location = self.interner.id_location(object);

Expand Down
9 changes: 8 additions & 1 deletion compiler/noirc_frontend/src/hir/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,15 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec<Type
}

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

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

type_checker.verify_trait_constraint(
&constraint.typ,
constraint.trait_id,
Expand Down
2 changes: 1 addition & 1 deletion noir_stdlib/src/lib.nr
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,4 @@ pub fn wrapping_mul<T>(x: T, y: T) -> T {
}

#[builtin(as_witness)]
pub fn as_witness(x: Field) {}
pub fn as_witness(x: Field) {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "trait_method_mut_self"
type = "bin"
authors = [""]
compiler_version = ">=0.29.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
x = "5"
y = "10"
74 changes: 74 additions & 0 deletions test_programs/execution_success/trait_method_mut_self/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
use dep::std::hash::Hasher;
use dep::std::hash::poseidon2::Poseidon2Hasher;

fn main(x: Field, y: pub Field) {
let mut a_mut_ref = AType { x };

pass_trait_by_value(a_mut_ref, y);
assert(a_mut_ref.x == x);

pass_trait_by_value_impl_param(a_mut_ref, y);
assert(a_mut_ref.x == x);

pass_trait_by_mut_ref(&mut a_mut_ref, y);
assert(a_mut_ref.x == y);

let mut hasher = Poseidon2Hasher::default();
hasher.write(x);
hasher.write(y);
let expected_hash = hasher.finish();
// Check that we get the same result when using the hasher in a
// method that purely uses trait methods without a supplied implementation.
assert(hash_simple_array::<Poseidon2Hasher>([x, y]) == expected_hash);
}

trait SomeTrait {
fn set_value(&mut self, new_value: Field) -> ();

fn get_value(self) -> Field;
}

struct AType {
x: Field
}

impl SomeTrait for AType {
fn set_value(&mut self, new_value: Field) -> () {
self.x = new_value;
}

fn get_value(self) -> Field {
self.x
}
}

fn pass_trait_by_value_impl_param(mut a_mut_ref: impl SomeTrait, value: Field) {
// We auto add a mutable reference to the object type if the method call expects a mutable self
a_mut_ref.set_value(value);
assert(a_mut_ref.get_value() == value);
}

fn pass_trait_by_value<T>(mut a_mut_ref: T, value: Field) where T: SomeTrait {
// We auto add a mutable reference to the object type if the method call expects a mutable self
a_mut_ref.set_value(value);
assert(a_mut_ref.get_value() == value);
}

fn pass_trait_by_mut_ref<T>(a_mut_ref: &mut T, value: Field) where T: SomeTrait {
// We auto add a mutable reference to the object type if the method call expects a mutable self
a_mut_ref.set_value(value);
}

fn hash_simple_array<H>(input: [Field; 2]) -> Field where H: Hasher + Default {
// Check that we can call a trait method instead of a trait implementation
// TODO: Need to remove the need for this type annotation
// TODO: Curently, without the annotation we will get `Expression type is ambiguous` when trying to use the `hasher`
let mut hasher: H = H::default();
// Regression that the object is converted to a mutable reference type `&mut _`.
// Otherwise will see `Expected type &mut _, found type H`.
// Then we need to make sure to also auto dereference later in the type checking process
// when searching for a matching impl or else we will get `No matching impl found for `&mut H: Hasher`
hasher.write(input[0]);
hasher.write(input[1]);
hasher.finish()
}
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
Loading