[flang] Allow record advancement in external formatted sequential READ

[klausler]

Summary

The '/' control edit descriptor causes a runtime crash for an
external formatted sequential READ because the AdvanceRecord()
member function for external units implemented only the tasks
to finish reading the current record. Split those out into
a new FinishReadingRecord() member function, call that instead
from EndIoStatement(), and change AdvanceRecord() to both
finish reading the current record and to begin reading the next
one.

[PeteSteinfeld]

All builds, tests, and looks good.

flang/runtime/io-stmt.h

public:
   void BackspaceRecord();
   void HandleRelativePosition(std::int64_t);
   int EndIoStatement();
   ConnectionState &GetConnectionState();
   IoErrorHandler &GetIoErrorHandler() const;
   ExternalFileUnit *GetExternalFileUnit() const; // null if internal unit
   MutableModes &mutableModes();
   void BeginReadingRecord();
+  void FinishReadingRecord();
   bool Inquire(InquiryKeywordHash, char *, std::size_t);
   bool Inquire(InquiryKeywordHash, bool &);
   bool Inquire(InquiryKeywordHash, std::int64_t, bool &); // PENDING=
   bool Inquire(InquiryKeywordHash, std::int64_t &);
 
   // N.B.: this also works with base classes
   template <typename A> A *get_if() const {
     return std::visit(

 // Base class for all per-I/O statement state classes.
 // Inherits IoErrorHandler from its base.
 struct IoStatementBase : public DefaultFormatControlCallbacks {
   using DefaultFormatControlCallbacks::DefaultFormatControlCallbacks;
   int EndIoStatement();
   std::optional<DataEdit> GetNextDataEdit(IoStatementState &, int = 1);
   ExternalFileUnit *GetExternalFileUnit() const { return nullptr; }
   void BeginReadingRecord() {}
-
+  void FinishReadingRecord() {}
   bool Inquire(InquiryKeywordHash, char *, std::size_t);
   bool Inquire(InquiryKeywordHash, bool &);
   bool Inquire(InquiryKeywordHash, std::int64_t, bool &);
   bool Inquire(InquiryKeywordHash, std::int64_t &);
   void BadInquiryKeywordHashCrash(InquiryKeywordHash);
 };
 
 struct InputStatementState {};
public:
   bool Emit(const char16_t *, std::size_t chars /* not bytes */);
   bool Emit(const char32_t *, std::size_t chars /* not bytes */);
   std::optional<char32_t> GetCurrentChar();
   bool AdvanceRecord(int = 1);
   void BackspaceRecord();
   void HandleRelativePosition(std::int64_t);
   void HandleAbsolutePosition(std::int64_t);
   void BeginReadingRecord();
-
-private:
-  bool beganReading_{false};
+  void FinishReadingRecord();
 };
 
 template <Direction DIR, typename CHAR>
 class ExternalFormattedIoStatementState : public ExternalIoStatementState<DIR>,
                                           public FormattedStatementState {
 public:
   using CharType = CHAR;
   ExternalFormattedIoStatementState(ExternalFileUnit &, const CharType *format,

flang/runtime/io-stmt.cpp

int NoUnitIoStatementState::EndIoStatement() {
   auto result{IoStatementBase::EndIoStatement()};
   FreeMemory(this);
   return result;
 }
 
 template <Direction DIR> int ExternalIoStatementState<DIR>::EndIoStatement() {
   if constexpr (DIR == Direction::Input) {
     BeginReadingRecord(); // in case of READ with no data items
+    if (!unit().nonAdvancing) {
+      FinishReadingRecord();
     }
-  if (!unit().nonAdvancing && GetIoStat() != IostatEnd) {
+  } else {
+    if (!unit().nonAdvancing) {
       unit().AdvanceRecord(*this);
     }
-  if constexpr (DIR == Direction::Output) {
     unit().FlushIfTerminal(*this);
   }
   return ExternalIoStatementBase::EndIoStatement();
 }
 
 template <Direction DIR>
 bool ExternalIoStatementState<DIR>::Emit(
     const char *data, std::size_t bytes, std::size_t elementBytes) {

 template <Direction DIR>
 void ExternalIoStatementState<DIR>::HandleRelativePosition(std::int64_t n) {
   return unit().HandleRelativePosition(n);
 }
 
 template <Direction DIR>
 void ExternalIoStatementState<DIR>::BeginReadingRecord() {
   if constexpr (DIR == Direction::Input) {
-    if (!beganReading_) {
-      beganReading_ = true;
-      unit().BeginReadingRecord(*this);
-    }
+  } else {
+    Crash("ExternalIoStatementState<Direction::Output>::BeginReadingRecord() "
+          "called");
+  }
+
+template <Direction DIR>
+void ExternalIoStatementState<DIR>::FinishReadingRecord() {
+  if constexpr (DIR == Direction::Input) {
+    unit().FinishReadingRecord(*this);
+  } else {
+    Crash("ExternalIoStatementState<Direction::Output>::FinishReadingRecord() "
+          "called");
   }
 }
 
 template <Direction DIR, typename CHAR>
 ExternalFormattedIoStatementState<DIR, CHAR>::ExternalFormattedIoStatementState(
     ExternalFileUnit &unit, const CHAR *format, std::size_t formatLength,
     const char *sourceFile, int sourceLine)
     : ExternalIoStatementState<DIR>{unit, sourceFile, sourceLine},

flang/runtime/unit.h

public:
   }
 
   bool Emit(
       const char *, std::size_t, std::size_t elementBytes, IoErrorHandler &);
   bool Receive(char *, std::size_t, std::size_t elementBytes, IoErrorHandler &);
   std::optional<char32_t> GetCurrentChar(IoErrorHandler &);
   void SetLeftTabLimit();
   void BeginReadingRecord(IoErrorHandler &);
+  void FinishReadingRecord(IoErrorHandler &);
   bool AdvanceRecord(IoErrorHandler &);
   void BackspaceRecord(IoErrorHandler &);
   void FlushIfTerminal(IoErrorHandler &);
   void Endfile(IoErrorHandler &);
   void Rewind(IoErrorHandler &);
   void EndIoStatement();
   void SetPosition(std::int64_t pos) {
     frameOffsetInFile_ = pos;
private:
   void BackspaceVariableFormattedRecord(IoErrorHandler &);
   bool SetSequentialVariableFormattedRecordLength();
   void DoImpliedEndfile(IoErrorHandler &);
   void DoEndfile(IoErrorHandler &);
 
   int unitNumber_{-1};
   Direction direction_{Direction::Output};
   bool impliedEndfile_{false}; // seq. output has taken place
+  bool beganReadingRecord_{false};
 
   Lock lock_;
 
   // When an I/O statement is in progress on this unit, holds its state.
   std::variant<std::monostate, OpenStatementState, CloseStatementState,
       ExternalFormattedIoStatementState<Direction::Output>,
       ExternalFormattedIoStatementState<Direction::Input>,
       ExternalListIoStatementState<Direction::Output>,

flang/runtime/unit.cpp

 
 void ExternalFileUnit::SetLeftTabLimit() {
   leftTabLimit = furthestPositionInRecord;
   positionInRecord = furthestPositionInRecord;
 }
 
 void ExternalFileUnit::BeginReadingRecord(IoErrorHandler &handler) {
   RUNTIME_CHECK(handler, direction_ == Direction::Input);
+  if (beganReadingRecord_) {
+    return;
+  }
+  beganReadingRecord_ = true;
   if (access == Access::Sequential) {
     if (endfileRecordNumber && currentRecordNumber >= *endfileRecordNumber) {
       handler.SignalEnd();
     } else if (isFixedRecordLength) {
       RUNTIME_CHECK(handler, recordLength.has_value());
       auto need{static_cast<std::size_t>(recordOffsetInFrame_ + *recordLength)};
       auto got{ReadFrame(frameOffsetInFile_, need, handler)};
       if (got < need) {
         handler.SignalEnd();
       }
     } else if (isUnformatted) {
       BeginSequentialVariableUnformattedInputRecord(handler);
     } else { // formatted
       BeginSequentialVariableFormattedInputRecord(handler);
     }
   }
 }
 
-bool ExternalFileUnit::AdvanceRecord(IoErrorHandler &handler) {
-  bool ok{true};
-  if (direction_ == Direction::Input) {
-    if (access == Access::Sequential) {
-      RUNTIME_CHECK(handler, recordLength.has_value());
-      if (isFixedRecordLength) {
-        frameOffsetInFile_ += recordOffsetInFrame_ + *recordLength;
-        recordOffsetInFrame_ = 0;
-      } else if (isUnformatted) {
-        // Retain footer in frame for more efficient BACKSPACE
-        frameOffsetInFile_ += recordOffsetInFrame_ + *recordLength;
-        recordOffsetInFrame_ = sizeof(std::uint32_t);
-        recordLength.reset();
-      } else { // formatted
-        if (Frame()[recordOffsetInFrame_ + *recordLength] == '\r') {
-          ++recordOffsetInFrame_;
-        }
-        recordOffsetInFrame_ += *recordLength + 1;
-        RUNTIME_CHECK(handler, Frame()[recordOffsetInFrame_ - 1] == '\n');
-        recordLength.reset();
-      }
-    }
+void ExternalFileUnit::FinishReadingRecord(IoErrorHandler &handler) {
+  RUNTIME_CHECK(handler, direction_ == Direction::Input && beganReadingRecord_);
+  beganReadingRecord_ = false;
+  ++currentRecordNumber;
+  BeginRecord();
+}
+
+bool ExternalFileUnit::AdvanceRecord(IoErrorHandler &handler) {
+  bool ok{true};
+  if (direction_ == Direction::Input) {
+    FinishReadingRecord(handler);
+    BeginReadingRecord(handler);
   } else { // Direction::Output
     if (!isUnformatted) {
       if (isFixedRecordLength && recordLength) {
         if (furthestPositionInRecord < *recordLength) {
           WriteFrame(frameOffsetInFile_, *recordLength, handler);
           std::memset(Frame() + recordOffsetInFrame_ + furthestPositionInRecord,
               ' ', *recordLength - furthestPositionInRecord);
         }
       } else {
         positionInRecord = furthestPositionInRecord;
         ok &= Emit("\n", 1, 1, handler); // TODO: Windows CR+LF
       }
     }
     frameOffsetInFile_ +=
         recordOffsetInFrame_ + recordLength.value_or(furthestPositionInRecord);
     recordOffsetInFrame_ = 0;
     impliedEndfile_ = true;
-  }
     ++currentRecordNumber;
     BeginRecord();
+  }
   return ok;
 }
 
 void ExternalFileUnit::BackspaceRecord(IoErrorHandler &handler) {
   if (access != Access::Sequential) {
     handler.SignalError(IostatBackspaceNonSequential,
         "BACKSPACE(UNIT=%d) on non-sequential file", unitNumber());
   } else {