Skip to content

Conversation

@Princetimix69
Copy link
Contributor

Summary

Fixes a variable shadowing issue in the derive(starknet::Event) macro where struct members named keys or data would collide with the generated deserialize function arguments.


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

⚠️ Note:
To keep maintainer workload sustainable, we generally do not accept PRs that
are only minor wording, grammar, formatting, or style changes.
Such PRs may be closed without detailed review.


Why is this change needed?

When a user defines a Starknet Event struct with members named keys or data, the generated deserialize function creates local variables with the same names. These local variables shadow the ref keys and ref data arguments required for the deserialization process, causing compilation errors or incorrect behavior.

By prefixing the local variables for deserialized values (e.g., using __val_keys instead of keys), this PR ensures that the function arguments remain accessible throughout the function body.


What was the behavior or documentation before?

The derive(starknet::Event) macro generated variable names identical to their struct member names. If a member was named keys or data, it would shadow the deserialization buffer arguments keys and data.


What is the behavior or documentation after?

The macro now generates local variables with a __val_ prefix (e.g., __val_membername). This prevents any collision with the fixed argument names keys and data, allowing Event structs to safely use these names as member fields.


Related issue or discussion (if any)


Additional context

@chatgpt-codex-connector
Copy link

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@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.

@orizi made 1 comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @Princetimix69).


a discussion (no related file):
please make sure to locally run tests before submitting PRs.

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