fix(lsp): suggest all possible trait methods, but only visible ones #7027

asterite · 2025-01-10T19:29:02Z

Description

Problem

Towards #6956

Summary

I noticed a couple of issues in how we suggest trait methods in LSP:

We don't show all of the trait methods if more than one could be called (only one, the first one). That is, if there are multiple traits with the same method name and none is currently imported, all of those should be suggested.
Non-visible trait methods were suggested

Solving these two things required some changes on the compiler side, outside of just LSP, and to avoid eventually creating a giant PR that solves all of #6956 I decided to split just this into a smaller PR.

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.

…Type>`

github-actions · 2025-01-10T19:35:24Z

Execution Memory Report

Program	Peak Memory
keccak256	74.67M
workspace	123.90M
regression_4709	316.00M
ram_blowup_regression	512.61M
rollup-base-public	479.46M
rollup-base-private	325.86M
private-kernel-tail	180.43M
private-kernel-reset	245.32M
private-kernel-inner	208.69M

github-actions · 2025-01-10T19:35:24Z

Compilation Memory Report

Program	Peak Memory
keccak256	78.13M
workspace	123.82M
regression_4709	424.09M
ram_blowup_regression	1.58G
rollup-base-public	4.85G
rollup-base-private	1.26G
private-kernel-tail	207.14M
private-kernel-reset	669.12M
private-kernel-inner	294.38M

github-actions · 2025-01-10T19:36:19Z

Execution Report

Program	Execution Time	%
sha256_regression	0.052s	1%
regression_4709	0.001s	0%
ram_blowup_regression	0.631s	0%
rollup-root	0.104s	-1%
rollup-block-merge	0.104s	-2%
rollup-base-public	1.226s	-1%
rollup-base-private	0.458s	0%
private-kernel-tail	0.019s	-6%
private-kernel-reset	0.315s	-2%
private-kernel-inner	0.071s	0%

github-actions · 2025-01-10T19:36:20Z

Compilation Report

Program	Compilation Time	%
sha256_regression	1.244s	-3%
regression_4709	0.781s	-3%
ram_blowup_regression	17.980s	-1%
rollup-root	3.510s	-2%
rollup-block-merge	3.448s	-2%
rollup-base-public	41.420s	-5%
rollup-base-private	13.540s	1%
private-kernel-tail	0.959s	-10%
private-kernel-reset	6.878s	-3%
private-kernel-inner	2.016s	-5%

compiler/noirc_frontend/src/elaborator/mod.rs

compiler/noirc_frontend/src/hir_def/traits.rs

Co-authored-by: jfecher <jake@aztecprotocol.com>

…oir-lang/noir#7037) feat: avoid inserting `inc_rc` instructions into ACIR (noir-lang/noir#7036) fix(lsp): suggest all possible trait methods, but only visible ones (noir-lang/noir#7027) chore: add more protocol circuits to reports (noir-lang/noir#6977) feat: avoid generating a new witness when checking if linear expression is zero (noir-lang/noir#7031) feat: skip codegen of zero iteration loops (noir-lang/noir#7030)

…ake averages with single runs (noir-lang/noir#7048) fix(nargo_fmt): let doc comment could come after regular comment (noir-lang/noir#7046) fix(nargo_fmt): don't consider identifiers the same if they are equal… (noir-lang/noir#7043) feat: auto-import traits when suggesting trait methods (noir-lang/noir#7037) feat: avoid inserting `inc_rc` instructions into ACIR (noir-lang/noir#7036) fix(lsp): suggest all possible trait methods, but only visible ones (noir-lang/noir#7027) chore: add more protocol circuits to reports (noir-lang/noir#6977) feat: avoid generating a new witness when checking if linear expression is zero (noir-lang/noir#7031) feat: skip codegen of zero iteration loops (noir-lang/noir#7030)

… with single runs (noir-lang/noir#7048) fix(nargo_fmt): let doc comment could come after regular comment (noir-lang/noir#7046) fix(nargo_fmt): don't consider identifiers the same if they are equal… (noir-lang/noir#7043) feat: auto-import traits when suggesting trait methods (noir-lang/noir#7037) feat: avoid inserting `inc_rc` instructions into ACIR (noir-lang/noir#7036) fix(lsp): suggest all possible trait methods, but only visible ones (noir-lang/noir#7027) chore: add more protocol circuits to reports (noir-lang/noir#6977) feat: avoid generating a new witness when checking if linear expression is zero (noir-lang/noir#7031) feat: skip codegen of zero iteration loops (noir-lang/noir#7030)

…oir-lang/noir#7066) feat!: Handle generic fields in `StructDefinition::fields` and move old functionality to `StructDefinition::fields_as_written` (noir-lang/noir#7067) chore(ci): Check various inliner aggressiveness setttings in Brillig reports (noir-lang/noir#7049) chore: reenable reports on rollup root circuits (noir-lang/noir#7061) fix: don't always select trait impl when verifying trait constraints (noir-lang/noir#7041) chore: mark some critical libraries as good again (noir-lang/noir#7065) fix: Reduce memory usage in mem2reg (noir-lang/noir#7053) feat: Allow associated types to be ellided from trait constraints (noir-lang/noir#7026) chore(perf): try using vec-collections's VecSet in AliasSet (noir-lang/noir#7058) chore: reduce number of iterations of `acvm::compiler::compile` (noir-lang/noir#7050) chore: add `noir_check_shuffle` as a critical library (noir-lang/noir#7056) chore: clippy warning fix (noir-lang/noir#7051) chore(ci): Unify compilation/execution report jobs that take averages with single runs (noir-lang/noir#7048) fix(nargo_fmt): let doc comment could come after regular comment (noir-lang/noir#7046) fix(nargo_fmt): don't consider identifiers the same if they are equal… (noir-lang/noir#7043) feat: auto-import traits when suggesting trait methods (noir-lang/noir#7037) feat: avoid inserting `inc_rc` instructions into ACIR (noir-lang/noir#7036) fix(lsp): suggest all possible trait methods, but only visible ones (noir-lang/noir#7027) chore: add more protocol circuits to reports (noir-lang/noir#6977) feat: avoid generating a new witness when checking if linear expression is zero (noir-lang/noir#7031) feat: skip codegen of zero iteration loops (noir-lang/noir#7030)

…ir#7066) feat!: Handle generic fields in `StructDefinition::fields` and move old functionality to `StructDefinition::fields_as_written` (noir-lang/noir#7067) chore(ci): Check various inliner aggressiveness setttings in Brillig reports (noir-lang/noir#7049) chore: reenable reports on rollup root circuits (noir-lang/noir#7061) fix: don't always select trait impl when verifying trait constraints (noir-lang/noir#7041) chore: mark some critical libraries as good again (noir-lang/noir#7065) fix: Reduce memory usage in mem2reg (noir-lang/noir#7053) feat: Allow associated types to be ellided from trait constraints (noir-lang/noir#7026) chore(perf): try using vec-collections's VecSet in AliasSet (noir-lang/noir#7058) chore: reduce number of iterations of `acvm::compiler::compile` (noir-lang/noir#7050) chore: add `noir_check_shuffle` as a critical library (noir-lang/noir#7056) chore: clippy warning fix (noir-lang/noir#7051) chore(ci): Unify compilation/execution report jobs that take averages with single runs (noir-lang/noir#7048) fix(nargo_fmt): let doc comment could come after regular comment (noir-lang/noir#7046) fix(nargo_fmt): don't consider identifiers the same if they are equal… (noir-lang/noir#7043) feat: auto-import traits when suggesting trait methods (noir-lang/noir#7037) feat: avoid inserting `inc_rc` instructions into ACIR (noir-lang/noir#7036) fix(lsp): suggest all possible trait methods, but only visible ones (noir-lang/noir#7027) chore: add more protocol circuits to reports (noir-lang/noir#6977) feat: avoid generating a new witness when checking if linear expression is zero (noir-lang/noir#7031) feat: skip codegen of zero iteration loops (noir-lang/noir#7030)

…ir#7070) feat(LSP): code action to import trait in a method call (noir-lang/noir#7066) feat!: Handle generic fields in `StructDefinition::fields` and move old functionality to `StructDefinition::fields_as_written` (noir-lang/noir#7067) chore(ci): Check various inliner aggressiveness setttings in Brillig reports (noir-lang/noir#7049) chore: reenable reports on rollup root circuits (noir-lang/noir#7061) fix: don't always select trait impl when verifying trait constraints (noir-lang/noir#7041) chore: mark some critical libraries as good again (noir-lang/noir#7065) fix: Reduce memory usage in mem2reg (noir-lang/noir#7053) feat: Allow associated types to be ellided from trait constraints (noir-lang/noir#7026) chore(perf): try using vec-collections's VecSet in AliasSet (noir-lang/noir#7058) chore: reduce number of iterations of `acvm::compiler::compile` (noir-lang/noir#7050) chore: add `noir_check_shuffle` as a critical library (noir-lang/noir#7056) chore: clippy warning fix (noir-lang/noir#7051) chore(ci): Unify compilation/execution report jobs that take averages with single runs (noir-lang/noir#7048) fix(nargo_fmt): let doc comment could come after regular comment (noir-lang/noir#7046) fix(nargo_fmt): don't consider identifiers the same if they are equal… (noir-lang/noir#7043) feat: auto-import traits when suggesting trait methods (noir-lang/noir#7037) feat: avoid inserting `inc_rc` instructions into ACIR (noir-lang/noir#7036) fix(lsp): suggest all possible trait methods, but only visible ones (noir-lang/noir#7027) chore: add more protocol circuits to reports (noir-lang/noir#6977) feat: avoid generating a new witness when checking if linear expression is zero (noir-lang/noir#7031) feat: skip codegen of zero iteration loops (noir-lang/noir#7030)

feat(LSP): code action to import trait in a method call (noir-lang/noir#7066) feat!: Handle generic fields in `StructDefinition::fields` and move old functionality to `StructDefinition::fields_as_written` (noir-lang/noir#7067) chore(ci): Check various inliner aggressiveness setttings in Brillig reports (noir-lang/noir#7049) chore: reenable reports on rollup root circuits (noir-lang/noir#7061) fix: don't always select trait impl when verifying trait constraints (noir-lang/noir#7041) chore: mark some critical libraries as good again (noir-lang/noir#7065) fix: Reduce memory usage in mem2reg (noir-lang/noir#7053) feat: Allow associated types to be ellided from trait constraints (noir-lang/noir#7026) chore(perf): try using vec-collections's VecSet in AliasSet (noir-lang/noir#7058) chore: reduce number of iterations of `acvm::compiler::compile` (noir-lang/noir#7050) chore: add `noir_check_shuffle` as a critical library (noir-lang/noir#7056) chore: clippy warning fix (noir-lang/noir#7051) chore(ci): Unify compilation/execution report jobs that take averages with single runs (noir-lang/noir#7048) fix(nargo_fmt): let doc comment could come after regular comment (noir-lang/noir#7046) fix(nargo_fmt): don't consider identifiers the same if they are equal… (noir-lang/noir#7043) feat: auto-import traits when suggesting trait methods (noir-lang/noir#7037) feat: avoid inserting `inc_rc` instructions into ACIR (noir-lang/noir#7036) fix(lsp): suggest all possible trait methods, but only visible ones (noir-lang/noir#7027) chore: add more protocol circuits to reports (noir-lang/noir#6977) feat: avoid generating a new witness when checking if linear expression is zero (noir-lang/noir#7031) feat: skip codegen of zero iteration loops (noir-lang/noir#7030)

Automated pull of development from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE chore: add end step for formatting workflow (noir-lang/noir#7070) feat(LSP): code action to import trait in a method call (noir-lang/noir#7066) feat!: Handle generic fields in `StructDefinition::fields` and move old functionality to `StructDefinition::fields_as_written` (noir-lang/noir#7067) chore(ci): Check various inliner aggressiveness setttings in Brillig reports (noir-lang/noir#7049) chore: reenable reports on rollup root circuits (noir-lang/noir#7061) fix: don't always select trait impl when verifying trait constraints (noir-lang/noir#7041) chore: mark some critical libraries as good again (noir-lang/noir#7065) fix: Reduce memory usage in mem2reg (noir-lang/noir#7053) feat: Allow associated types to be ellided from trait constraints (noir-lang/noir#7026) chore(perf): try using vec-collections's VecSet in AliasSet (noir-lang/noir#7058) chore: reduce number of iterations of `acvm::compiler::compile` (noir-lang/noir#7050) chore: add `noir_check_shuffle` as a critical library (noir-lang/noir#7056) chore: clippy warning fix (noir-lang/noir#7051) chore(ci): Unify compilation/execution report jobs that take averages with single runs (noir-lang/noir#7048) fix(nargo_fmt): let doc comment could come after regular comment (noir-lang/noir#7046) fix(nargo_fmt): don't consider identifiers the same if they are equal… (noir-lang/noir#7043) feat: auto-import traits when suggesting trait methods (noir-lang/noir#7037) feat: avoid inserting `inc_rc` instructions into ACIR (noir-lang/noir#7036) fix(lsp): suggest all possible trait methods, but only visible ones (noir-lang/noir#7027) chore: add more protocol circuits to reports (noir-lang/noir#6977) feat: avoid generating a new witness when checking if linear expression is zero (noir-lang/noir#7031) feat: skip codegen of zero iteration loops (noir-lang/noir#7030) END_COMMIT_OVERRIDE --------- Co-authored-by: Tom French <tom@tomfren.ch> Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> Co-authored-by: Jake Fecher <jfecher11@gmail.com>

asterite added 8 commits January 10, 2025 15:26

Change Methods::iter to return Option<&Type> instead of `&Option<…

d3e098c

…Type>`

Let Methods::iter() also return Option<TraitId>

8c31ade

No need to have mutating methods for public fields

8ba91b9

Correctly track trait impl method visibility

9661486

Fix small typo

7a2bd68

Suggest all possible trait functions, but only visible ones

ad9ca2f

Don't show visibility for trait and trait impl functions

1df763c

Test that multiple trait methods are suggested

c65b63a

Set trait impl methods visibility before elaborating functions

6202c67

asterite requested a review from a team January 10, 2025 20:20

jfecher reviewed Jan 10, 2025

View reviewed changes

compiler/noirc_frontend/src/elaborator/mod.rs Outdated Show resolved Hide resolved

compiler/noirc_frontend/src/elaborator/mod.rs Outdated Show resolved Hide resolved

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

asterite and others added 4 commits January 10, 2025 19:19

Update compiler/noirc_frontend/src/elaborator/mod.rs

6ee2c24

Co-authored-by: jfecher <jake@aztecprotocol.com>

fix

b353e08

Use mutable methods again

1b990fc

Set visibility in existing loop

12cee2e

jfecher approved these changes Jan 13, 2025

View reviewed changes

asterite added this pull request to the merge queue Jan 13, 2025

Merged via the queue into master with commit 7d46287 Jan 13, 2025
87 checks passed

asterite deleted the ab/lsp-non-visible-trait-methods branch January 13, 2025 15:11

noirwhal mentioned this pull request Jan 13, 2025

chore: Release Noir(1.0.0-beta.2) #6914

Open

AztecBot mentioned this pull request Jan 13, 2025

feat: Sync from noir AztecProtocol/aztec-packages#11196

Merged

AztecBot mentioned this pull request Jan 15, 2025

chore(master): Release 0.70.0 AztecProtocol/aztec-packages#11107

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(lsp): suggest all possible trait methods, but only visible ones #7027

fix(lsp): suggest all possible trait methods, but only visible ones #7027

asterite commented Jan 10, 2025

github-actions bot commented Jan 10, 2025 •

edited

Loading

github-actions bot commented Jan 10, 2025 •

edited

Loading

github-actions bot commented Jan 10, 2025 •

edited

Loading

github-actions bot commented Jan 10, 2025 •

edited

Loading

fix(lsp): suggest all possible trait methods, but only visible ones #7027

fix(lsp): suggest all possible trait methods, but only visible ones #7027

Conversation

asterite commented Jan 10, 2025

Description

Problem

Summary

Additional Context

Documentation

PR Checklist

github-actions bot commented Jan 10, 2025 • edited Loading

Execution Memory Report

github-actions bot commented Jan 10, 2025 • edited Loading

Compilation Memory Report

github-actions bot commented Jan 10, 2025 • edited Loading

Execution Report

github-actions bot commented Jan 10, 2025 • edited Loading

Compilation Report

github-actions bot commented Jan 10, 2025 •

edited

Loading

github-actions bot commented Jan 10, 2025 •

edited

Loading

github-actions bot commented Jan 10, 2025 •

edited

Loading

github-actions bot commented Jan 10, 2025 •

edited

Loading