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

[parser] enh: allow whitespace in Format specification #4060

Merged
merged 5 commits into from
May 25, 2024

Conversation

gxyd
Copy link
Contributor

@gxyd gxyd commented May 22, 2024

Blank characters may precede the initial left parenthesis of the format specification and additional blank characters may appear at any point within the format specification, with no effect on the interpretation of the format specification.

e.g.; (I 1 2) is same as (I12) for format specification and are both valid.

Earlier, we used to raise tokenizer error because of no support for it

issue was initially raised in #4040

Blank characters may precede the initial left parenthesis of the
format specification and additional blank characters may appear
at any point within the format specification, with no effect on
the interpretation of the format specification.

e.g.; `(I 1 2)` is same as `(I12)` for format specification
and are both valid.

Earlier, we used to raise tokenizer error because of no support
for it

issue was initially raised in lfortran#4040
tests/format1.f90 Show resolved Hide resolved
@gxyd
Copy link
Contributor Author

gxyd commented May 22, 2024

Surprising that test fails on ubuntu but passes on mac and windows

@certik
Copy link
Contributor

certik commented May 22, 2024

The failure is:

format1.f90 * run
The JSON metadata differs against reference results
Reference JSON: tests/reference/run-format1-04216cf.json
Output JSON:    tests/output/run-format1-04216cf.json
Omitting 10 identical items
Differing items:
{'stdout_hash': '3ef6512ee97acc29ff3be42302f39e69449b6e7ce94f096536ce9436'} != {'stdout_hash': '3ffedee945d8f97620144a79854547c60ad12af5f2e2bde79f28eeec'}
Diff against: tests/reference/run-format1-04216cf.stdout
68c68
<      0.000
---
>    100.000

@certik certik marked this pull request as draft May 22, 2024 14:39
@gxyd gxyd marked this pull request as ready for review May 22, 2024 15:05
tests/format1.f90 Outdated Show resolved Hide resolved
@gxyd
Copy link
Contributor Author

gxyd commented May 22, 2024

I think the current failure of CI isn't related to this PR.

@certik
Copy link
Contributor

certik commented May 23, 2024

The failure is due to #3985.

src/lfortran/parser/tokenizer.re Outdated Show resolved Hide resolved
@gxyd
Copy link
Contributor Author

gxyd commented May 23, 2024

The failure is due to #3985.

I'll give that a look sometime I think.

@certik
Copy link
Contributor

certik commented May 25, 2024

I think it's fine. Not as verbose as before. I can't think right now of any way to simplify, so let's go with this.

Let's restore the original test and then we can merge it.

@gxyd gxyd enabled auto-merge (squash) May 25, 2024 07:18
@gxyd gxyd merged commit 7a1ea9e into lfortran:main May 25, 2024
23 checks passed
@gxyd gxyd deleted the whitespace_format branch May 25, 2024 07:28
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.

None yet

2 participants