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

Basic type conversions from address type #1524

Merged
merged 9 commits into from
Jul 16, 2019

Conversation

jakerockland
Copy link
Contributor

@jakerockland jakerockland commented Jul 10, 2019

What I did

Implements missing conversions from address to other basic types, as specified in #1093 — note conversion from address to bytes type has yet to be implemented, but was holding off on this for this specific PR as there are also a handful of other conversions to bytes type that need to be implemented still and feel that could just be its own PR.

How I did it

Updated convert.py to support new conversions.

How to verify it

Run the tests make test

Description for the changelog

Add missing conversions from address type to other basic types

Cute Animal Picture

image

@jakerockland jakerockland changed the title Conversions from address Basic type conversions from address type Jul 11, 2019
Copy link
Contributor

@jacqueswww jacqueswww left a comment

Choose a reason for hiding this comment

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

LGTM, just a minor note.

[
'signextend',
15,
['and', in_arg, 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF],
Copy link
Contributor

Choose a reason for hiding this comment

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

I think using SizeLimits here would be cleaner?

Copy link
Contributor

Choose a reason for hiding this comment

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

SizeLimits.ADDRSIZE specifically :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jacqueswww ah good call! 😅 I had overlooked that that constant existed — will make this change tonight.

@jakerockland
Copy link
Contributor Author

jakerockland commented Jul 16, 2019

@jacqueswww fixed!

@jacqueswww jacqueswww merged commit 95fa21a into vyperlang:master Jul 16, 2019
@outdoteth
Copy link

Is uint256 conversion implemented here? I don’t see it; only decimal, int128 and bool. Also, I don’t see these changes in the docs

@iamdefinitelyahuman
Copy link
Contributor

I believe it is implemented, yes. Good catch, that should definitely be documented.

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.

4 participants