Skip to content

fix(frontend)!: Preserve int type when quoting tokens #10330

Merged
jfecher merged 11 commits intomasterfrom
mv/unquote-preserve-int-type
Nov 12, 2025
Merged

fix(frontend)!: Preserve int type when quoting tokens #10330
jfecher merged 11 commits intomasterfrom
mv/unquote-preserve-int-type

Conversation

@vezenovm
Copy link
Contributor

Description

Problem*

Resolves #10326

Summary*

I just updated the Value::into_tokens method to keep the appropriate suffix type when converting a Value into Tokens. Thus the tokens in the Quoted value now maintain their integer type. When we then look to unquote those tokens they will be parsed with their appropriate integer type.

Working on this issue revealed #10328

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.

@vezenovm vezenovm changed the base branch from master to mv/meta-roundtrip-testing October 30, 2025 19:42
@jfecher
Copy link
Contributor

jfecher commented Oct 30, 2025

Does this run into the same issues found in #8369 ?

@vezenovm
Copy link
Contributor Author

vezenovm commented Oct 30, 2025

Does this run into the same issues found in #8369 ?

We can make a case like in #8369:

fn main() {
    let got: u8 = comptime {
        let original: u8 = 10;
        unquote!(quote { $original })
    };
    assert_eq(got, 10);
}
comptime fn unquote(code: Quoted) -> Quoted {
    code
}

On nightly this fails with Expected type u8, found type Field. This snippet successfully compiles on this branch.

It looks like #10326 can cause similar issues to #8369, but the cause is not exactly the same. In #8369 we were not appropriately binding the return type from a comptime context and thus ultimately unifying two different types (such as i32 and Field). The fact that we bind the polymorphic integer to a Field in #8369 feels like a separate issue. In this case, we have explicitly specified a type and are losing type information just when quoting values within the same comptime context. As per the linked issue we then get a type mismatch error (which is expected as we have dropped the original u8 type for a Field).

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: b3e21ec Previous: fe0bfa0 Ratio
test_report_zkpassport_noir_rsa_ 2 s 0 s +∞

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

CC: @TomAFrench

@vezenovm
Copy link
Contributor Author

vezenovm commented Oct 30, 2025

Looks like we're getting some errors related to attribute argument types and numeric generics. We're getting a lot of errors in noir-bignum for the type supplied to a numeric generic https://github.com/noir-lang/noir/actions/runs/18953822445/job/54125951530?pr=10330. Will need to investigate deeper to figure out what is going on.

Base automatically changed from mv/meta-roundtrip-testing to master October 30, 2025 20:48
@asterite
Copy link
Collaborator

Right, I think Jake tried it in the past when suffixes were introduced: #8970 (comment)

@vezenovm vezenovm added the Blocked PR is blocked on an issue label Nov 4, 2025
@jfecher
Copy link
Contributor

jfecher commented Nov 6, 2025

external repos should be compiling now. The issue was the kind checking for integer literals w/ type suffixes used in a type position was wrong.

We were using integersuffix.as_type() which returns something like u32 but the expected kind was Kind::Numeric(u32) - which is not the same. u32 has kind Normal, while 3u32 would have the expected Numeric(u32) kind.

To fix this I introduced a new helper, added some notes on both functions on when to use them, and added a regression.

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.

nice

edit: still failing 😓 bignum compiles now though

@jfecher jfecher enabled auto-merge November 6, 2025 21:00
@jfecher
Copy link
Contributor

jfecher commented Nov 6, 2025

From some quick tests it looks like aztec-nr and friends were a bit lax in making use of this unintended feature of integers changing types. Lots of errors like:

error: Expected type Field, found type u32
   ┌─ /.../aztec-packages/noir-projects/aztec-nr/aztec/src/macros/storage.nr:50:62
   │
50 │             generate_storage_field_constructor(typ, quote { $slot });
   │                                                              ----
   │
   ┌─ contracts/test/avm_test_contract/src/main.nr:52:5
   │
52 │     #[storage]
   │     ---------- While running this function attribute
   │

So there are some updates required to those repos - I think this PR is correct, just breaking.

@jfecher jfecher changed the title fix(frontend): Preserve int type when quoting tokens fix(frontend)!: Preserve int type when quoting tokens Nov 6, 2025
@jfecher
Copy link
Contributor

jfecher commented Nov 6, 2025

Ah, one more thing. Including the integer type suffix messes up using integers as tuple field accesses currently:

error: Unexpected '.', expected a field name or number
    ┌─ /home/runner/work/noir/noir/test-repo/noir-projects/noir-protocol-circuits/crates/types/src/tuple_serialization.nr:54:33
    │
 54 │         let serialized = self.$i.serialize();
    │                                 -
    ·
126 │ #[impl_serialize_for_tuple(2)]
    │ ------------------------------ Failed to parse macro's token stream into top-level item
    │
    = The resulting token stream was: (stream starts on next line)
        impl < T0, T1 > Serialize for(T0, T1)where T0: Serialize, T1: Serialize,  {
          let N: u32 =  < T0 as Serialize > ::N +  < T1 as Serialize > ::N;
          fn serialize(self) -> [Field;
          Self::N] {
              let mut result: [Field;
              Self::N] = std::mem::zeroed();
              let mut offset = 0;
              let serialized = self.0_u32.serialize();
              ...

Edit: Fixed by just allowing integer type suffixes in that position.

@jfecher jfecher disabled auto-merge November 6, 2025 21:08
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 'Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 5d09254 Previous: 60bfcf2 Ratio
rollup-block-root-single-tx 0.003 s 0.002 s 1.50
sha512-100-bytes 0.095 s 0.072 s 1.32

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

CC: @TomAFrench

github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Nov 10, 2025
)

After noir-lang/noir#10330, noir will start
enforcing the types of unquoted integers remain the same type. Before,
when you unquoted an integer such as `let n: u32 = 4;` e.g. in `quote {
let x = $n; }` the unquoted `n` would be a polymorphic integer literal
of any type. With the above PR, it unquotes instead to `let x = 4u32;` -
the type is preserved. We consider it a bug to rely on the previous
behavior, and there were 3 instances of it in aztec-nr so this is a
patch for those cases.
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Nov 10, 2025
)

After noir-lang/noir#10330, noir will start
enforcing the types of unquoted integers remain the same type. Before,
when you unquoted an integer such as `let n: u32 = 4;` e.g. in `quote {
let x = $n; }` the unquoted `n` would be a polymorphic integer literal
of any type. With the above PR, it unquotes instead to `let x = 4u32;` -
the type is preserved. We consider it a bug to rely on the previous
behavior, and there were 3 instances of it in aztec-nr so this is a
patch for those cases.
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Nov 10, 2025
)

After noir-lang/noir#10330, noir will start
enforcing the types of unquoted integers remain the same type. Before,
when you unquoted an integer such as `let n: u32 = 4;` e.g. in `quote {
let x = $n; }` the unquoted `n` would be a polymorphic integer literal
of any type. With the above PR, it unquotes instead to `let x = 4u32;` -
the type is preserved. We consider it a bug to rely on the previous
behavior, and there were 3 instances of it in aztec-nr so this is a
patch for those cases.
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Nov 11, 2025
)

After noir-lang/noir#10330, noir will start
enforcing the types of unquoted integers remain the same type. Before,
when you unquoted an integer such as `let n: u32 = 4;` e.g. in `quote {
let x = $n; }` the unquoted `n` would be a polymorphic integer literal
of any type. With the above PR, it unquotes instead to `let x = 4u32;` -
the type is preserved. We consider it a bug to rely on the previous
behavior, and there were 3 instances of it in aztec-nr so this is a
patch for those cases.
@vezenovm vezenovm removed the Blocked PR is blocked on an issue label Nov 11, 2025
@vezenovm
Copy link
Contributor Author

Looks like we're good here post the external repos commit. Reviewed the changes made since the initial review and all looks good. Going to merge.

@vezenovm vezenovm added this pull request to the merge queue Nov 11, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 11, 2025
@jfecher jfecher added this pull request to the merge queue Nov 11, 2025
Merged via the queue into master with commit 1b1985e Nov 12, 2025
132 checks passed
@jfecher jfecher deleted the mv/unquote-preserve-int-type branch November 12, 2025 00:00
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Nov 12, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(frontend)!: Preserve int type when quoting tokens
(noir-lang/noir#10330)
fix: check overflow for Pedersen grumpkin scalars
(noir-lang/noir#10462)
chore(frontend): Various tests in elaborator expressions submodule and
minor refactors (noir-lang/noir#10475)
chore: bump external pinned commits
(noir-lang/noir#10477)
fix: disallow keywords in attributes
(noir-lang/noir#10473)
chore: refactor codegen_control_flow
(noir-lang/noir#10320)
fix: builtin with body now errors instead of crashing
(noir-lang/noir#10474)
fix: handle ambiguous trait methods in assumed traits
(noir-lang/noir#10468)
fix: force_substitute bindings during monomorphization for associated
constants (noir-lang/noir#10467)
fix(brillig): Skip decrementing ref-count in array/vector copy and other
refactors (noir-lang/noir#10335)
fix(ssa): Cast to `u64` when inserting OOB checks in DIE
(noir-lang/noir#10463)
fix: disallow comptime-only types in non-comptime globals
(noir-lang/noir#10458)
chore(fuzzing): fix default artifact for brillig target
(noir-lang/noir#10465)
END_COMMIT_OVERRIDE
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Nov 12, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(frontend)!: Preserve int type when quoting tokens
(noir-lang/noir#10330)
fix: check overflow for Pedersen grumpkin scalars
(noir-lang/noir#10462)
chore(frontend): Various tests in elaborator expressions submodule and
minor refactors (noir-lang/noir#10475)
chore: bump external pinned commits
(noir-lang/noir#10477)
fix: disallow keywords in attributes
(noir-lang/noir#10473)
chore: refactor codegen_control_flow
(noir-lang/noir#10320)
fix: builtin with body now errors instead of crashing
(noir-lang/noir#10474)
fix: handle ambiguous trait methods in assumed traits
(noir-lang/noir#10468)
fix: force_substitute bindings during monomorphization for associated
constants (noir-lang/noir#10467)
fix(brillig): Skip decrementing ref-count in array/vector copy and other
refactors (noir-lang/noir#10335)
fix(ssa): Cast to `u64` when inserting OOB checks in DIE
(noir-lang/noir#10463)
fix: disallow comptime-only types in non-comptime globals
(noir-lang/noir#10458)
chore(fuzzing): fix default artifact for brillig target
(noir-lang/noir#10465)
END_COMMIT_OVERRIDE
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Nov 12, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(frontend)!: Preserve int type when quoting tokens
(noir-lang/noir#10330)
fix: check overflow for Pedersen grumpkin scalars
(noir-lang/noir#10462)
chore(frontend): Various tests in elaborator expressions submodule and
minor refactors (noir-lang/noir#10475)
chore: bump external pinned commits
(noir-lang/noir#10477)
fix: disallow keywords in attributes
(noir-lang/noir#10473)
chore: refactor codegen_control_flow
(noir-lang/noir#10320)
fix: builtin with body now errors instead of crashing
(noir-lang/noir#10474)
fix: handle ambiguous trait methods in assumed traits
(noir-lang/noir#10468)
fix: force_substitute bindings during monomorphization for associated
constants (noir-lang/noir#10467)
fix(brillig): Skip decrementing ref-count in array/vector copy and other
refactors (noir-lang/noir#10335)
fix(ssa): Cast to `u64` when inserting OOB checks in DIE
(noir-lang/noir#10463)
fix: disallow comptime-only types in non-comptime globals
(noir-lang/noir#10458)
chore(fuzzing): fix default artifact for brillig target
(noir-lang/noir#10465)
END_COMMIT_OVERRIDE
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.

Integer types not preserved when unquoting

3 participants