Skip to content

fix: push definition trait constraints after trait item constraint#9354

Merged
asterite merged 2 commits intomasterfrom
ab/object-type-unknown-in-method-call-with-trait-bound
Jul 29, 2025
Merged

fix: push definition trait constraints after trait item constraint#9354
asterite merged 2 commits intomasterfrom
ab/object-type-unknown-in-method-call-with-trait-bound

Conversation

@asterite
Copy link
Collaborator

@asterite asterite commented Jul 29, 2025

Description

Problem

Resolves #9349

Summary

I still don't fully understand how everything is tied together, but this comment from @jfecher made me wonder if it's indeed an ordering issue.

Inside type_check_variable_with_bindings we potentially push these constraints:

  1. a constraint for the trait method itself
  2. constraints for any where clause on the method

Before this PR we were pushing 2 before 1. There was also a comment about why 1 needs to come before 2, though since that comment was written the select_impl argument was added so I think that comment became obsolete at that point (since the constraints for the where clause won't lead to selecting a different impl).

Specifically, for the concrete example in the test we got that the type of the BigCurve::new call is:

fn() -> '7

Then we have this constraint for point 1:

'7: BigCurve<'6>

and this one for point 2:

'6: BigNum

We were checking point 2 before point 1, and it was failing because '6 was unbound.

When we flip the order of operations, when we solve '7: BigCurve<'6> we get these type bindings applied:

{
    '6: (
        '6,
        Normal,
        B'4,
    ),
}

At that point '6 is B'4 (which I guess has an assumed impl for BigNum, according to the function signature), then the constraint '6: BigNum will pass.

Additional Context

Thank you @jfecher ! I merely tried flipping the order here, the debugged things a bit more for the above explanation (it was hard to see that the bindings produced by verify_trait_constraint would make the second check work because when it's flipped we don't get to check the other one). The conclusion is that you solved it, I merely sent the code change 😅

(well, we still have to see if CI passes... I do wonder if flipping the order would cause a problem in other scenarios)

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.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 22ae2aa Previous: ccbea58 Ratio
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob 3 s 2 s 1.50
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_private-kernel-lib 2 s 1 s 2

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

Copy link
Contributor

@jfecher jfecher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Let's just add a comment to help protect this against further reorderings

@asterite asterite enabled auto-merge July 29, 2025 18:38
@asterite asterite added this pull request to the merge queue Jul 29, 2025
Merged via the queue into master with commit 7d98dc2 Jul 29, 2025
101 checks passed
@asterite asterite deleted the ab/object-type-unknown-in-method-call-with-trait-bound branch July 29, 2025 19:30
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Aug 4, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix: forbid self-referencing type aliases
(noir-lang/noir#9103)
chore: add a mem2reg test for when all references need to be invalidated
(noir-lang/noir#9377)
fix(ssa): Do not check ArrayGet/Set as unreachable for Brillig
(noir-lang/noir#9376)
chore: use SSA parser in all mem2reg tests
(noir-lang/noir#9372)
fix: trait where clause check fixes
(noir-lang/noir#9369)
fix: Correct doc comments for SSA passes
(noir-lang/noir#9371)
fix: prevent `SignedField::from(i128::MIN)` from crashing
(noir-lang/noir#9366)
fix: allow constants in the type-system to be negative
(noir-lang/noir#9360)
feat: show circuit output as a value of the program's return type
(noir-lang/noir#9364)
feat: add `FunctionDefinition::visibility`
(noir-lang/noir#9363)
chore(docs): Add example for `$crate` in docs
(noir-lang/noir#9361)
fix: Prevent accidental tuple sharing in comptime code
(noir-lang/noir#9313)
fix: perserve purities after SSA normalization
(noir-lang/noir#9355)
fix: modulo overflow in comptime
(noir-lang/noir#9348)
fix: handle short-syntax for trait constraints on trait generics
(noir-lang/noir#9167)
chore: enhance trait constraint comment
(noir-lang/noir#9358)
fix: replace implicitly added named generics with fresh type vars in
check_trait_impl_where_clause_matches_trait_where_clause
(noir-lang/noir#9352)
fix: push definition trait constraints after trait item constraint
(noir-lang/noir#9354)
chore(ci): Update status of noir_json_parser
(noir-lang/noir#9351)
fix(ssa): Keep reference count increments for array set values
(noir-lang/noir#9344)
chore: remove unused `compile_workspace`
(noir-lang/noir#9353)
chore: try printing byte arrays as strings in the SSA interpreter
(noir-lang/noir#9346)
feat(lsp): allow opening noir stdlib files
(noir-lang/noir#9339)
fix: do u128 operations with u128, not i128
(noir-lang/noir#9345)
chore(acir): ACIR parser error handling for blackbox inputs/outputs
(noir-lang/noir#9342)
fix: prevent invalid types in test/fuzz functions
(noir-lang/noir#9343)
chore(lsp): avoid redundant type checking
(noir-lang/noir#9337)
feat(acir): Parse ACIR memory and call opcodes
(noir-lang/noir#9331)
fix(ssa_gen): Add constraint on slice length before popping
(noir-lang/noir#9323)
chore: impl for u16 conversions
(noir-lang/noir#9314)
fix: substitute bindings in type before canonicalization
(noir-lang/noir#9328)
fix(ssa_interpreter): `push_back` and `pop_back` to slices with padding
(noir-lang/noir#9320)
fix: wildcard type should be allowed in lambda parameter types
(noir-lang/noir#9325)
chore: graceful handling of SIGPIPE
(noir-lang/noir#9075)
feat: return unsolvable opcode from `CircuitSimulator`
(noir-lang/noir#8943)
fix: allow nested fmtstr (noir-lang/noir#9309)
feat: Initial ACIR parser (arithmetic exprs and black box functions)
(noir-lang/noir#9316)
fix(mem2reg): Register aliases when the `IfElse` result in a reference
(noir-lang/noir#9305)
fix: Make Ssa-gen use existing reference when compiling `&mut
foo.bar.baz` (noir-lang/noir#9307)
fix: top-level item in dependency isn't always visible
(noir-lang/noir#9295)
fix(ssa-interpreter): Return error if slice length is 0 during popping
(noir-lang/noir#9308)
chore: Release Noir(1.0.0-beta.9)
(noir-lang/noir#9184)
chore(LSP): simplify code lens request handling
(noir-lang/noir#9279)
chore: add regression tests for #6383
(noir-lang/noir#9302)
fix: disallow `_` in signatures and struct members
(noir-lang/noir#9301)
fix: check associated types after validating where clause when looking
up trait impls, plus some unification fixes
(noir-lang/noir#9265)
chore: Add fmtstr to coercions list
(noir-lang/noir#9300)
chore: Add a helper function `fmtstr::as_quoted_str`
(noir-lang/noir#9293)
chore(docs): Copy Type Coercions docs into v1.0.0-beta.8 versioned docs
(noir-lang/noir#9298)
feat: only inject "out of bounds" checks in brillig
(noir-lang/noir#9200)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Object type unknown in method call when trait generic has bound

2 participants