fix(frontend): Resolve object types from method calls a single time

[vezenovm]

Description

Problem*

Resolves #5065

Probably resolves #4732 but need to test it or have aztec team test it.

Summary*

When working with a program like such where MyType implements MyTrait:

fn foo<T>() -> T where T: MyTrait {
    MyTrait::new()
}
fn concise_regression() -> MyType {
    Wrapper::new(foo()).unwrap()
}

We should be able to infer the return type of foo. We currently always push trait constraints onto a a Vec<(TraitConstraint, ExprId)>. We need to do this as we can have multiple trait constraints that need to be handled during monomorphization due to generics. However, when working with a method call this can cause us to store an old trait constraint that does not necessarily apply to the expression.

The nested function call in concise_regression initially adds a trait constraint simply for foo due to the call to Wrapper::new(foo()) and then another constraint for Wrapper::new(foo()).unwrap(). The call to Wrapper::new(foo()) cannot be bound to anything unless we introduce an intermediate variable. This felt like it would be overly complex and we just need to follow the accurate trait constraint for a function call expression.

Taking the test in the issue and this PR we have the following trait constraints on master for the foo expression:

TraitConstraint {
        typ: '23646 -> '23647,
        trait_id: TraitId(
            ModuleId {
                krate: Root(
                    1,
                ),
                local_id: LocalModuleId(
                    Index(
                        1,
                    ),
                ),
            },
        ),
        trait_generics: [],
    },
    TraitConstraint {
        typ: '23648 -> '23649 -> '23650 -> MyType,
        trait_id: TraitId(
            ModuleId {
                krate: Root(
                    1,
                ),
                local_id: LocalModuleId(
                    Index(
                        1,
                    ),
                ),
            },
        ),
        trait_generics: [],
    }

This is occurring due to an unnecessary type check on a method call's object type. This is cause a repeated trait constraint where one has incorrect type variables that cannot be resolved.

I have altered how MethodCall's and Call's are resolved as to avoid repeated type checks on the object type.

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 and/or cargo fmt on default settings.

[jfecher]

I think this approach misses the real bug. Looking at this again (and testing the example without the same new name used for both the impl and trait methods) I think this bug is actually caused by how we handle method calls currently.

To type check method calls we currently change the method call foo.bar(args..) into a function call bar(foo, args..) and then type check that. Perhaps it is related - e.g. a wrong ID is used there or we type check twice pushing a repeat trait constraint with a bad id. Notably, testing the example with .unwrap() replaced with Wrapper::unwrap(...) also fixes the issue.

[vezenovm]

Notably, testing the example with .unwrap() replaced with MyType::unwrap(...) also fixes the issue.

Thanks that is helpful info, I had not noticed that.

[vezenovm]

I think this bug is actually caused by how we handle method calls currently.

Good catch, this does look to be the case. Calling check_expression on the method_call.object a second time causes a second "Expression type is ambiguous" error to occur. Updating this PR with a new fix.

[jfecher]

Interestingly, the elaborator already works a bit differently here since I took the liberty of merging a few methods so that we don't resolve a method/function twice in this case. As a result it doesn't have this error either.

[vezenovm]

Interestingly, the elaborator already works a bit differently here since I took the liberty of merging a few methods so that we don't resolve a method/function twice in this case. As a result it doesn't have this error either.

I was able to get all tests passing with logic similar to the evaluator. We now avoid checking a call expression's arguments a second time.

[jfecher]

LGTM