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

Allow hexadecimal/octal/binary numeric literals #333

Closed
Y-Nak opened this issue Mar 24, 2021 · 5 comments · Fixed by #410
Closed

Allow hexadecimal/octal/binary numeric literals #333

Y-Nak opened this issue Mar 24, 2021 · 5 comments · Fixed by #410
Labels
comp: analyzer Everything that involves the analyzer pass comp: lowering Everything that involves the lowering pass comp: parser prio: P3 important type: feature type: lang-spec

Comments

@Y-Nak
Copy link
Member

Y-Nak commented Mar 24, 2021

What is wrong?

Currently, the analyzer doesn't allow numeric literal representation other than decimal. This is inconsistent with both Rust and Python specifications.

How can it be fixed

  1. Allow the representations in analyzer.
  2. Normalize the representations for Yul, I think this would be done in lowering pass.
@g-r-a-n-t
Copy link
Member

Thanks for the issue @Y-Nak, we definitely want this.

Normalize the representations for Yul, I think this would be done in lowering pass.

Normalizing might be at odds with Yul readability. I'm not sure if this should be a deal breaker, though.

From the docs: "Programs written in Yul should be readable, even if the code is generated by a compiler from Solidity or another high-level language."

@g-r-a-n-t
Copy link
Member

Yul supports hex, so I'm wondering if we want retain that representation in the Yul contracts, since certain expressions are easier to read with a given representation. As far as octal and binary go, we would definitely want to normalize that to hex in the lowering pass.

@Y-Nak
Copy link
Member Author

Y-Nak commented Mar 29, 2021

@g-r-a-n-t Thanks for your advice, it sounds really reasonable! Can I implement this in line with your suggestion?

@g-r-a-n-t
Copy link
Member

That would be great 🙂. We just need to wait for #338 to get merged before this can be fully implemented, but that should be all.

@Y-Nak
Copy link
Member Author

Y-Nak commented Apr 15, 2021

The simplest way to implement this would be adding the representation kind to AST in parsing pass, so I'd like to wait for #346.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: analyzer Everything that involves the analyzer pass comp: lowering Everything that involves the lowering pass comp: parser prio: P3 important type: feature type: lang-spec
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants