[flang] OPEN(RECL=) handling for sequential formatted I/O

RECL= is required for direct access I/O, but is permitted
as well for sequential I/O, where it is defined by the
standard to specify a maximum record (line) length.
The standard does not say what should happen when an
sequential formatted input record appears whose length is
unequal to RECL= when it is specified.

Precedents from other compilers are unclear: one raises an error,
some honor RECL= as an effective truncation, and a few ignore the
situation.  On output, all other compilers tested raised an
error when an attempt is made to emit a record longer than RECL=.

This patch treats RECL= as effective truncation on input and
as a hard limit with error on output, and also ensures that
RECL= can be set *longer* than the actual input record lengths.

Differential Revision: https://reviews.llvm.org/D115102

Author: klausler

flang/docs/Extensions.md

@@ -351,3 +351,12 @@ end
   the parent, allocatable or not;
   all finalization takes place before any deallocation;
   and no object or subobject will be finalized more than once.
+
+* When `RECL=` is set via the `OPEN` statement for a sequential formatted input
+  file, it functions as an effective maximum record length.
+  Longer records, if any, will appear as if they had been truncated to
+  the value of `RECL=`.
+  (Other compilers ignore `RECL=`, signal an error, or apply effective truncation
+  to some forms of input in this situation.)
+  For sequential formatted output, RECL= serves as a limit on record lengths
+  that raises an error when it is exceeded.

flang/runtime/connection.h

@@ -25,14 +25,12 @@ enum class Access { Sequential, Direct, Stream };
 inline bool IsRecordFile(Access a) { return a != Access::Stream; }
 
 // These characteristics of a connection are immutable after being
-// established in an OPEN statement, except for recordLength,
-// which is immutable only when isFixedRecordLength is true.
+// established in an OPEN statement.
 struct ConnectionAttributes {
   Access access{Access::Sequential}; // ACCESS='SEQUENTIAL', 'DIRECT', 'STREAM'
   std::optional<bool> isUnformatted; // FORM='UNFORMATTED' if true
   bool isUTF8{false}; // ENCODING='UTF-8'
-  bool isFixedRecordLength{false}; // RECL= on OPEN
-  std::optional<std::int64_t> recordLength; // RECL= or current record
+  std::optional<std::int64_t> openRecl; // RECL= on OPEN
 };
 
 struct ConnectionState : public ConnectionAttributes {
@@ -48,6 +46,15 @@ struct ConnectionState : public ConnectionAttributes {
     leftTabLimit.reset();
   }
 
+  std::optional<std::int64_t> EffectiveRecordLength() const {
+    // When an input record is longer than an explicit RECL= from OPEN
+    // it is effectively truncated on input.
+    return openRecl && recordLength && *openRecl < *recordLength ? openRecl
+                                                                 : recordLength;
+  }
+
+  std::optional<std::int64_t> recordLength;
+
   // 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

flang/runtime/internal-unit.cpp

@@ -17,7 +17,6 @@ namespace Fortran::runtime::io {
 template <Direction DIR>
 InternalDescriptorUnit<DIR>::InternalDescriptorUnit(
     Scalar scalar, std::size_t length) {
-  isFixedRecordLength = true;
   recordLength = length;
   endfileRecordNumber = 2;
   void *pointer{reinterpret_cast<void *>(const_cast<char *>(scalar))};
@@ -34,7 +33,6 @@ InternalDescriptorUnit<DIR>::InternalDescriptorUnit(
       terminator, that.SizeInBytes() <= d.SizeInBytes(maxRank, true, 0));
   new (&d) Descriptor{that};
   d.Check();
-  isFixedRecordLength = true;
   recordLength = d.ElementBytes();
   endfileRecordNumber = d.Elements() + 1;
 }

flang/runtime/io-api.cpp

@@ -543,7 +543,7 @@ bool IONAME(SetRec)(Cookie cookie, std::int64_t rec) {
         "REC= may not appear unless ACCESS='DIRECT'");
     return false;
   }
-  if (!connection.isFixedRecordLength || !connection.recordLength) {
+  if (!connection.openRecl) {
     io.GetIoErrorHandler().SignalError("RECL= was not specified");
     return false;
   }
@@ -554,7 +554,7 @@ bool IONAME(SetRec)(Cookie cookie, std::int64_t rec) {
   }
   connection.currentRecordNumber = rec;
   if (auto *unit{io.GetExternalFileUnit()}) {
-    unit->SetPosition((rec - 1) * *connection.recordLength);
+    unit->SetPosition((rec - 1) * *connection.openRecl);
   }
   return true;
 }
@@ -827,12 +827,11 @@ bool IONAME(SetRecl)(Cookie cookie, std::size_t n) {
   if (n <= 0) {
     io.GetIoErrorHandler().SignalError("RECL= must be greater than zero");
   }
-  if (open->wasExtant() && open->unit().isFixedRecordLength &&
-      open->unit().recordLength.value_or(n) != static_cast<std::int64_t>(n)) {
+  if (open->wasExtant() &&
+      open->unit().openRecl.value_or(n) != static_cast<std::int64_t>(n)) {
     open->SignalError("RECL= may not be changed for an open unit");
   }
-  open->unit().isFixedRecordLength = true;
-  open->unit().recordLength = n;
+  open->unit().openRecl = n;
   return true;
 }
 

flang/runtime/io-stmt.cpp

@@ -888,7 +888,7 @@ bool InquireUnitState::Inquire(
   case HashInquiryKeyword("DIRECT"):
     str = !unit().IsConnected() ? "UNKNOWN"
         : unit().access == Access::Direct ||
-            (unit().mayPosition() && unit().isFixedRecordLength)
+            (unit().mayPosition() && unit().openRecl)
         ? "YES"
         : "NO";
     break;
@@ -1056,8 +1056,8 @@ bool InquireUnitState::Inquire(
       result = -1;
     } else if (unit().access == Access::Stream) {
       result = -2;
-    } else if (unit().isFixedRecordLength && unit().recordLength) {
-      result = *unit().recordLength;
+    } else if (unit().openRecl) {
+      result = *unit().openRecl;
     } else {
       result = std::numeric_limits<std::uint32_t>::max();
     }

flang/runtime/io-stmt.h

@@ -192,17 +192,20 @@ class IoStatementState {
         return next;
       }
       const ConnectionState &connection{GetConnectionState()};
-      if (!connection.IsAtEOF() && connection.recordLength &&
-          connection.positionInRecord >= *connection.recordLength) {
-        IoErrorHandler &handler{GetIoErrorHandler()};
-        if (mutableModes().nonAdvancing) {
-          handler.SignalEor();
-        } else if (connection.isFixedRecordLength && !connection.modes.pad) {
-          handler.SignalError(IostatRecordReadOverrun);
-        }
-        if (connection.modes.pad) { // PAD='YES'
-          --*remaining;
-          return std::optional<char32_t>{' '};
+      if (!connection.IsAtEOF()) {
+        if (auto length{connection.EffectiveRecordLength()}) {
+          if (connection.positionInRecord >= *length) {
+            IoErrorHandler &handler{GetIoErrorHandler()};
+            if (mutableModes().nonAdvancing) {
+              handler.SignalEor();
+            } else if (connection.openRecl && !connection.modes.pad) {
+              handler.SignalError(IostatRecordReadOverrun);
+            }
+            if (connection.modes.pad) { // PAD='YES'
+              --*remaining;
+              return std::optional<char32_t>{' '};
+            }
+          }
         }
       }
     }

flang/runtime/unit.cpp

@@ -129,26 +129,27 @@ void ExternalFileUnit::OpenUnit(std::optional<OpenStatus> status,
   Open(status.value_or(OpenStatus::Unknown), action, position, handler);
   auto totalBytes{knownSize()};
   if (access == Access::Direct) {
-    if (!isFixedRecordLength || !recordLength) {
+    if (!openRecl) {
       handler.SignalError(IostatOpenBadRecl,
           "OPEN(UNIT=%d,ACCESS='DIRECT'): record length is not known",
           unitNumber());
-    } else if (*recordLength <= 0) {
+    } else if (*openRecl <= 0) {
       handler.SignalError(IostatOpenBadRecl,
           "OPEN(UNIT=%d,ACCESS='DIRECT',RECL=%jd): record length is invalid",
-          unitNumber(), static_cast<std::intmax_t>(*recordLength));
-    } else if (totalBytes && (*totalBytes % *recordLength != 0)) {
+          unitNumber(), static_cast<std::intmax_t>(*openRecl));
+    } else if (totalBytes && (*totalBytes % *openRecl != 0)) {
       handler.SignalError(IostatOpenBadAppend,
           "OPEN(UNIT=%d,ACCESS='DIRECT',RECL=%jd): record length is not an "
           "even divisor of the file size %jd",
-          unitNumber(), static_cast<std::intmax_t>(*recordLength),
+          unitNumber(), static_cast<std::intmax_t>(*openRecl),
           static_cast<std::intmax_t>(*totalBytes));
     }
+    recordLength = openRecl;
   }
   endfileRecordNumber.reset();
   currentRecordNumber = 1;
-  if (totalBytes && recordLength && *recordLength) {
-    endfileRecordNumber = 1 + (*totalBytes / *recordLength);
+  if (totalBytes && access == Access::Direct && openRecl.value_or(0) > 0) {
+    endfileRecordNumber = 1 + (*totalBytes / *openRecl);
   }
   if (position == Position::Append) {
     if (!endfileRecordNumber) {
@@ -276,21 +277,27 @@ bool ExternalFileUnit::Emit(const char *data, std::size_t bytes,
     std::size_t elementBytes, IoErrorHandler &handler) {
   auto furthestAfter{std::max(furthestPositionInRecord,
       positionInRecord + static_cast<std::int64_t>(bytes))};
-  if (recordLength) {
-    // It is possible for recordLength to have a value now for a
-    // variable-length output record if the previous operation
-    // was a BACKSPACE or non advancing input statement.
-    if (!isFixedRecordLength) {
-      recordLength.reset();
-      beganReadingRecord_ = false;
-    } else if (furthestAfter > *recordLength) {
+  if (openRecl) {
+    // Check for fixed-length record overrun, but allow for
+    // the unformatted sequential record header & footer, if any.
+    int extra{access == Access::Sequential && isUnformatted && *isUnformatted
+            ? static_cast<int>(sizeof(std::uint32_t))
+            : 0};
+    if (furthestAfter > 2 * extra + *openRecl) {
       handler.SignalError(IostatRecordWriteOverrun,
           "Attempt to write %zd bytes to position %jd in a fixed-size record "
           "of %jd bytes",
-          bytes, static_cast<std::intmax_t>(positionInRecord),
-          static_cast<std::intmax_t>(*recordLength));
+          bytes,
+          static_cast<std::intmax_t>(positionInRecord - extra /*header*/),
+          static_cast<std::intmax_t>(*openRecl));
       return false;
     }
+  } else if (recordLength) {
+    // It is possible for recordLength to have a value now for a
+    // variable-length output record if the previous operation
+    // was a BACKSPACE or non advancing input statement.
+    recordLength.reset();
+    beganReadingRecord_ = false;
   }
   WriteFrame(frameOffsetInFile_, recordOffsetInFrame_ + furthestAfter, handler);
   if (positionInRecord > furthestPositionInRecord) {
@@ -340,7 +347,9 @@ std::size_t ExternalFileUnit::GetNextInputBytes(
     const char *&p, IoErrorHandler &handler) {
   RUNTIME_CHECK(handler, direction_ == Direction::Input);
   p = FrameNextInput(handler, 1);
-  return p ? recordLength.value_or(positionInRecord + 1) - positionInRecord : 0;
+  return p ? EffectiveRecordLength().value_or(positionInRecord + 1) -
+          positionInRecord
+           : 0;
 }
 
 std::optional<char32_t> ExternalFileUnit::GetCurrentChar(
@@ -400,17 +409,20 @@ bool ExternalFileUnit::BeginReadingRecord(IoErrorHandler &handler) {
   RUNTIME_CHECK(handler, direction_ == Direction::Input);
   if (!beganReadingRecord_) {
     beganReadingRecord_ = true;
-    if (access == Access::Sequential) {
+    if (access == Access::Direct) {
+      RUNTIME_CHECK(handler, openRecl);
+      auto need{static_cast<std::size_t>(recordOffsetInFrame_ + *openRecl)};
+      auto got{ReadFrame(frameOffsetInFile_, need, handler)};
+      if (got >= need) {
+        recordLength = openRecl;
+      } else {
+        recordLength.reset();
+        handler.SignalEnd();
+      }
+    } else if (access == Access::Sequential) {
+      recordLength.reset();
       if (endfileRecordNumber && currentRecordNumber >= *endfileRecordNumber) {
         handler.SignalEnd();
-      } else if (isFixedRecordLength && access == Access::Direct) {
-        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 {
         RUNTIME_CHECK(handler, isUnformatted.has_value());
         if (isUnformatted.value_or(false)) {
@@ -422,8 +434,7 @@ bool ExternalFileUnit::BeginReadingRecord(IoErrorHandler &handler) {
     }
   }
   RUNTIME_CHECK(handler,
-      access != Access::Sequential || recordLength.has_value() ||
-          handler.InError());
+      recordLength.has_value() || !IsRecordFile(access) || handler.InError());
   return !handler.InError();
 }
 
@@ -435,7 +446,7 @@ void ExternalFileUnit::FinishReadingRecord(IoErrorHandler &handler) {
   } else if (access == Access::Sequential) {
     RUNTIME_CHECK(handler, recordLength.has_value());
     recordOffsetInFrame_ += *recordLength;
-    if (isFixedRecordLength && access == Access::Direct) {
+    if (openRecl && access == Access::Direct) {
       frameOffsetInFile_ += recordOffsetInFrame_;
       recordOffsetInFrame_ = 0;
     } else {
@@ -472,17 +483,15 @@ bool ExternalFileUnit::AdvanceRecord(IoErrorHandler &handler) {
   } else { // Direction::Output
     bool ok{true};
     RUNTIME_CHECK(handler, isUnformatted.has_value());
-    if (isFixedRecordLength && recordLength &&
-        furthestPositionInRecord < *recordLength) {
+    if (openRecl && furthestPositionInRecord < *openRecl) {
       // Pad remainder of fixed length record
-      WriteFrame(
-          frameOffsetInFile_, recordOffsetInFrame_ + *recordLength, handler);
+      WriteFrame(frameOffsetInFile_, recordOffsetInFrame_ + *openRecl, handler);
       std::memset(Frame() + recordOffsetInFrame_ + furthestPositionInRecord,
           isUnformatted.value_or(false) ? 0 : ' ',
-          *recordLength - furthestPositionInRecord);
-      furthestPositionInRecord = *recordLength;
+          *openRecl - furthestPositionInRecord);
+      furthestPositionInRecord = *openRecl;
     }
-    if (!(isFixedRecordLength && access == Access::Direct)) {
+    if (!(openRecl && access == Access::Direct)) {
       positionInRecord = furthestPositionInRecord;
       if (isUnformatted.value_or(false)) {
         // Append the length of a sequential unformatted variable-length record
@@ -527,7 +536,7 @@ void ExternalFileUnit::BackspaceRecord(IoErrorHandler &handler) {
       DoImpliedEndfile(handler);
       if (frameOffsetInFile_ + recordOffsetInFrame_ > 0) {
         --currentRecordNumber;
-        if (isFixedRecordLength && access == Access::Direct) {
+        if (openRecl && access == Access::Direct) {
           BackspaceFixedRecord(handler);
         } else {
           RUNTIME_CHECK(handler, isUnformatted.has_value());
@@ -670,11 +679,11 @@ void ExternalFileUnit::BeginSequentialVariableFormattedInputRecord(
 }
 
 void ExternalFileUnit::BackspaceFixedRecord(IoErrorHandler &handler) {
-  RUNTIME_CHECK(handler, recordLength.has_value());
-  if (frameOffsetInFile_ < *recordLength) {
+  RUNTIME_CHECK(handler, openRecl.has_value());
+  if (frameOffsetInFile_ < *openRecl) {
     handler.SignalError(IostatBackspaceAtFirstRecord);
   } else {
-    frameOffsetInFile_ -= *recordLength;
+    frameOffsetInFile_ -= *openRecl;
   }
 }