[flang] Fix "non-advancing" I/O, support $ in FORMAT

[klausler]

Summary

Non-advancing I/O was failing; ExternalFileUnit was losing
track of what writes had been committed to the file. Fix.
Also, support the common extension of $ and \ in a FORMAT
as being equivalent to ADVANCE=NO.

[PeteSteinfeld]

All builds, tests, and looks good. I noticed that you have a one word sentence in your commit message. I'm not sure if you intended that.

[klausler]

In D105046#2845047, @PeteSteinfeld wrote:

All builds, tests, and looks good. I noticed that you have a one word sentence in your commit message. I'm not sure if you intended that.

Now that you've accepted it, that one-word sentence can be put into the past tense, anyway.

flang/runtime/connection.h

void BeginRecord() {
     furthestPositionInRecord = 0;
     leftTabLimit.reset();
   }
 
   // Positions in a record file (sequential or direct, not stream)
   std::int64_t currentRecordNumber{1}; // 1 is first
   std::int64_t positionInRecord{0}; // offset in current record
   std::int64_t furthestPositionInRecord{0}; // max(position+bytes)
-  bool nonAdvancing{false}; // ADVANCE='NO'
 
   // Set at end of non-advancing I/O data transfer
   std::optional<std::int64_t> leftTabLimit; // offset in current record
 
   // currentRecordNumber value captured after ENDFILE/REWIND/BACKSPACE statement
   // or an end-of-file READ condition on a sequential access file
   std::optional<std::int64_t> endfileRecordNumber;
 
   // Mutable modes set at OPEN() that can be overridden in READ/WRITE & FORMAT
   MutableModes modes; // BLANK=, DECIMAL=, SIGN=, ROUND=, PAD=, DELIM=, kP
 };
 } // namespace Fortran::runtime::io
 #endif // FORTRAN_RUNTIME_IO_CONNECTION_H_

flang/runtime/format-implementation.h

if (ch == '(') {
         if (ch == 'T') { // Tn, TLn, TRn
           repeat = GetIntField(context);
         }
         HandleControl(context, static_cast<char>(ch), static_cast<char>(next),
             repeat ? *repeat : 1);
       }
     } else if (ch == '/') {
       context.AdvanceRecord(repeat && *repeat > 0 ? *repeat : 1);
+    } else if (ch == '$' || ch == '\\') {
+      context.mutableModes().nonAdvancing = true;
     } else {
       context.SignalError(IostatErrorInFormat,
           "Invalid character '%c' in FORMAT", static_cast<char>(ch));
       return 0;
     }
   }
 }
 

flang/runtime/format.h

struct MutableModes {
   std::uint8_t editingFlags{0}; // BN, DP, SS
   enum decimal::FortranRounding round{
       executionEnvironment
           .defaultOutputRoundingMode}; // RP/ROUND='PROCESSOR_DEFAULT'
   bool pad{true}; // PAD= mode on READ
   char delim{'\0'}; // DELIM=
   short scale{0}; // kP
   bool inNamelist{false}; // skip ! comments
+  bool nonAdvancing{false}; // ADVANCE='NO', or $ or \ in FORMAT
 };
 
 // A single edit descriptor extracted from a FORMAT
 struct DataEdit {
   char descriptor; // capitalized: one of A, I, B, O, Z, F, E(N/S/X), D, G
 
   // Special internal data edit descriptors for list-directed & NAMELIST I/O
   static constexpr char ListDirected{'g'}; // non-COMPLEX list-directed

flang/runtime/io-api.cpp

handler.SignalError(IostatErrorInKeyword, "Invalid %s='%.*s'", what,
         static_cast<int>(length), keyword);
     return false;
   }
 }
 
 bool IONAME(SetAdvance)(
     Cookie cookie, const char *keyword, std::size_t length) {
   IoStatementState &io{*cookie};
-  ConnectionState &connection{io.GetConnectionState()};
-  connection.nonAdvancing =
-      !YesOrNo(keyword, length, "ADVANCE", io.GetIoErrorHandler());
-  if (connection.nonAdvancing && connection.access == Access::Direct) {
+  bool nonAdvancing{
+      !YesOrNo(keyword, length, "ADVANCE", io.GetIoErrorHandler())};
+  if (nonAdvancing && io.GetConnectionState().access == Access::Direct) {
     io.GetIoErrorHandler().SignalError(
         "Non-advancing I/O attempted on direct access file");
+  } else {
+    io.mutableModes().nonAdvancing = nonAdvancing;
   }
   return true;
 }
 
 bool IONAME(SetBlank)(Cookie cookie, const char *keyword, std::size_t length) {
   IoStatementState &io{*cookie};
   ConnectionState &connection{io.GetConnectionState()};
   static const char *keywords[]{"NULL", "ZERO", nullptr};

flang/runtime/io-stmt.h

 private:
   ExternalFileUnit &unit_;
 };
 
 template <Direction DIR>
 class ExternalIoStatementState : public ExternalIoStatementBase,
                                  public IoDirectionState<DIR> {
 public:
-  using ExternalIoStatementBase::ExternalIoStatementBase;
+  ExternalIoStatementState(
+      ExternalFileUnit &, const char *sourceFile = nullptr, int sourceLine = 0);
+  MutableModes &mutableModes() { return mutableModes_; }
   int EndIoStatement();
   bool Emit(const char *, std::size_t, std::size_t elementBytes);
   bool Emit(const char *, std::size_t);
   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);
   bool BeginReadingRecord();
   void FinishReadingRecord();
+
+private:
+  // These are forked from ConnectionState's modes at the beginning
+  // of each formatted I/O statement so they may be overridden by control
+  // edit descriptors during the statement.
+  MutableModes mutableModes_;
 };
 
 template <Direction DIR, typename CHAR>
 class ExternalFormattedIoStatementState : public ExternalIoStatementState<DIR>,
                                           public FormattedIoStatementState {
 public:
   using CharType = CHAR;
   ExternalFormattedIoStatementState(ExternalFileUnit &, const CharType *format,
       std::size_t formatLength, const char *sourceFile = nullptr,
       int sourceLine = 0);
-  MutableModes &mutableModes() { return mutableModes_; }
   int EndIoStatement();
   std::optional<DataEdit> GetNextDataEdit(
       IoStatementState &, int maxRepeat = 1) {
     return format_.GetNextDataEdit(*this, maxRepeat);
   }
 
 private:
-  // These are forked from ConnectionState's modes at the beginning
-  // of each formatted I/O statement so they may be overridden by control
-  // edit descriptors during the statement.
-  MutableModes mutableModes_;
   FormatControl<ExternalFormattedIoStatementState> format_;
 };
 
 template <Direction DIR>
 class ExternalListIoStatementState : public ExternalIoStatementState<DIR>,
                                      public ListDirectedStatementState<DIR> {
 public:
   using ExternalIoStatementState<DIR>::ExternalIoStatementState;

flang/runtime/io-stmt.cpp

ExternalIoStatementBase::ExternalIoStatementBase(
     ExternalFileUnit &unit, const char *sourceFile, int sourceLine)
     : IoStatementBase{sourceFile, sourceLine}, unit_{unit} {}
 
 MutableModes &ExternalIoStatementBase::mutableModes() { return unit_.modes; }
 
 ConnectionState &ExternalIoStatementBase::GetConnectionState() { return unit_; }
 
 int ExternalIoStatementBase::EndIoStatement() {
-  if (unit_.nonAdvancing) {
+  if (mutableModes().nonAdvancing) {
     unit_.leftTabLimit = unit_.furthestPositionInRecord;
-    unit_.nonAdvancing = false;
   } else {
     unit_.leftTabLimit.reset();
   }
   auto result{IoStatementBase::EndIoStatement()};
   unit_.EndIoStatement(); // annihilates *this in unit_.u_
   return result;
 }
 

 }
 
 int NoUnitIoStatementState::EndIoStatement() {
   auto result{IoStatementBase::EndIoStatement()};
   FreeMemory(this);
   return result;
 }
 
+template <Direction DIR>
+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 (!unit().nonAdvancing) {
+    if (!mutableModes().nonAdvancing) {
       FinishReadingRecord();
     }
   } else {
-    if (!unit().nonAdvancing) {
+    if (!mutableModes().nonAdvancing) {
       unit().AdvanceRecord(*this);
     }
     unit().FlushIfTerminal(*this);
   }
   return ExternalIoStatementBase::EndIoStatement();
 }
 
 template <Direction DIR>
void ExternalIoStatementState<DIR>::FinishReadingRecord() {
   }
 }
 
 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},
-      mutableModes_{unit.modes}, format_{*this, format, formatLength} {}
+      format_{*this, format, formatLength} {}
 
 template <Direction DIR, typename CHAR>
 int ExternalFormattedIoStatementState<DIR, CHAR>::EndIoStatement() {
   format_.Finish(*this);
   return ExternalIoStatementState<DIR>::EndIoStatement();
 }
 
 std::optional<DataEdit> IoStatementState::GetNextDataEdit(int n) {
if (!remaining) { // list-directed or NAMELIST: check for separators
     if (!connection.IsAtEOF() && connection.isFixedRecordLength &&
         connection.recordLength &&
         connection.positionInRecord >= *connection.recordLength) {
       if (connection.modes.pad) { // PAD='YES'
         --*remaining;
         return std::optional<char32_t>{' '};
       }
       IoErrorHandler &handler{GetIoErrorHandler()};
-      if (connection.nonAdvancing) {
+      if (mutableModes().nonAdvancing) {
         handler.SignalEor();
       } else {
         handler.SignalError(IostatRecordReadOverrun);
       }
     }
   }
   return std::nullopt;
 }

 template class ChildListIoStatementState<Direction::Input>;
 template class ChildUnformattedIoStatementState<Direction::Output>;
 template class ChildUnformattedIoStatementState<Direction::Input>;
 
 int ExternalMiscIoStatementState::EndIoStatement() {
   ExternalFileUnit &ext{unit()};
   switch (which_) {
   case Flush:
-    ext.Flush(*this);
+    ext.FlushOutput(*this);
     std::fflush(nullptr); // flushes C stdio output streams (12.9(2))
     break;
   case Backspace:
     ext.BackspaceRecord(*this);
     break;
   case Endfile:
     ext.Endfile(*this);
     break;

flang/runtime/unit-map.cpp

for (int j{0}; j < buckets_; ++j) {
     }
   }
 }
 
 void UnitMap::FlushAll(IoErrorHandler &handler) {
   CriticalSection critical{lock_};
   for (int j{0}; j < buckets_; ++j) {
     for (Chain *p{bucket_[j].get()}; p; p = p->next.get()) {
-      p->unit.Flush(handler);
+      p->unit.FlushOutput(handler);
     }
   }
 }
 
 ExternalFileUnit *UnitMap::Find(const char *path) {
   if (path) {
     // TODO: Faster data structure
     for (int j{0}; j < buckets_; ++j) {

flang/runtime/unit.h

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();
   bool BeginReadingRecord(IoErrorHandler &);
   void FinishReadingRecord(IoErrorHandler &);
   bool AdvanceRecord(IoErrorHandler &);
   void BackspaceRecord(IoErrorHandler &);
+  void FlushOutput(IoErrorHandler &);
   void FlushIfTerminal(IoErrorHandler &);
   void Endfile(IoErrorHandler &);
   void Rewind(IoErrorHandler &);
   void EndIoStatement();
   void SetPosition(std::int64_t pos) {
     frameOffsetInFile_ = pos;
     recordOffsetInFrame_ = 0;
     BeginRecord();
private:
   void BeginSequentialVariableUnformattedInputRecord(IoErrorHandler &);
   void BeginSequentialVariableFormattedInputRecord(IoErrorHandler &);
   void BackspaceFixedRecord(IoErrorHandler &);
   void BackspaceVariableUnformattedRecord(IoErrorHandler &);
   void BackspaceVariableFormattedRecord(IoErrorHandler &);
   bool SetSequentialVariableFormattedRecordLength();
   void DoImpliedEndfile(IoErrorHandler &);
   void DoEndfile(IoErrorHandler &);
+  void CommitWrites();
 
   int unitNumber_{-1};
   Direction direction_{Direction::Output};
   bool impliedEndfile_{false}; // seq. output has taken place
   bool beganReadingRecord_{false};
 
   Lock lock_;
 

flang/runtime/unit.cpp

 void FlushOutputOnCrash(const Terminator &terminator) {
   if (!defaultOutput) {
     return;
   }
   CriticalSection critical{unitMapLock};
   if (defaultOutput) {
     IoErrorHandler handler{terminator};
     handler.HasIoStat(); // prevent nested crash if flush has error
-    defaultOutput->Flush(handler);
+    defaultOutput->FlushOutput(handler);
   }
 }
 
 ExternalFileUnit *ExternalFileUnit::LookUp(int unit) {
   return GetUnitMap().LookUp(unit);
 }
 
 ExternalFileUnit &ExternalFileUnit::LookUpOrCrash(
if (IsOpen()) {
     }
     if (!newPath.get() || isSamePath) {
       // OPEN of existing unit, STATUS='OLD' or unspecified, not new FILE=
       newPath.reset();
       return;
     }
     // Otherwise, OPEN on open unit with new FILE= implies CLOSE
     DoImpliedEndfile(handler);
-    Flush(handler);
+    FlushOutput(handler);
     Close(CloseStatus::Keep, handler);
   }
   set_path(std::move(newPath), newPathLength);
   Open(status.value_or(OpenStatus::Unknown), action, position, handler);
   auto totalBytes{knownSize()};
   if (access == Access::Direct) {
     if (!isFixedRecordLength || !recordLength) {
       handler.SignalError(IostatOpenBadRecl,
void ExternalFileUnit::OpenAnonymousUnit(std::optional<OpenStatus> status,
   auto path{SizedNew<char>{handler}(pathMaxLen)};
   std::snprintf(path.get(), pathMaxLen, "fort.%d", unitNumber_);
   OpenUnit(status, action, position, std::move(path), std::strlen(path.get()),
       convert, handler);
 }
 
 void ExternalFileUnit::CloseUnit(CloseStatus status, IoErrorHandler &handler) {
   DoImpliedEndfile(handler);
-  Flush(handler);
+  FlushOutput(handler);
   Close(status, handler);
 }
 
 void ExternalFileUnit::DestroyClosed() {
   GetUnitMap().DestroyClosed(*this); // destroys *this
 }
 
 bool ExternalFileUnit::SetDirection(
if (isFixedRecordLength && recordLength) {
         ok = ok &&
             Emit(reinterpret_cast<const char *>(&length), sizeof length,
                 sizeof length, handler);
       } else {
         // Terminate formatted variable length record
         ok = ok && Emit("\n", 1, 1, handler); // TODO: Windows CR+LF
       }
     }
-    frameOffsetInFile_ +=
-        recordOffsetInFrame_ + recordLength.value_or(furthestPositionInRecord);
-    recordOffsetInFrame_ = 0;
+    CommitWrites();
     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());
if (endfileRecordNumber && currentRecordNumber > *endfileRecordNumber) {
           }
         }
       }
     }
     BeginRecord();
   }
 }
 
+void ExternalFileUnit::FlushOutput(IoErrorHandler &handler) {
+  if (!mayPosition()) {
+    auto frameAt{FrameAt()};
+    if (frameOffsetInFile_ >= frameAt &&
+        frameOffsetInFile_ <
+            static_cast<std::int64_t>(frameAt + FrameLength())) {
+      // A Flush() that's about to happen to a non-positionable file
+      // needs to advance frameOffsetInFile_ to prevent attempts at
+      // impossible seeks
+      CommitWrites();
+    }
+  }
+  Flush(handler);
+}
+
 void ExternalFileUnit::FlushIfTerminal(IoErrorHandler &handler) {
   if (isTerminal()) {
-    Flush(handler);
+    FlushOutput(handler);
   }
 }
 
 void ExternalFileUnit::Endfile(IoErrorHandler &handler) {
   if (access != Access::Sequential) {
     handler.SignalError(IostatEndfileNonSequential,
         "ENDFILE(UNIT=%d) on non-sequential file", unitNumber());
   } else if (!mayWrite()) {
void ExternalFileUnit::Rewind(IoErrorHandler &handler) {
   } else {
     DoImpliedEndfile(handler);
     SetPosition(0);
     currentRecordNumber = 1;
   }
 }
 
 void ExternalFileUnit::EndIoStatement() {
-  frameOffsetInFile_ += recordOffsetInFrame_;
-  recordOffsetInFrame_ = 0;
   io_.reset();
   u_.emplace<std::monostate>();
   lock_.Drop();
 }
 
 void ExternalFileUnit::BeginSequentialVariableUnformattedInputRecord(
     IoErrorHandler &handler) {
   std::int32_t header{0}, footer{0};
if (error) {
     // TODO: error recovery
   }
   positionInRecord = sizeof header;
 }
 
 void ExternalFileUnit::BeginSequentialVariableFormattedInputRecord(
     IoErrorHandler &handler) {
   if (this == defaultInput && defaultOutput) {
-    defaultOutput->Flush(handler);
+    defaultOutput->FlushOutput(handler);
   }
   std::size_t length{0};
   do {
     std::size_t need{recordOffsetInFrame_ + length + 1};
     length = ReadFrame(frameOffsetInFile_, need, handler);
     if (length < need) {
       handler.SignalEnd();
       break;

 
 void ExternalFileUnit::DoEndfile(IoErrorHandler &handler) {
   endfileRecordNumber = currentRecordNumber;
   Truncate(frameOffsetInFile_ + recordOffsetInFrame_, handler);
   BeginRecord();
   impliedEndfile_ = false;
 }
 
+void ExternalFileUnit::CommitWrites() {
+  frameOffsetInFile_ +=
+      recordOffsetInFrame_ + recordLength.value_or(furthestPositionInRecord);
+  recordOffsetInFrame_ = 0;
+  BeginRecord();
+}
+
 ChildIo &ExternalFileUnit::PushChildIo(IoStatementState &parent) {
   OwningPtr<ChildIo> current{std::move(child_)};
   Terminator &terminator{parent.GetIoErrorHandler()};
   OwningPtr<ChildIo> next{New<ChildIo>{terminator}(parent, std::move(current))};
   child_.reset(next.release());
   return *child_;
 }