Skip to content

chore: Remove Noir builtin comptime mutable methods from macros#21801

Merged
vezenovm merged 1 commit into
nextfrom
jf/remove-comptime-mut2
Mar 25, 2026
Merged

chore: Remove Noir builtin comptime mutable methods from macros#21801
vezenovm merged 1 commit into
nextfrom
jf/remove-comptime-mut2

Conversation

@jfecher

@jfecher jfecher commented Mar 19, 2026

Copy link
Copy Markdown
Contributor

Remake of #21321 since that PR got messed up from a bad merge.

The following functions will be removed from Noir:

FunctionDefinition::add_attribute
FunctionDefinition::set_body
FunctionDefinition::set_parameters
FunctionDefinition::set_return_type
FunctionDefinition::set_return_public
FunctionDefinition::set_return_data
FunctionDefinition::set_unconstrained
Module::add_item
TypeDefinition::add_attribute
TypeDefinition::add_generic
TypeDefinition::set_fields

Only being replaced by the new functions FunctionDefinition::disable and TypeDefinition::add_abi which were the minimal set of functions determined to be needed here so that the Noir team can remove as many of the old mutating functions as possible.

Since the original PR, a call to f.add_attribute("noinitcheck") has been added. I've replaced this with an explicit call to noinitcheck(f) and adding f to a global list of functions that should be considered to have a noinitcheck attribute to keep the check for that attribute working.

@jfecher jfecher requested a review from vezenovm March 19, 2026 18:40
@jfecher jfecher requested a review from nventuro as a code owner March 19, 2026 18:40
@benesjan

Copy link
Copy Markdown
Contributor

Given this error in the CI run:

account/ecdsa_k_account_contract	error: No method named 'disable' found for type 'FunctionDefinition'
account/ecdsa_k_account_contract	   ┌─ /home/aztec-dev/aztec-packages/noir-projects/aztec-nr/aztec/src/macros/internals_functions_generation/mod.nr:31:9
account/ecdsa_k_account_contract	   │
account/ecdsa_k_account_contract	31 │         function.disable(error_message);
account/ecdsa_k_account_contract	   │         -------------------------------
account/ecdsa_k_account_contract	   │

Is this waiting for a new Noir to be synced to the repo?

@jfecher

jfecher commented Mar 23, 2026

Copy link
Copy Markdown
Contributor Author

Is this waiting for a new Noir to be synced to the repo?

Yep. We're looking at doing it in #21766. Main issue is that the deny warnings argument means we can't deprecate anything in the noir stdlib without it being a breaking change here. So #21766 is currently including both this change and the compiler update together. We merged another PR into nargo to not deprecate the old methods instead for now.

@jfecher jfecher force-pushed the jf/remove-comptime-mut2 branch from d35a1a0 to e4af615 Compare March 23, 2026 20:05
@jfecher jfecher force-pushed the jf/remove-comptime-mut2 branch 2 times, most recently from ecbc340 to d094299 Compare March 23, 2026 20:27
@vezenovm

Copy link
Copy Markdown
Contributor

@critesjosh @sklppy88 do you know why the docs deploy may be failing here?

@jfecher

jfecher commented Mar 23, 2026

Copy link
Copy Markdown
Contributor Author

Another bad squash. Embarrassing. Apologies for the extra review requests.

This PR should be ready to go now - unsure why docs seem to be using old noir versions without the new methods added in #21566.

@critesjosh

Copy link
Copy Markdown
Contributor

i updated the branch to see if it resolves the docs failure

@critesjosh

Copy link
Copy Markdown
Contributor

it looks like netlify (where our docs are hosted and built) does not compile noir from source, it downloads a nightly binary using noirup and its not getting the right one. im looking into it

@critesjosh

Copy link
Copy Markdown
Contributor

I just cleared the netlify cache and rebuilt and it looks like its working

@benesjan benesjan changed the base branch from merge-train/fairies to next March 25, 2026 13:18
@benesjan benesjan marked this pull request as ready for review March 25, 2026 13:18

@benesjan benesjan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@jfecher introducing the extra global var for the only self function was unnecessary as we can just check for the only self attribute directly. Simplified that in 045a96e

Now it looks good to merge 👍

@benesjan benesjan enabled auto-merge March 25, 2026 13:21
@benesjan

Copy link
Copy Markdown
Contributor

Failed to reproduce the failure of e2e_offchain_payment.test.ts locally so it's probably just a flake.

Will try to re-run and then bother someone with it

@benesjan benesjan added this pull request to the merge queue Mar 25, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Mar 25, 2026
@benesjan benesjan enabled auto-merge March 25, 2026 15:23
Remake of #21321 since that PR got messed up from a bad merge.

The following functions will be removed from Noir:
```rs
FunctionDefinition::add_attribute
FunctionDefinition::set_body
FunctionDefinition::set_parameters
FunctionDefinition::set_return_type
FunctionDefinition::set_return_public
FunctionDefinition::set_return_data
FunctionDefinition::set_unconstrained
Module::add_item
TypeDefinition::add_attribute
TypeDefinition::add_generic
TypeDefinition::set_fields
```
Only being replaced by the new functions `FunctionDefinition::disable` and `TypeDefinition::add_abi` which were the minimal set of functions determined to be needed here so that the Noir team can remove as many of the old mutating functions as possible.

Since the original PR, a call to `f.add_attribute("noinitcheck")` has been added. I've replaced this with an explicit call to `noinitcheck(f)` and adding `f` to a global list of functions that should be considered to have a noinitcheck attribute to keep the check for that attribute working.

Co-authored-by: benesjan <benesjan@users.noreply.github.com>
@AztecBot AztecBot force-pushed the jf/remove-comptime-mut2 branch from 41db394 to a252184 Compare March 25, 2026 15:30
@benesjan benesjan added this pull request to the merge queue Mar 25, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Mar 25, 2026
@jfecher jfecher added this pull request to the merge queue Mar 25, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Mar 25, 2026
@jfecher jfecher added this pull request to the merge queue Mar 25, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Mar 25, 2026
@vezenovm vezenovm added this pull request to the merge queue Mar 25, 2026
Merged via the queue into next with commit 5de59a7 Mar 25, 2026
25 checks passed
@vezenovm vezenovm deleted the jf/remove-comptime-mut2 branch March 25, 2026 18:50
@AztecBot

Copy link
Copy Markdown
Collaborator

❌ Failed to cherry-pick to v4-next due to conflicts. (🤖) View backport run.

AztecBot pushed a commit that referenced this pull request Apr 11, 2026
…ds from macros)

Cherry-pick of 5de59a7 — replaces deprecated Noir comptime APIs:
- add_attribute('abi(events)') -> add_abi('events')
- set_body/set_parameters/add_attribute/set_return_public -> function.disable()
- fn_has_noinitcheck now also checks is_fn_only_self()
- Removes f.add_attribute('noinitcheck') from only_self
Thunkar added a commit that referenced this pull request Apr 13, 2026
…#22485)

## Summary

Backport of #22393
(Update Noir to nightly-2026-04-10) to v4-next, plus 3 companion PRs
needed for Noir compatibility since v4-next was still on
nightly-2026-02-12.

## Companion PRs included

Since v4-next hadn't updated Noir since nightly-2026-02-12 (~2 months
behind), several intermediate Noir-adaptation PRs were also needed:

1. **#20702** — Remove unnecessary `comptime` qualifier from VK tree
constants (Noir broke comptime globals used in non-comptime contexts)
2. **#20798** — Remove unnecessary `let mut` across noir-projects (newer
Noir errors on this)
3. **#21801** — Replace deprecated Noir comptime APIs: `add_attribute()`
→ `add_abi()`, `set_body()`/`set_parameters()`/`set_return_public()` →
`function.disable()`, update `fn_has_noinitcheck` to also check
`is_fn_only_self()`

## Commits
1. **Cherry-pick with conflict markers** — raw cherry-pick of #22393
2. **Conflict resolution** — version bumps + reformatting resolved
3. **Cherry-pick #20702** — comptime globals + poseidon2 hash fix
4. **Cherry-pick #20798** — unnecessary mut removal (24 files)
5. **Cherry-pick #21801** — macro API migration (4 files)
6. **Remaining fixes** — private_context.nr mut + utils.nr style
alignment
github-merge-queue Bot pushed a commit that referenced this pull request Apr 16, 2026
…tlify (#21919)

## Summary

- Makes the Netlify preview build continue when aztec-nr API doc
generation fails, instead of aborting the entire deploy
- Fixes the issue seen in #21801 where the preview deploy fails because
the downloaded nargo nightly binary lacks new comptime APIs
(`FunctionDefinition::disable`, `TypeDefinition::add_abi`) that the
aztec-nr code uses

## Context

The Netlify preview build downloads a pre-built nargo binary via
`noirup`. When the noir submodule in aztec-packages is ahead of the
latest published nightly, `nargo doc` can fail because the binary
doesn't have APIs that aztec-nr's macros depend on. This blocks the
entire preview deploy even though the API docs are non-essential for PR
review.

With this change, the build logs a warning and continues without API
docs rather than failing entirely.

## Test plan

- [ ] Verify Netlify preview deploys successfully even when nargo
version is incompatible
- [ ] Verify API docs still generate normally when nargo version is
compatible
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants