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.

Sign up for GitHub

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

Jump to bottom

Use fgets() for reading from stdin and a temporay buffer to read from file #4021

Merged
merged 9 commits into from
Jun 1, 2024
Merged

Use fgets() for reading from stdin and a temporay buffer to read from file #4021

merged 9 commits into from
Jun 1, 2024

Conversation

kmr-srbh
Copy link
Contributor

@kmr-srbh kmr-srbh commented May 17, 2024 •
edited

fixes #4015

Use fgets() to read n characters from stdin where n is the length of the variable which will store the data. When reading from a file, we use a temporary buffer to store the whole data. Post this, we add the required chunk of data to the main buffer.

Console read

program main
    character(len=2) :: c
    read *, c
    print *, c
    print*, len(c)
end

Output on main

apple
5

Output on branch

ap
2

File read

program hello
    character(3) :: c
    open(11, file="hello.txt", form="formatted", access="stream", status="old")
    read(11, *) c
    print *, c
    print *, len(c)
end program

Content of hello.txt is

Hello

Output on main

Hello
5

Output on branch

Hel
3

@kmr-srbh kmr-srbh requested a review from gxyd May 17, 2024 14:42
@kmr-srbh kmr-srbh changed the title Use fgets() for reading from console and fread() to read char from file Use fgets() for reading from stdin and fread() to read n characters from file May 17, 2024
@kmr-srbh kmr-srbh changed the title Use fgets() for reading from stdin and fread() to read n characters from file Use fgets() for reading from stdin and fread() to read from file May 17, 2024
@kmr-srbh
Copy link
Contributor Author 

The following tests FAILED:
        282 - intrinsics_60 (Failed)
        669 - file_03 (Failed)

intrinsics_60.f90 fails because we are now reading whitespace characters also. I see that GFortran does not read whitespace characters. We can fix this by doing a char-by-char read. I read online that it can be slow. @certik could you please guide me on this?

file_03.f90 fails unexpectedly! All the other file tests pass, but this one fails.

@certik
Copy link
Contributor

certik commented May 18, 2024

Tests have to pass.

@certik certik marked this pull request as draft May 18, 2024 23:54
@kmr-srbh kmr-srbh requested review from gxyd and removed request for gxyd May 21, 2024 04:44
@Shaikh-Ubaid
Copy link
Member

The approach used in this PR seems fine to me. @kmr-srbh you just need to get the tests to pass. Just strip down the failing tests (for example intrinsics_60 or file_03) to the smallest code that fails. Now you have a minimum reproducible example. Use it to find a fix. Ensure your fix does not break any other tests. And that's it!

@kmr-srbh kmr-srbh added the enhancement Issues or pull request for enhancing existing functionality label May 26, 2024
@kmr-srbh
Copy link
Contributor Author

kmr-srbh commented May 27, 2024 •
edited

We now read files character by character skipping whitespaces. Padding input from stdin with spaces is implemented now too. However, the integration tests file_01.f90 and file_03.f90 fail with an EOF error. I am handling EOF in the implementation, but still the error occurs. Also, only file_03.f90 had failed before, but we have file_01.f90 failing now.

@kmr-srbh kmr-srbh marked this pull request as ready for review May 30, 2024 16:27
@kmr-srbh
Copy link
Contributor Author

This implementation is now complete.

@kmr-srbh
Copy link
Contributor Author

The Windows test fails unexpectedly.

@kmr-srbh kmr-srbh changed the title Use fgets() for reading from stdin and fread() to read from file Use fgets() for reading from stdin and a temporay buffer to read from file May 31, 2024
@kmr-srbh kmr-srbh mentioned this pull request May 31, 2024
Draft
@certik
Copy link
Contributor

certik commented Jun 1, 2024 •
edited

The Windows build error must be investigated. I am guessing some of the new functions that you use do not work on Windows.

@certik certik marked this pull request as draft June 1, 2024 17:27
@kmr-srbh kmr-srbh marked this pull request as ready for review June 1, 2024 20:14
@kmr-srbh
Copy link
Contributor Author

kmr-srbh commented Jun 1, 2024

@certik this PR is now ready.

certik
certik approved these changes Jun 1, 2024
View reviewed changes
Copy link
Contributor

@certik certik left a comment

Choose a reason for hiding this comment

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

I think that this is fine, thanks!

@certik certik merged commit 4ec407e into lfortran:main Jun 1, 2024
23 checks passed
@kmr-srbh kmr-srbh deleted the fix-read branch June 2, 2024 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gxyd gxyd gxyd left review comments

@certik certik certik approved these changes

@Shaikh-Ubaid Shaikh-Ubaid Awaiting requested review from Shaikh-Ubaid

Assignees
No one assigned
Labels
enhancement Issues or pull request for enhancing existing functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bug: read() gives invalid output
4 participants
@kmr-srbh @certik @Shaikh-Ubaid @gxyd