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

VIP: Temporarily remove type units #1881

Closed
iamdefinitelyahuman opened this issue Mar 10, 2020 · 2 comments · Fixed by #1896
Closed

VIP: Temporarily remove type units #1881

iamdefinitelyahuman opened this issue Mar 10, 2020 · 2 comments · Fixed by #1896
Labels
VIP: Discussion Used to denote VIPs and more complex issues that are waiting discussion in a meeting

Comments

@iamdefinitelyahuman
Copy link
Contributor

Simple Summary

The current implementation of type units feels inconsistent and confusing. I would like to remove them from the language in their current form.

Motivation

While working on type checking I keep running into issues with units. All of this is made much more difficult because there is very minimal documentation on the subject.

  1. uint256 is considered "unitless", so assignments such as uint256(wei) = uint256 are permitted. It took me a while to realize this was the case, I assumed uint256 was equivalent to uint256(None) and so should not be compatible with uint256(wei).
  2. Addition and subtraction between values with different units is disallowed, but multiplication and division is allowed and produces new compound unit types. This is completely undocumented and does not feel intuitive to me.
  3. wei_value and uint256(wei), despite both being uint256 with unit wei, are incompatible.
  4. Same for timestamp, timedelta, and uint256(sec). Per the documentation, timestamp additionally cannot be added or subtracted with another timestamp. I was unable to check with this firsthand, as attempting to assign to timestamp or timedelta gives me a TypeMistmatch. These feel broken.

While I'm not inherently opposed to the idea of units, the current implementation feels more confusing than beneficial. I propose we remove units altogether while refactoring. We can revisit the concept once refactoring is complete and documentation is up-to-date, and hopefully come up with another approach that's cleaner and more consistent.

Backwards Compatibility

It's a breaking change.

Copyright

Copyright and related rights waived via CC0

@fubuloubu fubuloubu added the VIP: Discussion Used to denote VIPs and more complex issues that are waiting discussion in a meeting label Mar 10, 2020
@fubuloubu fubuloubu mentioned this issue Mar 16, 2020
8 tasks
@fubuloubu
Copy link
Member

Meeting notes: discuss with the community, tentatively accepted for now

@iamdefinitelyahuman
Copy link
Contributor Author

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
VIP: Discussion Used to denote VIPs and more complex issues that are waiting discussion in a meeting
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants