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

Implement DictNew hint #351

Merged
merged 18 commits into from
May 24, 2024
Merged

Implement DictNew hint #351

merged 18 commits into from
May 24, 2024

Conversation

har777
Copy link
Contributor

@har777 har777 commented Apr 12, 2024

Implements #288

@har777 har777 marked this pull request as draft April 12, 2024 05:43
@har777 har777 linked an issue Apr 12, 2024 that may be closed by this pull request
@har777 har777 changed the title Dictnew hint Implement Dictnew hint May 13, 2024
@har777 har777 marked this pull request as ready for review May 14, 2024 09:23
@har777 har777 changed the base branch from main to dictupdate_hint May 15, 2024 15:13
@har777 har777 changed the title Implement Dictnew hint Implement DictNew hint May 15, 2024
Copy link
Member

@TAdev0 TAdev0 left a comment

Choose a reason for hiding this comment

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

@har777 LGTM!

pkg/hintrunner/zero/zerohint.go Outdated Show resolved Hide resolved
pkg/hintrunner/zero/zerohint_dictionaries.go Show resolved Hide resolved
@TAdev0 TAdev0 merged commit c2df4d3 into dictupdate_hint May 24, 2024
3 checks passed
@TAdev0 TAdev0 deleted the dictnew_hint branch May 24, 2024 10:45
har777 added a commit that referenced this pull request Jun 12, 2024
* Add basic skeleton

* Implement DefaultDictNew

* Add tests

* Implement DefaultRead

* Implement DictWrite

* Add simple integration test

* Update dict integration test

* Update dict integration test

* Fix imports

* Fix imports

* Implement DictUpdate

* Add comments + minor changes

* Remove unnecessary ctx init

* Add comment

* Add comment

* Clean up dict integration test

* Clean up dict integration test

* Clean up dict integration test

* Clean up dict integration test

* Add comment

* Treat dicts in zero hints differently

* Remove accidental newline

* Fix ignoring err message

* Remove some unnecessary code from tests

* Update implementation

* Fix typo

* Fix typo

* Fix typo

* Improved comments

* Add credit comment

* Add better comments

* Add better comments

* Add better comments

* Remove unused operand

* Fix freeOffset bug + add tests

* Update test

* Fix tests

* Add and use zeroDictInScopeEquals test util

* Update test

* Implement DictNew hint (#351)

* Add initial skeleton

* Implement DictNew hint

* Move dictionary manager to scope

* Fix lint errir

* Add GetDictionaryManager util

* Change return signature

* Fix pointer issue

* Clean up some tests

* Add comment

* Reuse context dict manager

* Fix some comments

* Add better comments

* Fix bad comment ident

* Add DictAccessSize constant
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.

DictNew
4 participants