Added types to iterator variables #1054
Conversation
|
I added tests to check that the iterator variables were actually adhering to the types, and had to fix an issue regarding handling decimal types. Not yet done with this, but did have some questions regarding some of the changes I made so far:
|
| @@ -117,6 +118,8 @@ def ann_assign(self): | |||
| pos = self.context.new_variable(varname, typ) | |||
| o = LLLnode.from_list('pass', typ=None, pos=pos) | |||
| if self.stmt.value is not None: | |||
| print('ann_assign does it get this far') | |||
| print(self.stmt.value, self.context) | |||
There was a problem hiding this comment.
Remove the print statements ;)
| # should canonicalize_type be used if this is the case? | ||
| # or another mapping? | ||
| if typ == 'fixed168x10': | ||
| typ = 'decimal' |
There was a problem hiding this comment.
Hmmm not sure canonicalize_type is really intended for this, I would suggest one just creates a simple list of supported types for this functionality with a if type not in (): raise TypeMismatch....
There was a problem hiding this comment.
We also have a base_types in utils.py that you could use for that list ;)
| @@ -70,6 +70,10 @@ def __init__(self, value, args=None, typ=None, location=None, pos=None, annotati | |||
| if isinstance(self.value, int): | |||
| self.valency = 1 | |||
| self.gas = 5 | |||
| elif isinstance(self.value, float): | |||
There was a problem hiding this comment.
are you sure this gets handled correctly? As far as I know the LLL compiler has no concept of float. So you will have to remove this. And the multiply the value by the decimal multiplier (in stmt code below) to get an int for consumption by LLL compiler.
There was a problem hiding this comment.
I double checked this, and you're right; it isn't needed, which means that it isn't being handled properly. To handle this for the decimal type we do need to use the DECIMAL_DIVISOR to convert it. Would it make sense to use the function to convert base types in parser_utils.py? Or to handle it in a different manner?
|
|
||
| @pytest.mark.parametrize('good_code', valid_list) | ||
| def test_type_success(good_code): | ||
| assert compiler.compile(good_code) is not None |
There was a problem hiding this comment.
We need additional tests that confirm each of these types actually produce the output that one thinks it does. I have suspicion decimal might not as it needs num * DECIMAL_DIVISOR // den to work as intended.
There was a problem hiding this comment.
Do you think taking some of the tests from the types testing and combining those with the for loops will work? Or are some different tests needed?
There was a problem hiding this comment.
Have a look at the tests for the in list() functionality (test_for_in_list.py) for a guide on what type of tests we need here. I think creating a new test file "test_for_range." (outside /syntax/) should do just fine.
jacqueswww
changed the title
jacqueswww
changed the title
- What I did
Added types to iterator variables, to close #1039
Example:
for x in range(N, type=bytes32):- How I did it
Parsed the for loop object for keyword "type", and if it exists, uses that as the type for the iterator. If not, type defaults to int128 for backwards compatibility
Checks to make sure that the type is valid as well
- How to verify it
Added two extra blocks to tests/parser/syntax/test_for_range.py and they compile. Also tested by modifying examples/crowdfund.vy loop to have types and it compiles (did not commit those tests)
- Description for the changelog
Added types to iterator objects
- Cute Animal Picture