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

[peter klausler via flang-commits]

Author: peter klausler
Date: 2021-08-11T13:14:07-07:00
New Revision: 39c38c21527340f2347ac318bf43bfa6d0fc318d

URL: https://github.com/llvm/llvm-project/commit/39c38c21527340f2347ac318bf43bfa6d0fc318d
DIFF: https://github.com/llvm/llvm-project/commit/39c38c21527340f2347ac318bf43bfa6d0fc318d.diff

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

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.

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

Added: 
    

Modified: 
    flang/runtime/io-stmt.cpp
    flang/runtime/io-stmt.h
    flang/unittests/Runtime/ListInputTest.cpp

Removed: 
    


################################################################################
diff  --git a/flang/runtime/io-stmt.cpp b/flang/runtime/io-stmt.cpp
index 56ea6129d5501..084fc781673ea 100644
--- a/flang/runtime/io-stmt.cpp
+++ b/flang/runtime/io-stmt.cpp
@@ -653,13 +653,13 @@ ListDirectedStatementState<Direction::Input>::GetNextDataEdit(
     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;
@@ -669,7 +669,6 @@ ListDirectedStatementState<Direction::Input>::GetNextDataEdit(
     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_) {
@@ -677,6 +676,15 @@ ListDirectedStatementState<Direction::Input>::GetNextDataEdit(
     imaginaryPart_ = true;
     edit.descriptor = DataEdit::ListDirectedImaginaryPart;
   }
+  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();
+  }
+  eatComma_ = true;
   if (!ch) {
     return std::nullopt;
   }
@@ -685,25 +693,9 @@ ListDirectedStatementState<Direction::Input>::GetNextDataEdit(
     edit.descriptor = DataEdit::ListDirectedNullValue;
     return edit;
   }
-  bool isFirstItem{isFirstItem_};
-  isFirstItem_ = false;
-  if (*ch == comma) {
-    if (isFirstItem) {
-      edit.descriptor = DataEdit::ListDirectedNullValue;
-      return edit;
-    }
-    // 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;
-    }
+  if (*ch == comma) { // separator: null value
+    edit.descriptor = DataEdit::ListDirectedNullValue;
+    return edit;
   }
   if (imaginaryPart_) { // can't repeat components
     return edit;
@@ -734,8 +726,8 @@ ListDirectedStatementState<Direction::Input>::GetNextDataEdit(
       }
       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;
     }

diff  --git a/flang/runtime/io-stmt.h b/flang/runtime/io-stmt.h
index eeae5105a13cd..cfbafe2995a9c 100644
--- a/flang/runtime/io-stmt.h
+++ b/flang/runtime/io-stmt.h
@@ -223,15 +223,15 @@ class ListDirectedStatementState<Direction::Input>
   // 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};

diff  --git a/flang/unittests/Runtime/ListInputTest.cpp b/flang/unittests/Runtime/ListInputTest.cpp
index 7aa42905c4365..7bcfa0f6c67db 100644
--- a/flang/unittests/Runtime/ListInputTest.cpp
+++ b/flang/unittests/Runtime/ListInputTest.cpp
@@ -73,7 +73,7 @@ TEST(InputTest, TestListInputIntegerList) {
   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()};
@@ -83,13 +83,15 @@ TEST(InputTest, TestListInputIntegerList) {
   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]);
   }