-
-
Notifications
You must be signed in to change notification settings - Fork 804
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]: fix array index checks when the subscript is folded #3924
fix[lang]: fix array index checks when the subscript is folded #3924
Conversation
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #3924 +/- ##
===========================================
- Coverage 90.96% 51.52% -39.45%
===========================================
Files 95 95
Lines 14397 14396 -1
Branches 3191 3187 -4
===========================================
- Hits 13096 7417 -5679
- Misses 901 6394 +5493
- Partials 400 585 +185 ☔ View full report in Codecov by Sentry. |
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.
lgtm, however, a similar regression is there for Tuples:
The following compiles in v0.3.10
but not on master
x:constant(uint256) = 1
@external
def foo():
a:uint256 = 1
b:uint256 = 1
c:uint256 = (a,b)[x]
v:uint256 = (a,b)[1 + 0]
see
vyper/vyper/semantics/types/subscriptable.py
Lines 369 to 375 in b43ffac
def validate_index_type(self, node): | |
if not isinstance(node, vy_ast.Int): | |
raise InvalidType("Tuple indexes must be literals", node) | |
if node.value < 0: | |
raise ArrayIndexException("Vyper does not support negative indexing", node) | |
if node.value >= self.length: | |
raise ArrayIndexException("Index out of range", node) |
The fix might be more involved as
node.value
is used in several places: vyper/vyper/semantics/types/subscriptable.py
Lines 377 to 378 in b43ffac
def get_subscripted_type(self, node): | |
return self.member_types[node.value] |
common pattern: ```python if node.has_folded_value: node = node.get_folded_value() ``` => ``` node = node.reduced() ```
…lang#3924) this commit fixes a regression introduced in 56c4c9d. prior to 56c4c9d, the folded subscript would be checked for OOB access, but after 56c4c9d, expressions like `foo[0 - 1]` can slip past the typechecker (getting demoted to a runtime check). also, a common pattern is refactored. common pattern: ```python if node.has_folded_value: node = node.get_folded_value() ``` => ``` node = node.reduced() ```
What I did
fix a regression introduced in #3669. repro:
credit @trocher for reporting
How I did it
How to verify it
Commit message
Description for the changelog
Cute Animal Picture