[flang][runtime] Don't pad CHARACTER input at end of record unless PAD='YES'

[klausler]

Summary

When formatted CHARACTER input runs into the end of an input record,
the runtime usually fills the remainder of the variable with spaces,
but this should be conditional, and not done when PAD='NO'.

And while here, add some better comments to two members of connection.h
to make their non-obvious relationship more clear.

flang/runtime/connection.h

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
+
+  // positionInRecord is the 0-based offset in the current recurd to/from
+  // which the next data transfer will occur.  It can be past
+  // furthestPositionInRecord if moved by an X or T or TR control edit
+  // descriptor.
+  std::int64_t positionInRecord{0};
+
+  // furthestPositionInRecord is the 0-based offset of the greatest
+  // position in the current record to/from which any data transfer has
+  // occurred, plus one.  It can be viewed as a count of bytes processed.
   std::int64_t furthestPositionInRecord{0}; // max(position+bytes)
 
   // 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;

flang/runtime/edit-input.cpp

bool EditCharacterInput(
   if (connection.IsAtEOF()) {
     return false;
   }
   std::size_t remaining{length};
   if (edit.width && *edit.width > 0) {
     remaining = *edit.width;
   }
   // When the field is wider than the variable, we drop the leading
-  // characters.  When the variable is wider than the field, there's
+  // characters.  When the variable is wider than the field, there can be
   // trailing padding.
   const char *input{nullptr};
   std::size_t ready{0};
-  bool hitEnd{false};
   // Skip leading bytes.
   // These bytes don't count towards INQUIRE(IOLENGTH=).
   std::size_t skip{remaining > length ? remaining - length : 0};
   // Transfer payload bytes; these do count.
   while (remaining > 0) {
     if (ready == 0) {
       ready = io.GetNextInputBytes(input);
       if (ready == 0) {
-        hitEnd = true;
-        break;
+        if (io.CheckForEndOfRecord()) {
+          std::fill_n(x, length, ' '); // PAD='YES'
+        }
+        return !io.GetIoErrorHandler().InError();
       }
     }
     std::size_t chunk;
     bool skipping{skip > 0};
     if (connection.isUTF8) {
       chunk = MeasureUTF8Bytes(*input);
       if (skipping) {
         --skip;
while (remaining > 0) {
     if (!skipping) {
       io.GotChar(chunk);
     }
     io.HandleRelativePosition(chunk);
     ready -= chunk;
   }
   // Pad the remainder of the input variable, if any.
   std::fill_n(x, length, ' ');
-  if (hitEnd) {
-    io.CheckForEndOfRecord(); // signal any needed error
-  }
   return true;
 }
 
 template bool EditRealInput<2>(IoStatementState &, const DataEdit &, void *);
 template bool EditRealInput<3>(IoStatementState &, const DataEdit &, void *);
 template bool EditRealInput<4>(IoStatementState &, const DataEdit &, void *);
 template bool EditRealInput<8>(IoStatementState &, const DataEdit &, void *);
 template bool EditRealInput<10>(IoStatementState &, const DataEdit &, void *);

flang/runtime/io-api.cpp

bool IONAME(InputCharacter)(
   cookie->CheckFormattedStmtType<Direction::Input>("InputCharacter");
   StaticDescriptor staticDescriptor;
   Descriptor &descriptor{staticDescriptor.descriptor()};
   descriptor.Establish(kind, length, reinterpret_cast<void *>(x), 0);
   return descr::DescriptorIO<Direction::Input>(*cookie, descriptor);
 }
 
 bool IONAME(InputAscii)(Cookie cookie, char *x, std::size_t length) {
-  return IONAME(InputCharacter(cookie, x, length, 1));
+  return IONAME(InputCharacter)(cookie, x, length, 1);
 }
 
 bool IONAME(OutputLogical)(Cookie cookie, bool truth) {
   cookie->CheckFormattedStmtType<Direction::Output>("OutputLogical");
   StaticDescriptor staticDescriptor;
   Descriptor &descriptor{staticDescriptor.descriptor()};
   descriptor.Establish(
       TypeCategory::Logical, sizeof truth, reinterpret_cast<void *>(&truth), 0);

flang/unittests/Runtime/ExternalIOTest.cpp

TEST(ExternalIOTests, TestNonAvancingInput) {
 
   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()))
+    bool result{
+        IONAME(InputAscii)(io, inputItem.item.data(), inputItem.item.length())};
+    ASSERT_EQ(result, inputItem.expectedIoStat == IostatOk)
         << "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++;
   }
   // CLOSE(UNIT=unit)