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

Add support for underscores in numeric literals #2538 #2545

Merged
merged 9 commits into from
Aug 28, 2023
Merged

Conversation

evan-schott
Copy link
Contributor

Motivation

As specified in #2538 the goal is to add support for underscores in numeric literals such as writing 1_000_000 instead of 1000000 to improve readability. In this way the Leo language further aligns with the syntax of Rust. Here are some examples.

Description of changes

While making changes to the compiler I noticed that adding this feature results in tuple indexing inconsistent with Rust. For example, while rust does allow numeric literals like 1_1 and 01 it does not allow them to index tuples meaning x.1_1 and x.01 should fail when trying to access 11th and 1st field of tuple x.

In order to fix this, I made it so the lexer prepends a 0 to any literals with an underscore in them. Previously, 01 would be lexed as 01, and when tuple indexing is parsed it throws an error if there are any leading zeros (and the number has more than one digit). This solution is a little bit ugly, and other alternatives that touch only lexer.rs might be preferable.

Test Plan

I ran the test suite and everything passed. I also added new test files parser/literal/underscore.leo and parser/literal/underscore_fail.leo to test underscore usage across fields, groups, integers, and scalars. Lastly, I added new tests in unreachable/eat_int.leo to check that tuple indexing is consistent with Rust.

Related PRs

Leo Grammar PR

@evan-schott evan-schott requested a review from d0cd August 22, 2023 18:28
@evan-schott evan-schott linked an issue Aug 22, 2023 that may be closed by this pull request
@evan-schott
Copy link
Contributor Author

evan-schott commented Aug 24, 2023

Updates:

  • Although initially it seemed like only a small section of the parser needed to be tweaked in order to change how numbers are consumed, it turned out that the feature would also involve further constraints in the parsing of tuple indexes, as well modifications to the typechecker in order to make sure loops can be properly unrolled in the next pass.
  • I also added a check and custom errors to make sure that loop bounds are the same type and that the start bound is less than the stop bound.

Description of changes:

  • For the parsing of tuple indexing I updated the function that evaluates if the index is valid syntax, and changed it so that the number must have no leading zeros, and no underscores, in order to be consistent with Rust.
  • I modified the type checker pass by adding logic to account for the possibility that the bounds could contain an underscore. This way when the loop unrolling pass takes place the bounds are valid numeric literals, and the loop can be successfully unrolled.

Testing:

  • I added further tests in compiler/statements to check for decreasing loop ranges and bounds that contain underscores.

d0cd

This comment was marked as resolved.

@codecov
Copy link

codecov bot commented Aug 24, 2023

Codecov Report

Merging #2545 (47e881f) into testnet3 (a255113) will increase coverage by 1.07%.
Report is 4 commits behind head on testnet3.
The diff coverage is 95.83%.

@@             Coverage Diff              @@
##           testnet3    #2545      +/-   ##
============================================
+ Coverage     79.03%   80.10%   +1.07%     
============================================
  Files           159      158       -1     
  Lines          5309     5309              
  Branches       5309     5309              
============================================
+ Hits           4196     4253      +57     
+ Misses         1113     1056      -57     
Files Changed Coverage Δ
...piler/passes/src/type_checking/check_statements.rs 82.85% <66.66%> (-0.60%) ⬇️
compiler/ast/src/value/mod.rs 59.74% <100.00%> (+33.54%) ⬆️
compiler/parser/src/parser/context.rs 96.00% <100.00%> (+2.12%) ⬆️
compiler/parser/src/parser/expression.rs 82.47% <100.00%> (+0.06%) ⬆️
compiler/parser/src/tokenizer/lexer.rs 100.00% <100.00%> (ø)
...iler/passes/src/type_checking/check_expressions.rs 82.41% <100.00%> (ø)
errors/src/errors/parser/parser_errors.rs 78.72% <100.00%> (+0.94%) ⬆️
...rors/src/errors/type_checker/type_checker_error.rs 76.35% <100.00%> (+0.32%) ⬆️

... and 48 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@evan-schott evan-schott requested a review from d0cd August 25, 2023 21:16
Copy link
Collaborator

@d0cd d0cd left a comment

Choose a reason for hiding this comment

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

LGTM!

@d0cd d0cd merged commit e180118 into testnet3 Aug 28, 2023
7 of 8 checks passed
@d0cd d0cd deleted the feat/underscore branch August 28, 2023 12:17
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.

[Feature] Support underscores in numeric literals.
2 participants