fix: always type-check turbofish, and error when it's not allowed by asterite · Pull Request #8437 · noir-lang/noir

asterite · 2025-05-09T17:56:21Z

Description

Problem

Resolves #8434

Summary

Now when we get to elaborate a path we first type-check all of its turbofish. That already makes sure all turbofish is type-checked: previously it was only type-checked in some scenarios. For this I introduced two new types, TypedPath and TypedPathSegment... those names don't convince me much because the only thing typed there is the turbofish, but I couldn't find a better name.

Then, looking up paths always takes a TypedPath, which makes sure turbofish is resolved in all cases.

Finally, in some existing scenarios I made the compiler produce an error if there's turbofish when it's not allowed. At least all the cases in #8434 are covered.

Additional Context

Before this PR we were solving turobfish with target Kinds in mind. For example if a generic kind was let N: u32 then we'd solve constants with those kinds. Now that we solve turbofish before knowing what they'll be matched against, these are solved with Kind::Any. Then, once we know the target kind, any Kind::Any is replaced with the target kind.

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.

compiler/noirc_frontend/src/hir_def/types.rs

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>

github-actions

⚠️ 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: `74ce22d`	Previous: `c7ff3e7`	Ratio
`rollup-block-root`	`13.4` s	`11` s	`1.22`

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

CC: @TomAFrench

github-actions

⚠️ Performance Alert ⚠️

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

Benchmark suite	Current: `74ce22d`	Previous: `c7ff3e7`	Ratio
`rollup-block-root`	`189` s	`123` s	`1.54`

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

CC: @TomAFrench

github-actions

ACVM Benchmarks

Details

Benchmark suite	Current: `317a0f9`	Previous: `c7ff3e7`	Ratio
`purely_sequential_opcodes`	`272996` ns/iter (`± 920`)	`271439` ns/iter (`± 1328`)	`1.01`
`perfectly_parallel_opcodes`	`245300` ns/iter (`± 5259`)	`239984` ns/iter (`± 1806`)	`1.02`
`perfectly_parallel_batch_inversion_opcodes`	`3590465` ns/iter (`± 5105`)	`3224821` ns/iter (`± 23411`)	`1.11`

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

github-actions

Artifact Size

Details

Benchmark suite	Current: `317a0f9`	Previous: `c7ff3e7`	Ratio
`private-kernel-inner`	`1110.4` KB	`1110.4` KB	`1`
`private-kernel-reset`	`2033.9` KB	`2033.9` KB	`1`
`private-kernel-tail`	`638.3` KB	`638.3` KB	`1`
`rollup-base-private`	`5175.6` KB	`5175.6` KB	`1`
`rollup-base-public`	`3959` KB	`3959` KB	`1`
`rollup-block-root-empty`	`257.1` KB	`257.1` KB	`1`
`rollup-block-root-single-tx`	`25677.1` KB	`25677.1` KB	`1`
`rollup-block-root`	`25704` KB	`25704` KB	`1`
`rollup-merge`	`179.3` KB	`179.3` KB	`1`
`rollup-root`	`470.5` KB	`470.5` KB	`1`

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

github-actions

Compilation Time

Details

Benchmark suite	Current: `317a0f9`	Previous: `c7ff3e7`	Ratio
`private-kernel-inner`	`2.272` s	`2.392` s	`0.95`
`private-kernel-reset`	`6.424` s	`6.702` s	`0.96`
`private-kernel-tail`	`1.15` s	`1.214` s	`0.95`
`rollup-base-private`	`16.94` s	`17.44` s	`0.97`
`rollup-base-public`	`13.56` s	`13.46` s	`1.01`
`rollup-block-root-empty`	`1.232` s	`1.236` s	`1.00`
`rollup-block-root-single-tx`	`126` s	`124` s	`1.02`
`rollup-block-root`	`127` s	`123` s	`1.03`
`rollup-merge`	`1.06` s	`1.052` s	`1.01`
`rollup-root`	`1.576` s	`1.636` s	`0.96`

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

github-actions

Execution Time

Details

Benchmark suite	Current: `317a0f9`	Previous: `c7ff3e7`	Ratio
`private-kernel-inner`	`0.027` s	`0.028` s	`0.96`
`private-kernel-reset`	`0.162` s	`0.162` s	`1`
`private-kernel-tail`	`0.016` s	`0.016` s	`1`
`rollup-base-private`	`0.327` s	`0.329` s	`0.99`
`rollup-base-public`	`0.209` s	`0.209` s	`1`
`rollup-block-root`	`11.6` s	`11` s	`1.05`
`rollup-merge`	`0.004` s	`0.004` s	`1`
`rollup-root`	`0.013` s	`0.013` s	`1`

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

github-actions

Test Suite Duration

Details

Benchmark suite	Current: `317a0f9`	Previous: `c7ff3e7`	Ratio
`test_report_AztecProtocol_aztec-packages_noir-projects_aztec-nr`	`61` s	`64` s	`0.95`
`test_report_AztecProtocol_aztec-packages_noir-projects_noir-contracts`	`91` s	`96` s	`0.95`
`test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob`	`43` s	`44` s	`0.98`
`test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_private-kernel-lib`	`183` s	`183` s	`1`
`test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_rollup-lib`	`186` s	`182` s	`1.02`
`test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_types`	`57` s	`57` s	`1`
`test_report_noir-lang_noir-bignum_`	`404` s	`404` s	`1`
`test_report_noir-lang_noir_bigcurve_`	`250` s	`248` s	`1.01`
`test_report_noir-lang_sha512_`	`29` s	`31` s	`0.94`
`test_report_zkpassport_noir_rsa_`	`24` s	`22` s	`1.09`

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

github-actions

Execution Memory

Details

Benchmark suite	Current: `317a0f9`	Previous: `c7ff3e7`	Ratio
`private-kernel-inner`	`235.13` MB	`235.13` MB	`1`
`private-kernel-reset`	`257.61` MB	`257.61` MB	`1`
`private-kernel-tail`	`213.46` MB	`213.46` MB	`1`
`rollup-base-private`	`542.51` MB	`542.51` MB	`1`
`rollup-base-public`	`534.7` MB	`534.7` MB	`1`
`rollup-block-root`	`1440` MB	`1440` MB	`1`
`rollup-merge`	`363.67` MB	`363.67` MB	`1`
`rollup-root`	`369.59` MB	`369.59` MB	`1`

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

github-actions

Compilation Memory

Details

Benchmark suite	Current: `317a0f9`	Previous: `c7ff3e7`	Ratio
`private-kernel-inner`	`310.79` MB	`310.79` MB	`1`
`private-kernel-reset`	`566.27` MB	`566.27` MB	`1`
`private-kernel-tail`	`230.61` MB	`230.63` MB	`1.00`
`rollup-base-private`	`1440` MB	`1440` MB	`1`
`rollup-base-public`	`1450` MB	`1450` MB	`1`
`rollup-block-root-empty`	`395.9` MB	`395.91` MB	`1.00`
`rollup-block-root-single-tx`	`7890` MB	`7890` MB	`1`
`rollup-block-root`	`7890` MB	`7890` MB	`1`
`rollup-merge`	`377.94` MB	`377.96` MB	`1.00`
`rollup-root`	`438.89` MB	`438.89` MB	`1`

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

Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE fix: always type-check turbofish, and error when it's not allowed (noir-lang/noir#8437) chore: Release Noir(1.0.0-beta.5) (noir-lang/noir#7955) feat(greybox_fuzzer): Parallel fuzz tests (noir-lang/noir#8432) fix(ssa): Mislabeled instructions with side effects in EnableSideEffectsIf removal pass (noir-lang/noir#8355) feat: SSA pass impact report (noir-lang/noir#8393) chore: bump external pinned commits (noir-lang/noir#8433) chore: separate benchmarking from github actions more (noir-lang/noir#7943) chore(fuzz): Break up the AST fuzzer `compare` module (noir-lang/noir#8431) chore(fuzz): Rename `init_vs_final` to `min_vs_full` (noir-lang/noir#8430) fix!: error on tuple mismatch (noir-lang/noir#8424) chore: bump external pinned commits (noir-lang/noir#8429) chore(acir): Test whether the predicate has an effect on slice intrinsics (noir-lang/noir#8421) feat(ssa): Mark transitively dead parameters during DIE (noir-lang/noir#8254) fix(ssa_gen): Do not code gen fetching of empty arrays when initializing the data bus (noir-lang/noir#8426) chore: remove `.aztec-sync-commit` (noir-lang/noir#8415) chore(test): Add more unit tests for `inline_functions_with_at_most_one_instruction` (noir-lang/noir#8418) chore: add minor docs for interpreter (noir-lang/noir#8397) fix: print slice composite types surrounded by parentheses (noir-lang/noir#8412) feat: Skip SSA passes that contain any of the given messages (noir-lang/noir#8416) fix: disable range constraints using the predicate (noir-lang/noir#8396) chore: bumping external libraries (noir-lang/noir#8406) chore: redo typo PR by shystrui1199 (noir-lang/noir#8405) feat(test): add `nargo_fuzz_target` (noir-lang/noir#8308) fix: allow names to collide in the values/types namespaces (noir-lang/noir#8286) fix: Fix sequencing of side-effects in lvalue (noir-lang/noir#8384) feat(greybox_fuzzer): Maximum executions parameter added (noir-lang/noir#8390) fix: warn on and discard unreachable statements after break and continue (noir-lang/noir#8382) fix: add handling for u128 infix ops in interpreter (noir-lang/noir#8392) chore: move acirgen tests into separate file (noir-lang/noir#8376) feat(fuzz): initial version of comptime vs brillig target for AST fuzzer (noir-lang/noir#8335) chore: apply lints to `ast_fuzzer` (noir-lang/noir#8386) chore: add note on AI generated PRs in `CONTRIBUTING.md` (noir-lang/noir#8385) chore: document flattening pass (noir-lang/noir#8312) fix: comptime shift-right overflow is zero (noir-lang/noir#8380) feat: let static_assert accept any type for its message (noir-lang/noir#8322) fix(expand): output safety comment before statements (noir-lang/noir#8378) chore: avoid need to rebuild after running tests (noir-lang/noir#8379) chore: bump dependencies (noir-lang/noir#8372) chore: Add GITHUB_TOKEN to cross build (noir-lang/noir#8370) chore: redo typo PR by GarmashAlex (noir-lang/noir#8364) chore: remove unsafe code from greybox fuzzer (noir-lang/noir#8315) feat: add `--fuzz-timeout` to `nargo test` options (noir-lang/noir#8326) chore: bump external pinned commits (noir-lang/noir#8334) fix(expand): try to use "Self" in function calls (noir-lang/noir#8353) fix: Fix evaluation order of assignments with side-effects in their rhs (noir-lang/noir#8342) fix: let comptime Field value carry the field's sign (noir-lang/noir#8343) fix: Ordering of items in callstacks (noir-lang/noir#8338) chore: add snapshosts for nargo expand tests (noir-lang/noir#8318) fix(ownership): Clone global arrays (noir-lang/noir#8328) chore: Replace all SSA interpreter panics with error variants (noir-lang/noir#8311) feat: Metamorphic AST fuzzing (noir-lang/noir#8299) fix: fix some Display implementations for AST nodes (noir-lang/noir#8316) chore: remove leftover file (noir-lang/noir#8313) fix: uses non-zero points with ec-add-unsafe (noir-lang/noir#8248) END_COMMIT_OVERRIDE --------- Co-authored-by: AztecBot <tech@aztecprotocol.com> Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>

asterite added 13 commits May 9, 2025 11:51

Validate paths before resolving them (type-check turbofish)

ab603d9

Use Kind::Any when solving turbofish (the kind is checked later on)

3e2c4e4

clippy

b0891a4

Substitute Kind::Any

a654add

Docs

065d306

Ingnore nargo expand test related to enums

85852e8

Error on turbofish on global

25a6410

Use as_ident in one place

0ef1f32

No need for is_ident to be public anymore

a6d7d07

Error on local variable with turbofish

a88f7e1

Error on turbofish in enum match local variable pattern

ce48616

Error if there's turbofish after Self

3296f74

Error on turbofish on associated constant

baaf3bd

asterite changed the title ~~Ab/typed path~~ fix: always type-check turbofish, and error when it's not allowed May 9, 2025

Fix test output

09dfc24

TomAFrench approved these changes May 9, 2025

View reviewed changes

compiler/noirc_frontend/src/hir_def/types.rs Outdated Show resolved Hide resolved

Update compiler/noirc_frontend/src/hir_def/types.rs

74ce22d

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>

github-actions bot reviewed May 9, 2025

View reviewed changes

asterite added the bench-show Display benchmark results on PR label May 9, 2025

Merge branch 'master' into ab/typed-path

317a0f9

github-actions bot reviewed May 9, 2025

View reviewed changes

asterite enabled auto-merge May 9, 2025 19:32

asterite added this pull request to the merge queue May 9, 2025

Merged via the queue into master with commit 944d234 May 9, 2025
116 checks passed

asterite deleted the ab/typed-path branch May 9, 2025 19:51

noirwhal mentioned this pull request May 9, 2025

chore: Release Noir(1.0.0-beta.6) #8438

Merged

AztecBot mentioned this pull request May 10, 2025

chore: Bump Noir reference AztecProtocol/aztec-packages#14055

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: always type-check turbofish, and error when it's not allowed#8437

fix: always type-check turbofish, and error when it's not allowed#8437
asterite merged 16 commits intomasterfrom
ab/typed-path

asterite commented May 9, 2025 •

edited

Loading

Uh oh!

Uh oh!

github-actions bot left a comment

Uh oh!

github-actions bot left a comment

Uh oh!

github-actions bot left a comment

Uh oh!

github-actions bot left a comment

Uh oh!

github-actions bot left a comment

Uh oh!

github-actions bot left a comment

Uh oh!

github-actions bot left a comment

Uh oh!

github-actions bot left a comment

Uh oh!

github-actions bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

asterite commented May 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Problem

Summary

Additional Context

Documentation

PR Checklist

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

ACVM Benchmarks

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Artifact Size

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Compilation Time

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Execution Time

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Test Suite Duration

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Execution Memory

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Compilation Memory

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

asterite commented May 9, 2025 •

edited

Loading