This is an archive of the discontinued LLVM Phabricator instance.

[flang] Fix handling of files without terminating newlines.
ClosedPublic

Authored by DavidTruby on Apr 21 2020, 11:48 AM.

Details

Diff Detail

Event Timeline

DavidTruby created this revision.Apr 21 2020, 11:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 21 2020, 11:48 AM
sscalpone added inline comments.Apr 21 2020, 3:26 PM
flang/lib/Parser/source.cpp
129–130

Could you merge these two sections since they both deal with files that don't end with a newline character?

ChinouneMehdi added a reviewer: Restricted Project.Apr 21 2020, 8:37 PM
ChinouneMehdi added a project: Restricted Project.

Added test and unified handling of empty files with non-newline terminated files.

@sscalpone I've mostly added this for discussion; I don't think there's another way of doing this with the way we currently handle newlines but I also don't think doing this is the best way forward. We might want to merge it to fix the bug in the short term while we look at newline handling though.

sscalpone requested changes to this revision.Apr 22 2020, 1:17 PM

Please add a test case to cover "if we have spare memory"

This revision now requires changes to proceed.Apr 22 2020, 1:17 PM

Well, currently this will crash if there isn't enough free memory with an error in the MemoryBuffer code. What other sensible behaviour is there? We can't continue if there's no memory as we'll have an infinite loop in the parser so crashing out seems the only option?

I'm asking about the case where "(content().size() >= buf_->getBufferSize())" is false.

Oh, right, I see. I'll add that to the test case!

Added test case including carriage returns; this tests the case where we already
have enough memory to add a newline at the end since we've moved the carriage
returns there.

sscalpone accepted this revision.Apr 24 2020, 9:15 AM

Thanks David!

This revision is now accepted and ready to land.Apr 24 2020, 9:15 AM
This revision was automatically updated to reflect the committed changes.