Skip to content

chore: move a couple of SharedMutableValues functions outside of impl#13283

Merged
nventuro merged 1 commit intomasterfrom
ab/shared-mutable-values-move-fns-outside-of-impl
Apr 3, 2025
Merged

chore: move a couple of SharedMutableValues functions outside of impl#13283
nventuro merged 1 commit intomasterfrom
ab/shared-mutable-values-move-fns-outside-of-impl

Conversation

@asterite
Copy link
Contributor

@asterite asterite commented Apr 3, 2025

This was discussed a bit over slack, but here's some context.

In Rust this gives an error:

pub struct Foo<T> {
    x: T,
}

impl<T> Foo<T> {
    fn one(x: T) {
        Bar::two(x); // Cannot infer the value of const parameter U
    }
}

struct Bar<T, const U: usize> {
    x: T,
}

impl<T, const U: usize> Bar<T, U> {
    fn two(x: T) -> Foo<T> {
        Foo { x }
    }
}

fn main() {
    Foo::<i32>::one(1);
}

In the call Bar::two Rust needs to know what is the value of U. It can be solved by doing Bar::<T, 1234>::two(x) or using any numeric value... because the numeric value isn't used in that impl function... which probably means that it doesn't need to "belong" to Bar.

In this Noir PR gets merged that's what will happen. We have a similar case in this codebase where SharedMutableValues has two methods:

  • unpack_value_change: doesn't refer to INITIAL_DELAY
  • unpack_delay_change: doesn't refer to T

So, this PR moves those two methods outside of the impl, only specifying the generics that are needed.

@asterite asterite changed the title Move a couple of SharedMutableValues functions outside of impl chore: move a couple of SharedMutableValues functions outside of impl Apr 3, 2025
@asterite asterite requested a review from nventuro April 3, 2025 19:37
@nventuro nventuro added this pull request to the merge queue Apr 3, 2025
Merged via the queue into master with commit df9a40c Apr 3, 2025
7 of 8 checks passed
@nventuro nventuro deleted the ab/shared-mutable-values-move-fns-outside-of-impl branch April 3, 2025 23:14
github-merge-queue bot pushed a commit that referenced this pull request Apr 4, 2025
🤖 I have created a new Aztec Packages release
---


##
[0.84.0](v0.83.1...v0.84.0)
(2025-04-04)


### ⚠ BREAKING CHANGES

* `UnsconstrainedContext` --> `UtilityContext`
([#13246](#13246))
* `#[utility]` function
([#13243](#13243))
* Validate public setup fns and gas in p2p
([#13154](#13154))

### Features

* `#[utility]` function
([#13243](#13243))
([945ffa2](945ffa2))
* **avm:** tx hint init
([#13218](#13218))
([60a1a92](60a1a92))
* Remove 4 byte metadata from bb-produced proof
([#13231](#13231))
([0dcc915](0dcc915))
* To enable better ci dashboard.
([#13272](#13272))
([61c6375](61c6375))


### Bug Fixes

* **avm:** fix lookup builder and FF hashing
([#13263](#13263))
([2633856](2633856))
* ci3-external concurrency bug, reduce grind set
([2c5e830](2c5e830)),
closes
[#13285](#13285)
* ci3-external.yml
([#13291](#13291))
([6ad68ed](6ad68ed))
* Validate public setup fns and gas in p2p
([#13154](#13154))
([1ef4add](1ef4add)),
closes
[#10958](#10958)


### Miscellaneous

* `UnsconstrainedContext` --&gt; `UtilityContext`
([#13246](#13246))
([69df86f](69df86f))
* add some PrivateSet tests
([#13270](#13270))
([bd9e690](bd9e690))
* bump full prover test to 32 cores. hoping to boost speed.
([#13293](#13293))
([c8e95dd](c8e95dd))
* deflake p2p reqresp test
([#13271](#13271))
([b9164fa](b9164fa))
* don't dump on fail. click the link instead.
([#13292](#13292))
([ba0fb4d](ba0fb4d))
* flake
([#13277](#13277))
([62c32eb](62c32eb))
* make rahul happy with migration notes
([#13255](#13255))
([3dd75a6](3dd75a6))
* minor simulator utils cleanup
([#13250](#13250))
([8a622c9](8a622c9))
* move a couple of `SharedMutableValues` functions outside of impl
([#13283](#13283))
([df9a40c](df9a40c))
* nuking debug-only logger and various unused functionality in
`foundation`
([#13187](#13187))
([2d38e60](2d38e60))
* prevent eth devnet config contention in ci
([#13260](#13260))
([1581836](1581836))
* renaming unconstrained function as utility in TS
([#13249](#13249))
([34d03bb](34d03bb))
* replace relative paths to noir-protocol-circuits
([b5b99f8](b5b99f8))
* Speed up note hashes test
([#13282](#13282))
([ad23358](ad23358))
* update gov and proposer configs
([#13281](#13281))
([e1a5be3](e1a5be3))
* update slashing test port
([#13274](#13274))
([9a1ddc5](9a1ddc5))
* Want to fail fast on test runs and the wider ci run.
([#13258](#13258))
([f0553b8](f0553b8))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
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.

2 participants