feat: support argument names by ilbertt · Pull Request #612 · dfinity/candid

ilbertt · 2025-06-10T07:50:47Z

Overview
Adds support for named arguments: if a .did Candid declaration file declares functions, classes or services with named arguments, those names are parsed by the candid-parser and preserved in the generated bindings.
At the same time, the names of the arguments are preserved if the Candid declaration file is generated from a Rust canister.

Requirements
To consider this feature complete, the Motoko compiler should preserve the actor methods names from Motoko canisters to the Candid declaration files.

Considerations
This change should be backwards compatible.

Picked up from https://github.com/ilbertt/candid/tree/luca/arg-names

luc-blaeser

Very nice PR. Just a few little comments, mostly just questions for my own understanding. Thank you, Luca!

luc-blaeser

Thanks for following up. Ready to merge IMO

ilbertt · 2025-06-10T12:04:11Z

Let's wait for the fixes on the bench tests before merging this PR, cc @mraszyk

github-actions · 2025-06-10T16:18:06Z

Name	Max Mem (Kb)	Encode	Decode
blob	4_224	20_459_344 ($\textcolor{red}{0.00\%}$)	12_083_680 ($\textcolor{green}{-0.00\%}$)
btreemap	75_456	4_641_549_583 ($\textcolor{red}{0.00\%}$)	15_320_117_359 ($\textcolor{green}{-0.00\%}$)
nns	128	2_094_411 ($\textcolor{green}{-0.01\%}$)	5_652_196 ($\textcolor{red}{0.21\%}$)
nns_list_proposal	1_088	7_777_860 ($\textcolor{red}{0.26\%}$)	80_650_009 ($\textcolor{green}{-0.01\%}$)
option_list	128	7_909_886 ($\textcolor{red}{0.01\%}$)	26_307_108 ($\textcolor{red}{0.72\%}$)
text	6_336	20_456_583 ($\textcolor{red}{0.00\%}$)	17_315_582 ($\textcolor{green}{-0.00\%}$)
variant_list	128	7_962_587 ($\textcolor{red}{0.00\%}$)	24_569_887 ($\textcolor{green}{-0.00\%}$)
vec_int16	16_704	168_584_944 ($\textcolor{red}{0.00\%}$)	1_096_834_739 ($\textcolor{green}{-0.00\%}$)

Parser cost: 17_722_077 ($\textcolor{red}{0.01\%}$)
Extra args: 3_253_907 ($\textcolor{green}{-0.03\%}$)

Click to see raw report

---------------------------------------------------

Benchmark: blob
  total:
    instructions: 32.55 M (-0.00%) (change within noise threshold)
    heap_increase: 66 pages (no change)
    stable_memory_increase: 0 pages (no change)

  1. Encoding (scope):
    calls: 1 (no change)
    instructions: 20.46 M (0.00%) (change within noise threshold)
    heap_increase: 66 pages (no change)
    stable_memory_increase: 0 pages (no change)

  2. Decoding (scope):
    calls: 1 (no change)
    instructions: 12.08 M (-0.00%) (change within noise threshold)
    heap_increase: 0 pages (no change)
    stable_memory_increase: 0 pages (no change)

---------------------------------------------------

Benchmark: text
  total:
    instructions: 37.77 M (-0.00%) (change within noise threshold)
    heap_increase: 99 pages (no change)
    stable_memory_increase: 0 pages (no change)

  1. Encoding (scope):
    calls: 1 (no change)
    instructions: 20.46 M (0.00%) (change within noise threshold)
    heap_increase: 66 pages (no change)
    stable_memory_increase: 0 pages (no change)

  2. Decoding (scope):
    calls: 1 (no change)
    instructions: 17.32 M (-0.00%) (change within noise threshold)
    heap_increase: 33 pages (no change)
    stable_memory_increase: 0 pages (no change)

---------------------------------------------------

Benchmark: vec_int16
  total:
    instructions: 1.27 B (-0.00%) (change within noise threshold)
    heap_increase: 261 pages (no change)
    stable_memory_increase: 0 pages (no change)

  1. Encoding (scope):
    calls: 1 (no change)
    instructions: 168.58 M (0.00%) (change within noise threshold)
    heap_increase: 261 pages (no change)
    stable_memory_increase: 0 pages (no change)

  2. Decoding (scope):
    calls: 1 (no change)
    instructions: 1.10 B (-0.00%) (change within noise threshold)
    heap_increase: 0 pages (no change)
    stable_memory_increase: 0 pages (no change)

---------------------------------------------------

Benchmark: btreemap
  total:
    instructions: 19.96 B (0.00%) (change within noise threshold)
    heap_increase: 1179 pages (no change)
    stable_memory_increase: 0 pages (no change)

  1. Encoding (scope):
    calls: 1 (no change)
    instructions: 4.64 B (0.00%) (change within noise threshold)
    heap_increase: 159 pages (no change)
    stable_memory_increase: 0 pages (no change)

  2. Decoding (scope):
    calls: 1 (no change)
    instructions: 15.32 B (-0.00%) (change within noise threshold)
    heap_increase: 1020 pages (no change)
    stable_memory_increase: 0 pages (no change)

---------------------------------------------------

Benchmark: option_list
  total:
    instructions: 34.22 M (0.56%) (change within noise threshold)
    heap_increase: 2 pages (no change)
    stable_memory_increase: 0 pages (no change)

  1. Encoding (scope):
    calls: 1 (no change)
    instructions: 7.91 M (0.01%) (change within noise threshold)
    heap_increase: 0 pages (no change)
    stable_memory_increase: 0 pages (no change)

  2. Decoding (scope):
    calls: 1 (no change)
    instructions: 26.31 M (0.72%) (change within noise threshold)
    heap_increase: 2 pages (no change)
    stable_memory_increase: 0 pages (no change)

---------------------------------------------------

Benchmark: variant_list
  total:
    instructions: 32.53 M (-0.00%) (change within noise threshold)
    heap_increase: 2 pages (no change)
    stable_memory_increase: 0 pages (no change)

  1. Encoding (scope):
    calls: 1 (no change)
    instructions: 7.96 M (0.00%) (change within noise threshold)
    heap_increase: 0 pages (no change)
    stable_memory_increase: 0 pages (no change)

  2. Decoding (scope):
    calls: 1 (no change)
    instructions: 24.57 M (-0.00%) (change within noise threshold)
    heap_increase: 2 pages (no change)
    stable_memory_increase: 0 pages (no change)

---------------------------------------------------

Benchmark: nns
  total:
    instructions: 26.30 M (0.05%) (change within noise threshold)
    heap_increase: 2 pages (no change)
    stable_memory_increase: 0 pages (no change)

  0. Parsing (scope):
    calls: 1 (no change)
    instructions: 17.72 M (0.01%) (change within noise threshold)
    heap_increase: 2 pages (no change)
    stable_memory_increase: 0 pages (no change)

  1. Encoding (scope):
    calls: 1 (no change)
    instructions: 2.09 M (-0.01%) (change within noise threshold)
    heap_increase: 0 pages (no change)
    stable_memory_increase: 0 pages (no change)

  2. Decoding (scope):
    calls: 1 (no change)
    instructions: 5.65 M (0.21%) (change within noise threshold)
    heap_increase: 0 pages (no change)
    stable_memory_increase: 0 pages (no change)

---------------------------------------------------

Benchmark: nns_list_proposal
  total:
    instructions: 88.43 M (0.02%) (change within noise threshold)
    heap_increase: 17 pages (no change)
    stable_memory_increase: 0 pages (no change)

  1. Encoding (scope):
    calls: 1 (no change)
    instructions: 7.78 M (0.26%) (change within noise threshold)
    heap_increase: 3 pages (no change)
    stable_memory_increase: 0 pages (no change)

  2. Decoding (scope):
    calls: 1 (no change)
    instructions: 80.65 M (-0.01%) (change within noise threshold)
    heap_increase: 14 pages (no change)
    stable_memory_increase: 0 pages (no change)

---------------------------------------------------

Benchmark: extra_args
  total:
    instructions: 3.25 M (-0.03%) (change within noise threshold)
    heap_increase: 0 pages (no change)
    stable_memory_increase: 0 pages (no change)

---------------------------------------------------

Summary:
  instructions:
    status:   No significant changes 👍
    counts:   [total 9 | regressed 0 | improved 0 | new 0 | unchanged 9]
    change:   [max +189.35K | p75 +12.92K | median -34 | p25 -64 | min -1.12K]
    change %: [max +0.56% | p75 +0.02% | median -0.00% | p25 -0.00% | min -0.03%]

  heap_increase:
    status:   No significant changes 👍
    counts:   [total 9 | regressed 0 | improved 0 | new 0 | unchanged 9]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  stable_memory_increase:
    status:   No significant changes 👍
    counts:   [total 9 | regressed 0 | improved 0 | new 0 | unchanged 9]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

---------------------------------------------------
Successfully persisted results to canbench_results.yml

luc-blaeser · 2025-06-11T13:06:06Z

+    let args = <>;
+    let mut named_args: Vec<String> = args.iter().filter_map(|a| a.name.clone()).collect();
+    named_args.sort();
+    check_unique(named_args.iter()).map_err(|e| error(e))?;


Just a note for future refactoring, nothing to fix here: Probably the function check_unique should be renamed as it requires sorted input. One could assume that a general unique check works generally on unsorted input.

luc-blaeser

Nice adjustments, good catch of numbered args case.

…ate (#652) This PR refactors recent changes (#612 and #640) to avoid a major version bump in `candid` crate. > * Breaking changes: > + `pp_args` and `pp_init_args` now require a `&[ArgType]` parameter. The `pp_rets` function has been added, with the signature of the old `pp_args`. > + `pretty::candid::pp_label` now takes a `&Label` parameter, instead of a `&SharedLabel`. The previously altered API (`pp_args`, `pp_init_args`, `pp_label`) has been restored to maintain backward compatibility. The intended new behavior is now exposed via new, non-breaking methods: `pp_named_args`, `pp_named_init_args` and `pp_label_raw`.

**Overview** Revert the changes made in #612 to remove the breaking changes.

This reverts commit 43160d7.

Named arguments can also be in the return type, and can be ignored. This PR restores that was changed in https://github.com/dfinity/candid/pull/612/files#diff-c5ff9610c5d6538ec79f3d465965d2b3a3e3b628b16854a91b4b4a90ba766a57L220-L230, in order to keep parsing the return types with arguments without throwing. Updates a test case to check if the behavior has been restored.

ilbertt added 2 commits June 8, 2025 14:04

feat: preserve arg names

db73dd5

test: add init arg case

d23784c

ilbertt requested a review from a team as a code owner June 10, 2025 07:50

luc-blaeser reviewed Jun 10, 2025

View reviewed changes

Comment thread rust/candid/src/types/internal.rs Outdated

luc-blaeser reviewed Jun 10, 2025

View reviewed changes

Comment thread rust/candid/src/pretty/candid.rs Outdated

luc-blaeser reviewed Jun 10, 2025

View reviewed changes

Comment thread rust/candid_parser/src/bindings/motoko.rs Outdated

luc-blaeser approved these changes Jun 10, 2025

View reviewed changes

ilbertt added 3 commits June 10, 2025 10:35

fix: count for " : " only if there's an arg name

5926191

refactor: use if let Some pattern for options

fd0660c

style: fix lint

99fd7f2

ilbertt requested a review from luc-blaeser June 10, 2025 08:58

perf: fix benches

34690f5

luc-blaeser approved these changes Jun 10, 2025

View reviewed changes

chore: update changelog

1467059

fix: avoid numeric arguments

a29f462

ilbertt requested a review from luc-blaeser June 11, 2025 08:31

ilbertt added 2 commits June 11, 2025 14:29

fix: check for unique named arguments

ef8e00b

test: fix syntax

02ffdfe

luc-blaeser reviewed Jun 11, 2025

View reviewed changes

Comment thread rust/candid_parser/src/types.rs Outdated

luc-blaeser reviewed Jun 11, 2025

View reviewed changes

Comment thread rust/candid_parser/tests/assets/ok/fieldnat.did Outdated

luc-blaeser approved these changes Jun 11, 2025

View reviewed changes

ilbertt added 4 commits June 11, 2025 18:47

refactor: parse name before constructing self

2a2903b

docs: update new_with_name comment

554f39c

Merge remote-tracking branch 'origin/master' into luca/support-arg-names

ac34dc3

Merge remote-tracking branch 'origin/master' into luca/support-arg-names

4ce4c1d

ilbertt merged commit 61a49c8 into master Jun 11, 2025
9 checks passed

ilbertt deleted the luca/support-arg-names branch June 11, 2025 17:57

lwshang mentioned this pull request Jul 25, 2025

feat: introduce new methods to avoid breaking API change in candid crate #652

Merged

ilbertt added a commit that referenced this pull request Jul 28, 2025

refactor: revert #612

1d075c2

ilbertt mentioned this pull request Jul 28, 2025

refactor: revert #612 #660

Merged

ilbertt added a commit that referenced this pull request Jul 29, 2025

refactor: revert #612 (#660)

43160d7

**Overview** Revert the changes made in #612 to remove the breaking changes.

ilbertt added a commit that referenced this pull request Jul 29, 2025

Revert "refactor: revert #612 (#660)"

af44cfb

This reverts commit 43160d7.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: support argument names#612

feat: support argument names#612
ilbertt merged 14 commits into
masterfrom
luca/support-arg-names

ilbertt commented Jun 10, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

luc-blaeser left a comment

Uh oh!

luc-blaeser left a comment

Uh oh!

ilbertt commented Jun 10, 2025

Uh oh!

github-actions Bot commented Jun 10, 2025 •

edited

Loading

Uh oh!

luc-blaeser Jun 11, 2025

Uh oh!

Uh oh!

Uh oh!

luc-blaeser left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

ilbertt commented Jun 10, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

luc-blaeser left a comment

Choose a reason for hiding this comment

Uh oh!

luc-blaeser left a comment

Choose a reason for hiding this comment

Uh oh!

ilbertt commented Jun 10, 2025

Uh oh!

github-actions Bot commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

luc-blaeser Jun 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

luc-blaeser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Jun 10, 2025 •

edited

Loading