Skip to content

Removed various TODOs.#9341

Merged
orizi merged 1 commit intomainfrom
orizi/12-25-removed_various_todos
Dec 28, 2025
Merged

Removed various TODOs.#9341
orizi merged 1 commit intomainfrom
orizi/12-25-removed_various_todos

Conversation

@orizi
Copy link
Collaborator

@orizi orizi commented Dec 25, 2025

Summary

This PR cleans up TODOs and improves code quality in several ways:

  1. Clarified comments in box.cairo about function exposure
  2. Refactored EcPointAdd implementation by replacing nested match statements with more concise if let syntax
  3. Replaced repetitive ConstPowHelper implementations with a macro to reduce code duplication
  4. Removed outdated TODOs that are no longer relevant
  5. Fixed ResOperand::to_res_description() to properly handle BigInt values

Type of change

Please check one:

  • Bug fix (fixes incorrect behavior)
  • New feature
  • Performance improvement
  • Documentation change with concrete technical impact
  • Style, wording, formatting, or typo-only change

Why is this change needed?

This PR addresses several issues:

  1. The EcPointAdd implementation had verbose and potentially error-prone nested match statements
  2. The ConstPowHelper implementations were duplicated for each numeric type, making maintenance difficult
  3. Several TODOs were outdated and no longer applicable to the current codebase

What was the behavior or documentation before?

  • EcPointAdd used nested match statements for control flow
  • ConstPowHelper had separate, duplicated implementations for each numeric type
  • Several outdated TODOs were scattered throughout the codebase

What is the behavior or documentation after?

  • EcPointAdd now uses cleaner if let syntax
  • ConstPowHelper implementations are generated using a macro
  • Outdated TODOs have been removed

Additional context

This PR is primarily focused on code cleanup and maintenance, removing technical debt and improving code quality without changing functionality.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Collaborator Author

orizi commented Dec 25, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@orizi orizi force-pushed the orizi/12-25-removed_various_todos branch from 3e91bbe to fb98e09 Compare December 25, 2025 14:44
@orizi orizi marked this pull request as ready for review December 25, 2025 14:46
* Some by a minor fix
* Some because no longer relevant, since solved or outdated.
Copy link
Contributor

@eytan-starkware eytan-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eytan-starkware reviewed 12 files and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @TomerStarkware).

Copy link
Contributor

@eytan-starkware eytan-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@eytan-starkware made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @TomerStarkware).

Copy link
Collaborator

@TomerStarkware TomerStarkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@TomerStarkware reviewed 5 files and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @orizi).

@orizi orizi added this pull request to the merge queue Dec 28, 2025
Merged via the queue into main with commit f7b306f Dec 28, 2025
55 checks passed
@orizi orizi deleted the orizi/12-25-removed_various_todos branch December 28, 2025 16:03
Princetimix69 pushed a commit to Princetimix69/cairo that referenced this pull request Jan 5, 2026
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