[flang] Signal EOR in non advancing IO and move to next record

[jeanPerier]

Summary

When an end of record is met in non advancing IO:

• Set IOSTAT if present according to 12.11.4 (5).
• Position the file to the next record (12.11.4 (4)).

The previous code was only signaling EOR for fixed record length IO.
Reading at 12.11.4, I do not find the rational for this condition, so I
removed it.
It also does not seem the presence of padding should prevent
the EOR signaling.

The positionning to the next record was block when EOR is signaling
in FinishReadingRecord because ErrorHandler.isError() is true in this
case.
EOR in input is not an error, but I am not confident to modify
ErrorHandler.isError() to cover that. However, In FinishReadingRecord,
the code should not bail if the error is simply an end of record.

I did not check the SIZE requirements here because GetSize runtime is
not yet implemented.

flang/runtime/io-stmt.cpp

 ExternalIoStatementState<DIR>::ExternalIoStatementState(
     ExternalFileUnit &unit, const char *sourceFile, int sourceLine)
     : ExternalIoStatementBase{unit, sourceFile, sourceLine}, mutableModes_{
                                                                  unit.modes} {}
 
 template <Direction DIR> int ExternalIoStatementState<DIR>::EndIoStatement() {
   if constexpr (DIR == Direction::Input) {
     BeginReadingRecord(); // in case there were no I/O items
-    if (!mutableModes().nonAdvancing) {
+    if (!mutableModes().nonAdvancing || GetIoStat() == IostatEor) {
       FinishReadingRecord();
     }
   } else {
     if (!mutableModes().nonAdvancing) {
       unit().AdvanceRecord(*this);
     }
     unit().FlushIfTerminal(*this);
   }
if (!remaining) { // list-directed or NAMELIST: check for separators
     }
   } else if (*remaining > 0) {
     if (auto next{GetCurrentChar()}) {
       --*remaining;
       HandleRelativePosition(1);
       return next;
     }
     const ConnectionState &connection{GetConnectionState()};
-    if (!connection.IsAtEOF() && connection.isFixedRecordLength &&
-        connection.recordLength &&
+    if (!connection.IsAtEOF() && connection.recordLength &&
         connection.positionInRecord >= *connection.recordLength) {
-      if (connection.modes.pad) { // PAD='YES'
-        --*remaining;
-        return std::optional<char32_t>{' '};
-      }
       IoErrorHandler &handler{GetIoErrorHandler()};
       if (mutableModes().nonAdvancing) {
         handler.SignalEor();
-      } else {
+      } else if (connection.isFixedRecordLength && !connection.modes.pad) {
         handler.SignalError(IostatRecordReadOverrun);
       }
+      if (connection.modes.pad) { // PAD='YES'
+        --*remaining;
+        return std::optional<char32_t>{' '};
+      }
     }
   }
   return std::nullopt;
 }
 
 std::optional<char32_t> IoStatementState::GetNextNonBlank() {
   auto ch{GetCurrentChar()};
   bool inNamelist{GetConnectionState().modes.inNamelist};

flang/runtime/unit.cpp

RUNTIME_CHECK(handler,
       access != Access::Sequential || recordLength.has_value() ||
           handler.InError());
   return !handler.InError();
 }
 
 void ExternalFileUnit::FinishReadingRecord(IoErrorHandler &handler) {
   RUNTIME_CHECK(handler, direction_ == Direction::Input && beganReadingRecord_);
   beganReadingRecord_ = false;
-  if (handler.InError()) {
+  if (handler.InError() && handler.GetIoStat() != IostatEor) {
     // avoid bogus crashes in END/ERR circumstances
   } else if (access == Access::Sequential) {
     RUNTIME_CHECK(handler, recordLength.has_value());
     if (isFixedRecordLength) {
       frameOffsetInFile_ += recordOffsetInFrame_ + *recordLength;
       recordOffsetInFrame_ = 0;
     } else {
       RUNTIME_CHECK(handler, isUnformatted.has_value());

flang/unittests/Runtime/ExternalIOTest.cpp

 
 #include "CrashHandlerFixture.h"
 #include "gtest/gtest.h"
 #include "flang/Runtime/io-api.h"
 #include "flang/Runtime/main.h"
 #include "flang/Runtime/stop.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cstring>
+#include <string_view>
 
 using namespace Fortran::runtime::io;
 
 struct ExternalIOTests : public CrashHandlerFixture {};
 
 TEST(ExternalIOTests, TestDirectUnformatted) {
   // OPEN(NEWUNIT=unit,ACCESS='DIRECT',ACTION='READWRITE',&
   //   FORM='UNFORMATTED',RECL=8,STATUS='SCRATCH')
TEST(ExternalIOTests, TestSequentialVariableFormatted) {
   }
 
   // CLOSE(UNIT=unit,STATUS='DELETE')
   io = IONAME(BeginClose)(unit, __FILE__, __LINE__);
   ASSERT_TRUE(IONAME(SetStatus)(io, "DELETE", 6)) << "SetStatus(DELETE)";
   ASSERT_EQ(IONAME(EndIoStatement)(io), IostatOk)
       << "EndIoStatement() for Close";
 }
+
+TEST(ExternalIOTests, TestNonAvancingInput) {
+  // OPEN(NEWUNIT=unit,ACCESS='SEQUENTIAL',ACTION='READWRITE',&
+  //   FORM='FORMATTED',STATUS='SCRATCH')
+  auto *io{IONAME(BeginOpenNewUnit)(__FILE__, __LINE__)};
+  ASSERT_TRUE(IONAME(SetAccess)(io, "SEQUENTIAL", 10))
+      << "SetAccess(SEQUENTIAL)";
+  ASSERT_TRUE(IONAME(SetAction)(io, "READWRITE", 9)) << "SetAction(READWRITE)";
+  ASSERT_TRUE(IONAME(SetForm)(io, "FORMATTED", 9)) << "SetForm(FORMATTED)";
+  ASSERT_TRUE(IONAME(SetStatus)(io, "SCRATCH", 7)) << "SetStatus(SCRATCH)";
+
+  int unit{-1};
+  ASSERT_TRUE(IONAME(GetNewUnit)(io, unit)) << "GetNewUnit()";
+  ASSERT_EQ(IONAME(EndIoStatement)(io), IostatOk)
+      << "EndIoStatement() for OpenNewUnit";
+
+  // Write the file to be used for the input test.
+  static constexpr std::string_view records[] = {
+      "ABCDEFGH", "IJKLMNOP", "QRSTUVWX"};
+  static constexpr std::string_view fmt{"(A)"};
+  for (const auto &record : records) {
+    // WRITE(UNIT=unit,FMT=fmt) record
+    io = IONAME(BeginExternalFormattedOutput)(
+        fmt.data(), fmt.length(), unit, __FILE__, __LINE__);
+    ASSERT_TRUE(IONAME(OutputAscii)(io, record.data(), record.length()))
+        << "OutputAscii()";
+    ASSERT_EQ(IONAME(EndIoStatement)(io), IostatOk)
+        << "EndIoStatement() for OutputAscii";
+  }
+
+  // REWIND(UNIT=unit)
+  io = IONAME(BeginRewind)(unit, __FILE__, __LINE__);
+  ASSERT_EQ(IONAME(EndIoStatement)(io), IostatOk)
+      << "EndIoStatement() for Rewind";
+
+  struct TestItems {
+    std::string item;
+    int expectedIoStat;
+    std::string expectedItemValue;
+  };
+  // Actual non advancing input IO test
+  TestItems inputItems[]{
+      {std::string(4, '+'), IostatOk, "ABCD"},
+      {std::string(4, '+'), IostatOk, "EFGH"},
+      {std::string(4, '+'), IostatEor, "    "},
+      {std::string(2, '+'), IostatOk, "IJ"},
+      {std::string(8, '+'), IostatEor, "KLMNOP  "},
+      {std::string(10, '+'), IostatEor, "QRSTUVWX  "},
+  };
+
+  int j{0};
+  for (auto &inputItem : inputItems) {
+    // READ(UNIT=unit, FMT=fmt, ADVANCE='NO', IOSTAT=iostat) inputItem
+    io = IONAME(BeginExternalFormattedInput)(
+        fmt.data(), fmt.length(), unit, __FILE__, __LINE__);
+    IONAME(EnableHandlers)(io, true, false, false, false, false);
+    ASSERT_TRUE(IONAME(SetAdvance)(io, "NO", 2)) << "SetAdvance(NO)" << j;
+    ASSERT_TRUE(
+        IONAME(InputAscii)(io, inputItem.item.data(), inputItem.item.length()))
+        << "InputAscii() " << j;
+    ASSERT_EQ(IONAME(EndIoStatement)(io), inputItem.expectedIoStat)
+        << "EndIoStatement() for Read " << j;
+    ASSERT_EQ(inputItem.item, inputItem.expectedItemValue)
+        << "Input-item value after non advancing read " << j;
+    j++;
+  }
+}