-
Notifications
You must be signed in to change notification settings - Fork 122
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
Add bin and oct integer literals & Allow underscores in them #99
Conversation
I think we should also resolve issue #103 in this PR |
0b10101
)
@anton-trunov Done! I've also added oct literals ( |
@Gusarich Awesome stuff! I've noticed you have positive tests, which is great, let's also add a few negative tests as well, like |
|
Ah, sorry, it was a typo: |
Currently parser allows trailing zeroes.. should I also modify this behaviour? |
src/grammar/grammar.ohm
Outdated
integerLiteralDec = digit+ ("_" | digit)* | ||
integerLiteralHex = "0x" hexDigit+ ("_" | hexDigit)* | ||
| "0X" hexDigit+ ("_" | hexDigit)* | ||
integerLiteralBin = "0b" binDigit+ ("_" | binDigit)* | ||
| "0B" binDigit+ ("_" | binDigit)* | ||
integerLiteralOct = "0o" octDigit+ ("_" | octDigit)* | ||
| "0O" octDigit+ ("_" | octDigit)* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd model literals after JS/TS, because it's kind of a go-to language for TON, so people could copy some literals from Tact code into TS tests or something like that.
There is one caveat though: JS's octal literals starting with 0
. For the reasons of backwards compatibility it might be a bit too late to not allow integer literals with leading zeros. So this might result in some suboptimal UX when copying things from Tact to TS tests. (We could introduce a warning about this, though).
These are JS's restrictions, regarding underscores in numeric literals:
- JS does not allow
_
at the end of literals. - As mentioned earlier, we should also forbid
_
after leading zeros. - JS does not allow multiple
_
in a row (so4_2
is a valid literal, but4__2
is not).
Of course, some other languages, like OCaml are very liberal in this respect, e.g. it's ok with 0004____2_
being a valid integer literal. However, I think going with JS's grammar (except for octals) is something we should do.
@@ -547,7 +547,7 @@ semantics.addOperation<ASTNode>('resolve_expression', { | |||
|
|||
// Literals | |||
integerLiteral(n) { | |||
return createNode({ kind: 'number', value: BigInt(n.sourceString), ref: createRef(this) }); // Parses dec-based integer and hex-based integers | |||
return createNode({ kind: 'number', value: BigInt(n.sourceString.replaceAll('_', '')), ref: createRef(this) }); // Parses dec, hex, and bin numbers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, looks like we translate all integer literals into FunC's decimal literals (because we strip this info here). Let's not change it now, but might be nice to have direct correspondence (modulo binary and octal literals, which FunC to does not support yet)
You probably meant leading zeros: I tried to address it here: #99 (comment) |
Looks like there are some merge conflicts from your previous PR :) |
8b3ccec
to
b0a5e29
Compare
b0a5e29
to
556b184
Compare
--foobar gives an intermediate name to a production thereby resolving the arity conflicts, see https://github.com/ohmjs/ohm/blob/39ccead882ad35dad73d99dea854ff60e7146291/examples/math/index.html#L180
ohm doesn't have
binDigit
so I needed to define it myself in grammar