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: type check for constant arrays #2580

Merged
merged 4 commits into from
Jan 2, 2022

Conversation

tserg
Copy link
Collaborator

@tserg tserg commented Jan 1, 2022

What I did

Fix type checker issue in #2534 and #2130.

The type checking for Subscript VyperNode type currently assumes that an exact type can be derived for its value. Ordinarily, this does not raise an issue for array variables as the value of the Subscript node in this instance would be a Name node type, from which an exact type can be determined.

However, where the array is a constant, the value of the Subscript node is now a List node. When deriving the type for a List node, a list of possible types are returned instead of a specific type (e.g. an array of [0, 1, 2] will return a list of uint8, int128, int256 and uint256). As a result, a StructureException of 'Ambiguous type' will be thrown when there is more than one possible type for an array.

How I did it

Relax the assumption that an exact type can be derived for a Subscript node where its value is a List node:

  • In vyper/semantics/validation/annotation.py, the visit_Subscript function is amended to get all possible types where value is a List node. If only one type is returned, it will be assigned as the base type. If more than one value is returned, we iterate through the list of possible types and return the type that matches the type_ argument supplied to this function.
  • In vyper/semantics/validation/utils.py, the types_from_Subscript function is amended to return all possible types where value is a List node.

How to verify it

See test cases.

Description for the changelog

Fix type checker issue for constant/literal arrays

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@codecov-commenter
Copy link

codecov-commenter commented Jan 1, 2022

Codecov Report

Merging #2580 (c73c185) into master (255d74c) will decrease coverage by 0.03%.
The diff coverage is 84.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2580      +/-   ##
==========================================
- Coverage   85.70%   85.67%   -0.04%     
==========================================
  Files          90       90              
  Lines        9040     9052      +12     
  Branches     2329     2336       +7     
==========================================
+ Hits         7748     7755       +7     
- Misses        812      814       +2     
- Partials      480      483       +3     
Impacted Files Coverage Δ
vyper/semantics/validation/annotation.py 94.40% <80.00%> (-1.12%) ⬇️
vyper/semantics/validation/utils.py 91.21% <100.00%> (+0.13%) ⬆️
vyper/builtin_functions/functions.py 88.20% <0.00%> (-0.35%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 255d74c...c73c185. Read the comment docs.

vyper/semantics/validation/annotation.py Outdated Show resolved Hide resolved
@fubuloubu fubuloubu merged commit 3feb775 into vyperlang:master Jan 2, 2022
@tserg tserg deleted the fix/array_type_check_clean branch January 2, 2022 09:34
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.

None yet

4 participants