Skip to content

feat!: collect names for return types in parser#684

Merged
ilbertt merged 6 commits intonextfrom
luca/fix-named-rets
Oct 9, 2025
Merged

feat!: collect names for return types in parser#684
ilbertt merged 6 commits intonextfrom
luca/fix-named-rets

Conversation

@ilbertt
Copy link
Copy Markdown
Contributor

@ilbertt ilbertt commented Oct 9, 2025

Overview
Named arguments can also be in the return type according to the spec. This PR updates the grammar to collect the names of the return types.

As a consequence, it fixes a regression introduced in https://github.com/dfinity/candid/pull/612/files#diff-c5ff9610c5d6538ec79f3d465965d2b3a3e3b628b16854a91b4b4a90ba766a57L220-L230, in order to keep parsing the return types with arguments without errors.

Updates a test case to check if the behavior has been restored.

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 ilbertt requested a review from a team as a code owner October 9, 2025 14:03
@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 9, 2025

Name Max Mem (Kb) Encode Decode
blob 4_224 4_208_077 ($\textcolor{red}{0.00\%}$) 2_120_230 ($\textcolor{red}{0.00\%}$)
btreemap 75_456 4_701_848_211 ($\textcolor{red}{0.00\%}$) 15_580_167_369 ($\textcolor{green}{-0.24\%}$)
nns 192 1_628_223 ($\textcolor{red}{1.16\%}$) 3_354_727 ($\textcolor{red}{0.21\%}$)
nns_list_proposal 1_088 6_570_615 ($\textcolor{green}{-0.09\%}$) 47_649_447 ($\textcolor{red}{0.07\%}$)
option_list 128 8_026_733 ($\textcolor{green}{-0.00\%}$) 25_896_825 ($\textcolor{green}{-0.60\%}$)
text 6_336 4_204_221 7_877_878 ($\textcolor{red}{0.00\%}$)
variant_list 128 8_021_720 ($\textcolor{red}{0.00\%}$) 24_231_303 ($\textcolor{green}{-0.32\%}$)
vec_int16 16_704 123_695_548 ($\textcolor{red}{0.00\%}$) 950_032_334 ($\textcolor{red}{0.00\%}$)
  • Parser cost: 17_168_623 ($\textcolor{red}{0.09\%}$)
  • Extra args: 2_522_653 ($\textcolor{red}{0.01\%}$)
Click to see raw report
---------------------------------------------------

Benchmark: blob
  total:
    instructions: 6.33 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: 4.21 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: 2.12 M (0.00%) (change within noise threshold)
    heap_increase: 0 pages (no change)
    stable_memory_increase: 0 pages (no change)

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

Benchmark: text
  total:
    instructions: 12.08 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: 4.20 M (no change)
    heap_increase: 66 pages (no change)
    stable_memory_increase: 0 pages (no change)

  2. Decoding (scope):
    calls: 1 (no change)
    instructions: 7.88 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.07 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: 123.70 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: 950.03 M (0.00%) (change within noise threshold)
    heap_increase: 0 pages (no change)
    stable_memory_increase: 0 pages (no change)

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

Benchmark: btreemap
  total:
    instructions: 20.28 B (-0.18%) (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.70 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.58 B (-0.24%) (change within noise threshold)
    heap_increase: 1020 pages (no change)
    stable_memory_increase: 0 pages (no change)

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

Benchmark: option_list
  total:
    instructions: 33.93 M (-0.46%) (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: 8.03 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: 25.90 M (-0.60%) (change within noise threshold)
    heap_increase: 2 pages (no change)
    stable_memory_increase: 0 pages (no change)

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

Benchmark: variant_list
  total:
    instructions: 32.26 M (-0.24%) (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: 8.02 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.23 M (-0.32%) (change within noise threshold)
    heap_increase: 2 pages (no change)
    stable_memory_increase: 0 pages (no change)

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

Benchmark: nns
  total:
    instructions: 23.10 M (0.18%) (change within noise threshold)
    heap_increase: 3 pages (no change)
    stable_memory_increase: 0 pages (no change)

  0. Parsing (scope):
    calls: 1 (no change)
    instructions: 17.17 M (0.09%) (change within noise threshold)
    heap_increase: 3 pages (no change)
    stable_memory_increase: 0 pages (no change)

  1. Encoding (scope):
    calls: 1 (no change)
    instructions: 1.63 M (1.16%) (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: 3.35 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: 54.22 M (0.05%) (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: 6.57 M (-0.09%) (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: 47.65 M (0.07%) (change within noise threshold)
    heap_increase: 14 pages (no change)
    stable_memory_increase: 0 pages (no change)

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

Benchmark: extra_args
  total:
    instructions: 2.52 M (0.01%) (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 +40.39K | p75 +246 | median +153 | p25 -77.61K | min -36.70M]
    change %: [max +0.18% | p75 0.01% | median 0.00% | p25 -0.18% | min -0.46%]

  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

@ilbertt ilbertt changed the title fix: restore parsing behavior changed in #612 feat!: collect names for return types in parser Oct 9, 2025
@ilbertt ilbertt merged commit 7773117 into next Oct 9, 2025
6 checks passed
@ilbertt ilbertt deleted the luca/fix-named-rets branch October 9, 2025 17:08
ilbertt added a commit to dfinity/icp-js-bindgen that referenced this pull request Oct 9, 2025
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.

2 participants