[flang] Fix list-directed plural repeated null values at end of record

[klausler]

Summary

A repeated null value at the end of an input record with a count > 1
would incorrectly advance to the next record when resumed. Fix.

Improve some poor naming and code flow noticed while debugging, so
next time will be easier.

Extend a unit test to check this case.

[PeteSteinfeld]

All builds, tests, and looks good.

flang/runtime/io-stmt.h

std::optional<DataEdit> GetNextDataEdit(
       IoStatementState &, int maxRepeat = 1);
 
   // Each NAMELIST input item is a distinct "list-directed"
   // input statement.  This member function resets this state
   // so that repetition and null values work correctly for each
   // successive NAMELIST input item.
   void ResetForNextNamelistItem() {
     remaining_ = 0;
-    isFirstItem_ = true;
+    eatComma_ = false;
     realPart_ = imaginaryPart_ = false;
   }
 
 private:
   int remaining_{0}; // for "r*" repetition
-  std::int64_t initialRecordNumber_;
-  std::int64_t initialPositionInRecord_;
-  bool isFirstItem_{true}; // leading separator implies null first item
+  std::int64_t repeatRecordNumber_;
+  std::int64_t repeatPositionInRecord_;
+  bool eatComma_{false}; // consume comma after previously read item
   bool hitSlash_{false}; // once '/' is seen, nullify further items
   bool realPart_{false};
   bool imaginaryPart_{false};
 };
 
 template <Direction DIR, typename CHAR = char>
 class InternalIoStatementState : public IoStatementBase,
                                  public IoDirectionState<DIR> {

flang/runtime/io-stmt.cpp

if (hitSlash_) { // everything after '/' is nullified
     edit.descriptor = DataEdit::ListDirectedNullValue;
     return edit;
   }
   char32_t comma{','};
   if (io.mutableModes().editingFlags & decimalComma) {
     comma = ';';
   }
   if (remaining_ > 0 && !realPart_) { // "r*c" repetition in progress
-    while (connection.currentRecordNumber > initialRecordNumber_) {
+    while (connection.currentRecordNumber > repeatRecordNumber_) {
       io.BackspaceRecord();
     }
-    connection.HandleAbsolutePosition(initialPositionInRecord_);
+    connection.HandleAbsolutePosition(repeatPositionInRecord_);
     if (!imaginaryPart_) {
       edit.repeat = std::min<int>(remaining_, maxRepeat);
-      auto ch{io.GetNextNonBlank()};
+      auto ch{io.GetCurrentChar()};
       if (!ch || *ch == ' ' || *ch == '\t' || *ch == comma) {
         // "r*" repeated null
         edit.descriptor = DataEdit::ListDirectedNullValue;
       }
     }
     remaining_ -= edit.repeat;
     return edit;
   }
   // Skip separators, handle a "r*c" repeat count; see 13.10.2 in Fortran 2018
-  auto ch{io.GetNextNonBlank()};
   if (imaginaryPart_) {
     imaginaryPart_ = false;
   } else if (realPart_) {
     realPart_ = false;
     imaginaryPart_ = true;
     edit.descriptor = DataEdit::ListDirectedImaginaryPart;
   }
-  if (!ch) {
-    return std::nullopt;
-  }
-  if (*ch == '/') {
-    hitSlash_ = true;
-    edit.descriptor = DataEdit::ListDirectedNullValue;
-    return edit;
-  }
-  bool isFirstItem{isFirstItem_};
-  isFirstItem_ = false;
-  if (*ch == comma) {
-    if (isFirstItem) {
-      edit.descriptor = DataEdit::ListDirectedNullValue;
-      return edit;
-    }
+  auto ch{io.GetNextNonBlank()};
+  if (ch && *ch == comma && eatComma_) {
     // Consume comma & whitespace after previous item.
     // This includes the comma between real and imaginary components
     // in list-directed/NAMELIST complex input.
     io.HandleRelativePosition(1);
     ch = io.GetNextNonBlank();
-    if (!ch) {
-      return std::nullopt;
-    }
-    if (*ch == comma || *ch == '/') {
-      edit.descriptor = DataEdit::ListDirectedNullValue;
-      return edit;
-    }
+  }
+  eatComma_ = true;
+  if (*ch == '/') {
+    hitSlash_ = true;
+  if (*ch == comma) { // separator: null value
+    edit.descriptor = DataEdit::ListDirectedNullValue;
+    return edit;
   }
   if (imaginaryPart_) { // can't repeat components
     return edit;
   }
   if (*ch >= '0' && *ch <= '9') { // look for "r*" repetition count
     auto start{connection.positionInRecord};
     int r{0};
     do {
if (r > 0 && ch && *ch == '*') { // subtle: r must be nonzero
         edit.descriptor = DataEdit::ListDirectedNullValue;
         return edit;
       }
       if (!ch || *ch == ' ' || *ch == '\t' || *ch == comma) { // "r*" null
         edit.descriptor = DataEdit::ListDirectedNullValue;
       }
       edit.repeat = std::min<int>(r, maxRepeat);
       remaining_ = r - edit.repeat;
-      initialRecordNumber_ = connection.currentRecordNumber;
-      initialPositionInRecord_ = connection.positionInRecord;
+      repeatRecordNumber_ = connection.currentRecordNumber;
+      repeatPositionInRecord_ = connection.positionInRecord;
     } else { // not a repetition count, just an integer value; rewind
       connection.positionInRecord = start;
     }
   }
   if (!imaginaryPart_ && ch && *ch == '(') {
     realPart_ = true;
     io.HandleRelativePosition(1);
     edit.descriptor = DataEdit::ListDirectedRealPart;

flang/unittests/Runtime/ListInputTest.cpp

 }
 
 TEST(InputTest, TestListInputIntegerList) {
   constexpr int numBuffers{2};
   constexpr int maxBufferLength{32};
   char buffer[numBuffers][maxBufferLength];
   int j{0};
   SetCharacter(buffer[j++], maxBufferLength, "1 2 2*3  ,");
-  SetCharacter(buffer[j++], maxBufferLength, ",6,,8,1*");
+  SetCharacter(buffer[j++], maxBufferLength, ",6,,8,2*");
 
   StaticDescriptor<1> staticDescriptor;
   Descriptor &whole{staticDescriptor.descriptor()};
   SubscriptValue extent[]{numBuffers};
   whole.Establish(TypeCode{CFI_type_char}, maxBufferLength, &buffer, 1, extent,
       CFI_attribute_pointer);
   whole.Check();
   auto *cookie{IONAME(BeginInternalArrayListInput)(whole)};
 
-  constexpr int listInputLength{9};
+  constexpr int listInputLength{10};
 
   // Negative numbers will be overwritten by _expectedOutput_, and positive
   // numbers will not be as their indices are "Null values" of the Fortran 2018
   // standard 13.10.3.2 in the format strings _buffer_
-  std::int64_t actualOutput[listInputLength]{-1, -2, -3, -4, 5, -6, 7, -8, 9};
-  const std::int64_t expectedOutput[listInputLength]{1, 2, 3, 3, 5, 6, 7, 8, 9};
+  std::int64_t actualOutput[listInputLength]{
+      -1, -2, -3, -4, 5, -6, 7, -8, 9, 10};
+  const std::int64_t expectedOutput[listInputLength]{
+      1, 2, 3, 3, 5, 6, 7, 8, 9, 10};
   for (j = 0; j < listInputLength; ++j) {
     IONAME(InputInteger)(cookie, actualOutput[j]);
   }
 
   const auto status{IONAME(EndIoStatement)(cookie)};
   ASSERT_EQ(status, 0) << "list-directed input failed, status "
                        << static_cast<int>(status) << '\n';