-
-
Notifications
You must be signed in to change notification settings - Fork 870
feat: Storage Layout Overrides #2593
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2593 +/- ##
==========================================
- Coverage 86.29% 86.26% -0.03%
==========================================
Files 90 90
Lines 9291 9351 +60
Branches 2354 2370 +16
==========================================
+ Hits 8018 8067 +49
- Misses 789 796 +7
- Partials 484 488 +4
Continue to review full report at Codecov.
|
charles-cooper
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice approach, I think your approach is basically pretty clean. I made some suggestions which should make it easier to follow. I think the main thing would be to refactor StorageCollision so it only has a single publicly facing method, reserve_slot_range(self, slot: int, n_slots: int). This makes its usage clearer since there is only "one way to use it". Then, the internal implementation could be slightly simpler if you use a set instead of a dict.
charles-cooper
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few comments, also I think in Python double underscores are usually used for dunder methods; for private methods please use single underscores. Shaping up nicely, thank you!
This is my first contribution, so would be grateful for everyones time spent reviewing this PR
Targeting #2572
What I did
--storage-layout-fileto compiler which takes a list of storage layout filesHow I did it
storage_layout_overrides. If provided, this is used to custom data position for storage. If it is omitted, then the compiler functions as normallyHow to verify it
Description for the changelog
Added the ability to override storage slots for variables
Cute Animal Picture