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

1016 leading whitespaces after indentation are not parsed correctly #1023

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

styrix560
Copy link
Collaborator

Closes: #1016

The problem was, that after parsing indentation whitespace-parsing was postponed. When the whitespaces consisted of two spaces, we first checked if you could parse indentation from the remaining input. Because of the two spaces you could, so more indentation was parsed. The solution was to not postpone whitespace-parsing and instead do it immediately after parsing indentation.

I am more or less sure the test cases look okay. If I were to choose, however, I would pick less. So, I would appreciate feedback.

Checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

@styrix560 styrix560 self-assigned this Apr 21, 2024
@styrix560 styrix560 linked an issue Apr 21, 2024 that may be closed by this pull request
@styrix560 styrix560 requested a review from JonasWanke April 21, 2024 13:03
@jwbot jwbot added the P: Compiler: Frontend Package: The compiler frontend label Apr 21, 2024
@jwbot jwbot enabled auto-merge April 21, 2024 13:03
@jwbot
Copy link
Collaborator

jwbot commented Apr 21, 2024

🐰Bencher

ReportSun, April 21, 2024 at 13:13:42 UTC
ProjectCandy
Branch1016-leading-whitespaces-after-indentation-are-not-parsed-correctly
TestbedGitHub Actions: Ubuntu 22.04
BenchmarkEstimated CyclesEstimated Cycles Results
estimated cycles
InstructionsInstructions Results
instructions
L1 AccessesL1 Accesses Results
accesses
L2 AccessesL2 Accesses Results
accesses
RAM AccessesRAM Accesses Results
accesses
Total AccessesTotal Accesses Results
total-accesses
compile: Examples/fibonacci.candy ➖ (view plot)1791670090.000➖ (view plot)1189937665.000➖ (view plot)1618812475.000➖ (view plot)10751132.000➖ (view plot)3402913.000➖ (view plot)1632966520.000
compile: Examples/helloWorld.candy ➖ (view plot)284198198.000➖ (view plot)176272890.000➖ (view plot)248121798.000➖ (view plot)1479711.000➖ (view plot)819367.000➖ (view plot)250420876.000
vm_runtime: Examples/fibonacci.candy 10➖ (view plot)181431754.000➖ (view plot)106024775.000➖ (view plot)149334889.000➖ (view plot)795013.000➖ (view plot)803480.000➖ (view plot)150933382.000
vm_runtime: Examples/helloWorld.candy ➖ (view plot)154535457.000➖ (view plot)88645701.000➖ (view plot)124969072.000➖ (view plot)701441.000➖ (view plot)744548.000➖ (view plot)126415061.000

Bencher - Continuous Benchmarking
View Public Perf Page
Docs | Repo | Chat | Help

Copy link
Member

@JonasWanke JonasWanke left a comment

Choose a reason for hiding this comment

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

Really sorry for the long delay! I somehow missed this PR

Comment on lines 231 to 247
assert_rich_ir_snapshot!(
whitespaces_and_newlines("\n foo", 0, true),
@r###"
Remaining input: " foo"
Remaining input: "foo"
Parsed: Newline "\n"
Whitespace " "
"###
);
assert_rich_ir_snapshot!(
whitespaces_and_newlines(" \n foo", 0, true),
@r###"
Remaining input: " foo"
Remaining input: "foo"
Parsed: Whitespace " "
Newline "\n"
Whitespace " "
"###
);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the Whitespace here be wrapped in CstError::TooMuchWhitespace in the general case? (For parsed text, the output looks fine)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P: Compiler: Frontend Package: The compiler frontend
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Leading whitespaces after indentation are not parsed correctly
3 participants