Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: require trait primitive functions/calls to have their trait in scope #6901

Open
wants to merge 30 commits into
base: master
Choose a base branch
from

Conversation

asterite
Copy link
Collaborator

@asterite asterite commented Dec 20, 2024

Description

Based on top of #6895

Problem

Part of #6864

Summary

Similar to the previous PRs but this time for primitive types (Field::foo() and field.foo()). In reality Field::foo() already worked before this PR (as a result of some previous PR) but I added tests to make sure that's the case.

This PR also introduces a refactor: previously methods were kept in two places, one for structs and one for everything else (keyed under TypeMethodKey). Now they are all in a single place, and there's a new TypeMethodKey variant for structs. This greatly simplifies/unifies the code.

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.

Copy link
Contributor

Peak Memory Sample

Program Peak Memory
keccak256 78.51M
workspace 123.47M
regression_4709 422.94M
ram_blowup_regression 1.58G
private-kernel-tail 168.30M
private-kernel-reset 168.33M
private-kernel-inner 168.30M
parity-root 151.61M

Copy link
Contributor

github-actions bot commented Dec 20, 2024

Compilation Report

Program Compilation Time %
sha256_regression 1.064s 5%
regression_4709 0.777s -1%
ram_blowup_regression 14.900s 0%
rollup-root 3.750s -2%
rollup-block-merge 3.540s -4%
rollup-base-public 30.900s 4%
rollup-base-private 11.300s 0%

Copy link
Contributor

github-actions bot commented Jan 7, 2025

Execution Report

Program Execution Time %
sha256_regression 0.053s -2%
regression_4709 0.001s 0%
ram_blowup_regression 0.579s 0%
rollup-root 0.106s 0%
rollup-block-merge 0.106s 0%
rollup-base-public 1.330s 0%
rollup-base-private 0.522s 1%

Copy link
Contributor

github-actions bot commented Jan 7, 2025

Compilation Memory Report

Program Peak Memory
keccak256 78.56M
workspace 123.75M
regression_4709 422.97M
ram_blowup_regression 1.58G
rollup-base-public 2.55G
rollup-base-private 1.24G
private-kernel-tail 168.60M
private-kernel-reset 168.63M
private-kernel-inner 168.60M

Copy link
Contributor

github-actions bot commented Jan 7, 2025

Execution Memory Report

Program Peak Memory
keccak256 74.66M
workspace 123.56M
regression_4709 315.99M
ram_blowup_regression 512.41M
rollup-base-public 492.17M
rollup-base-private 332.93M
private-kernel-tail 168.60M
private-kernel-reset 168.63M
private-kernel-inner 168.60M

@TomAFrench TomAFrench marked this pull request as ready for review January 7, 2025 21:17
@TomAFrench TomAFrench marked this pull request as draft January 7, 2025 21:18
@TomAFrench TomAFrench force-pushed the ab/trait-functions-in-scope-for-primitives branch from e6b2286 to 7a33518 Compare January 7, 2025 21:19
Base automatically changed from ab/require-trait-to-be-in-scope-for-methods to master January 8, 2025 10:49
@asterite asterite marked this pull request as ready for review January 8, 2025 11:22
@asterite asterite requested a review from a team January 8, 2025 11:22
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