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

fix[lang]: typecheck hashmap indexes with folding #4007

Merged
merged 12 commits into from
May 9, 2024

Conversation

DanielSchiavini
Copy link
Contributor

@DanielSchiavini DanielSchiavini commented May 7, 2024

Fixes #3984

What I did

  • Updated index checks for when the subscript is folded

How I did it

  • Use the reduced (i.e. folded) value when checking the index type

How to verify it

  • Tests are included

Commit message

this commit fixes a bug in typechecking of folded values for hashmap
indexes. there was previously a carveout for subscript typechecking for
arrays; however, it should not apply to hashmap indexes.

this was introduced as an edge case resulting from the semantic changes
in 56c4c9dbc0. a related fix was applied in 75fb0594ab3, but it did not
handle hashmaps.

Description for the changelog

HashMap index is checked when the subscript is folded

Cute Animal Picture

image

@charles-cooper charles-cooper changed the title fix[codegen]: index checks when the subscript is folded fix[lang]: index checks when the subscript is folded May 7, 2024
Copy link
Member

@charles-cooper charles-cooper left a comment

Choose a reason for hiding this comment

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

this looks good to me. paging @Leminkay to double check

@charles-cooper charles-cooper changed the title fix[lang]: index checks when the subscript is folded fix[lang]: typecheck hashmap indexes with folding May 8, 2024
@Leminkay
Copy link

Leminkay commented May 8, 2024

LGTM!

@charles-cooper
Copy link
Member

@DanielSchiavini update the commit message please with a description of how the change was arrived at, and i'll merge

@charles-cooper charles-cooper enabled auto-merge (squash) May 9, 2024 22:26
@codecov-commenter
Copy link

codecov-commenter commented May 9, 2024

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 47.96%. Comparing base (4c66c8c) to head (93d1bb7).

❗ Current head 93d1bb7 differs from pull request most recent head 19e927a. Consider uploading reports for the commit 19e927a to get more accurate results

Files Patch % Lines
vyper/semantics/analysis/local.py 0.00% 4 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #4007       +/-   ##
===========================================
- Coverage   90.90%   47.96%   -42.94%     
===========================================
  Files         105      105               
  Lines       15246    15247        +1     
  Branches     3358     3358               
===========================================
- Hits        13859     7313     -6546     
- Misses        952     7326     +6374     
- Partials      435      608      +173     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@charles-cooper charles-cooper enabled auto-merge (squash) May 9, 2024 22:37
@charles-cooper charles-cooper merged commit 54616d6 into vyperlang:master May 9, 2024
151 checks passed
@DanielSchiavini DanielSchiavini deleted the hashmap-checks branch May 10, 2024 08:06
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.

HashMap index checks when the subscript is folded
4 participants