Skip to content

Conversation

@TomAFrench
Copy link
Member

Description

Problem

Resolves

Summary

Additional Context

User Documentation

Check one:

  • No user documentation needed.
  • Changes in docs/ included in this PR.
  • [For Experimental Features] Changes in docs/ 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.

@jfecher
Copy link
Contributor

jfecher commented Jan 16, 2026

To clarify, I think it's only impl Trait in a return position that is harmful.

The way it is currently implemented, we replace the "impl Trait" with the actual type returned so:

  • If a u32 is returned, you can use it as a u32, unlike in Rust / other languages.
  • It is effectively just return-type inference for functions

@jfecher
Copy link
Contributor

jfecher commented Jan 16, 2026

I think my past comment was a bit too strongly worded though - I didn't know we already gated these behind an experimental feature. That seems fine to me if so - sorry for the mistake.

Edit: I suppose I'm fine with removing the return value position entirely as well though - there'd likely not be anything we'd want to carry over for the actual implementation when we do do it properly. I'll leave it to your judgement.

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: b9b443a Previous: 5a4166e Ratio
rollup-block-root-single-tx 0.003 s 0.002 s 1.50

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

CC: @TomAFrench

@aakoshh
Copy link
Contributor

aakoshh commented Jan 19, 2026

Huh, I didn't know it was just a warning instead of an error, like enums. Just a reminder that we can add them in Nargo.toml to allow integration tests to use it.

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.

Having every unstable feature be an error is more consistent - we should be able to have a release with the previous warning-only version as well for ease of migration

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.

4 participants