Skip to content

Conversation

@VolodymyrBg
Copy link
Contributor

The cairo-run binary was calling DebugReplacer::enrich_function_names on the Sierra program and then immediately applying DebugReplacer::apply, which already traverses all functions and updates their FunctionId debug names. This resulted in an unnecessary extra pass over the function list and redundant string allocations without changing behavior. The change drops the enrich_function_names call and relies solely on apply, keeping the observable semantics identical while reducing work.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @VolodymyrBg)


crates/bin/cairo-run/src/main.rs line 80 at r1 (raw file):

        .context("Compilation failed without any diagnostics.")?
        .clone();
    let replacer = DebugReplacer { db };

Suggestion:

        .context("Compilation failed without any diagnostics.")?;
    let replacer = DebugReplacer { db };

@VolodymyrBg
Copy link
Contributor Author

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @VolodymyrBg)

crates/bin/cairo-run/src/main.rs line 80 at r1 (raw file):

        .context("Compilation failed without any diagnostics.")?
        .clone();
    let replacer = DebugReplacer { db };

Suggestion:

        .context("Compilation failed without any diagnostics.")?;
    let replacer = DebugReplacer { db };

corrected

Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

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

:lgtm:

@orizi reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @VolodymyrBg)

@orizi orizi enabled auto-merge December 10, 2025 22:00
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.

3 participants