[flang] Fix repeated "DT" editing

[klausler]

Summary

User-defined derived type editing in formatted I/O wasn't
working with repeat counts; e.g., "2DT(10)". The solution required
some code to be moved from GetNextDataEdit() to CueUpNextDataEdit() so
that a stack entry for a nonparenthesized repeated data edit
descriptor would work correctly -- all other data edit descriptors
are capable of dealing with repetition in their callees, so the bug
hadn't been exposed before.

Debugging this problem led to some improvements in error messages
for bad format strings, and those changes have been retained; also,
a dead member function was discovered and expunged.

flang/runtime/descriptor-io.cpp

 std::optional<bool> DefinedFormattedIo(IoStatementState &io,
     const Descriptor &descriptor, const typeInfo::SpecialBinding &special) {
   std::optional<DataEdit> peek{io.GetNextDataEdit(0 /*to peek at it*/)};
   if (peek &&
       (peek->descriptor == DataEdit::DefinedDerivedType ||
           peek->descriptor == DataEdit::ListDirected)) {
     // User-defined derived type formatting
     IoErrorHandler &handler{io.GetIoErrorHandler()};
-    DataEdit edit{*io.GetNextDataEdit()}; // consume it this time
+    DataEdit edit{*io.GetNextDataEdit(1)}; // now consume it; no repeats
     RUNTIME_CHECK(handler, edit.descriptor == peek->descriptor);
     char ioType[2 + edit.maxIoTypeChars];
     auto ioTypeLen{std::size_t{2} /*"DT"*/ + edit.ioTypeChars};
     if (edit.descriptor == DataEdit::DefinedDerivedType) {
       ioType[0] = 'D';
       ioType[1] = 'T';
       std::memcpy(ioType + 2, edit.ioType, edit.ioTypeChars);
     } else {

flang/runtime/format-implementation.h

FormatControl<CONTEXT>::FormatControl(const Terminator &terminator,
   RUNTIME_CHECK(terminator, maxHeight == maxHeight_);
   RUNTIME_CHECK(
       terminator, formatLength == static_cast<std::size_t>(formatLength_));
   stack_[0].start = offset_;
   stack_[0].remaining = Iteration::unlimited; // 13.4(8)
 }
 
 template <typename CONTEXT>
-int FormatControl<CONTEXT>::GetMaxParenthesisNesting(
-    IoErrorHandler &handler, const CharType *format, std::size_t formatLength) {
-  int maxNesting{0};
-  int nesting{0};
-  const CharType *end{format + formatLength};
-  std::optional<CharType> quote;
-  int repeat{0};
-  for (const CharType *p{format}; p < end; ++p) {
-    if (quote) {
-      if (*p == *quote) {
-        quote.reset();
-      }
-    } else if (*p >= '0' && *p <= '9') {
-      repeat = 10 * repeat + *p - '0';
-    } else if (*p != ' ') {
-      switch (*p) {
-      case '\'':
-      case '"':
-        quote = *p;
-        break;
-      case 'h':
-      case 'H': // 9HHOLLERITH
-        p += repeat;
-        if (p >= end) {
-          handler.SignalError(IostatErrorInFormat,
-              "Hollerith (%dH) too long in FORMAT", repeat);
-          return maxNesting;
-        }
-        break;
-      case ' ':
-        break;
-      case '(':
-        ++nesting;
-        maxNesting = std::max(nesting, maxNesting);
-        break;
-      case ')':
-        nesting = std::max(nesting - 1, 0);
-        break;
-      }
-      repeat = 0;
-    }
-  }
-  if (quote) {
-    handler.SignalError(
-        IostatErrorInFormat, "Unbalanced quotation marks in FORMAT string");
-  } else if (nesting) {
-    handler.SignalError(
-        IostatErrorInFormat, "Unbalanced parentheses in FORMAT string");
-  }
-  return maxNesting;
-}
-
-template <typename CONTEXT>
 int FormatControl<CONTEXT>::GetIntField(
     IoErrorHandler &handler, CharType firstCh) {
   CharType ch{firstCh ? firstCh : PeekNext()};
   if (ch != '-' && ch != '+' && (ch < '0' || ch > '9')) {
     handler.SignalError(IostatErrorInFormat,
         "Invalid FORMAT: integer expected at '%c'", static_cast<char>(ch));
     return 0;
   }
   int result{0};
   bool negate{ch == '-'};
   if (negate || ch == '+') {
+    if (firstCh) {
       firstCh = '\0';
+    } else {
+      ++offset_;
+    }
     ch = PeekNext();
   }
   while (ch >= '0' && ch <= '9') {
     if (result >
         std::numeric_limits<int>::max() / 10 - (static_cast<int>(ch) - '0')) {
       handler.SignalError(
           IostatErrorInFormat, "FORMAT integer field out of range");
       return result;

 
 // Locates the next data edit descriptor in the format.
 // Handles all repetition counts and control edit descriptors.
 // Generally assumes that the format string has survived the common
 // format validator gauntlet.
 template <typename CONTEXT>
 int FormatControl<CONTEXT>::CueUpNextDataEdit(Context &context, bool stop) {
   int unlimitedLoopCheck{-1};
+  // Do repetitions remain on an unparenthesized data edit?
+  while (height_ > 1 && format_[stack_[height_ - 1].start] != '(') {
+    offset_ = stack_[height_ - 1].start;
+    int repeat{stack_[height_ - 1].remaining};
+    --height_;
+    if (repeat > 0) {
+      return repeat;
+    }
+  }
   while (true) {
     std::optional<int> repeat;
     bool unlimited{false};
     auto maybeReversionPoint{offset_};
     CharType ch{GetNextChar(context)};
     while (ch == ',' || ch == ':') {
       // Skip commas, and don't complain if they're missing; the format
       // validator does that.
       if (stop && ch == ':') {
         return 0;
       }
       ch = GetNextChar(context);
     }
     if (ch == '-' || ch == '+' || (ch >= '0' && ch <= '9')) {
       repeat = GetIntField(context, ch);
       ch = GetNextChar(context);
     } else if (ch == '*') {
       unlimited = true;
       ch = GetNextChar(context);
       if (ch != '(') {
-        context.SignalError(IostatErrorInFormat,
-            "Invalid FORMAT: '*' may appear only before '('");
+        ReportBadFormat(context,
+            "Invalid FORMAT: '*' may appear only before '('",
+            maybeReversionPoint);
         return 0;
       }
     }
     ch = Capitalize(ch);
     if (ch == '(') {
       if (height_ >= maxHeight_) {
-        context.SignalError(IostatErrorInFormat,
-            "FORMAT stack overflow: too many nested parentheses");
+        ReportBadFormat(context,
+            "FORMAT stack overflow: too many nested parentheses",
+            maybeReversionPoint);
         return 0;
       }
       stack_[height_].start = offset_ - 1; // the '('
       RUNTIME_CHECK(context, format_[stack_[height_].start] == '(');
       if (unlimited || height_ == 0) {
         stack_[height_].remaining = Iteration::unlimited;
         unlimitedLoopCheck = offset_ - 1;
       } else if (repeat) {
         if (*repeat <= 0) {
           *repeat = 1; // error recovery
         }
         stack_[height_].remaining = *repeat - 1;
       } else {
         stack_[height_].remaining = 0;
       }
       if (height_ == 1) {
         // Subtle point (F'2018 13.4 para 9): tha last parenthesized group
         // at height 1 becomes the restart point after control reaches the
         // end of the format, including its repeat count.
-        stack_[0].start = maybeReversionPoint - 1;
+        stack_[0].start = maybeReversionPoint;
       }
       ++height_;
     } else if (height_ == 0) {
-      context.SignalError(IostatErrorInFormat, "FORMAT lacks initial '('");
+      ReportBadFormat(context, "FORMAT lacks initial '('", maybeReversionPoint);
       return 0;
     } else if (ch == ')') {
       if (height_ == 1) {
         if (stop) {
           return 0; // end of FORMAT and no data items remain
         }
         context.AdvanceRecord(); // implied / before rightmost )
       }
-      auto restart{stack_[height_ - 1].start + 1};
+      auto restart{stack_[height_ - 1].start};
+      if (format_[restart] == '(') {
+        ++restart;
+      }
       if (stack_[height_ - 1].remaining == Iteration::unlimited) {
         offset_ = restart;
         if (offset_ == unlimitedLoopCheck) {
-          context.SignalError(IostatErrorInFormat,
-              "Unlimited repetition in FORMAT lacks data edit descriptors");
+          ReportBadFormat(context,
+              "Unlimited repetition in FORMAT lacks data edit descriptors",
+              restart);
         }
       } else if (stack_[height_ - 1].remaining-- > 0) {
         offset_ = restart;
       } else {
         --height_;
       }
     } else if (ch == '\'' || ch == '"') {
       // Quoted 'character literal'
       CharType quote{ch};
       auto start{offset_};
       while (offset_ < formatLength_ && format_[offset_] != quote) {
         ++offset_;
       }
       if (offset_ >= formatLength_) {
-        context.SignalError(IostatErrorInFormat,
-            "FORMAT missing closing quote on character literal");
+        ReportBadFormat(context,
+            "FORMAT missing closing quote on character literal",
+            maybeReversionPoint);
         return 0;
       }
       ++offset_;
       std::size_t chars{
           static_cast<std::size_t>(&format_[offset_] - &format_[start])};
       if (PeekNext() == quote) {
         // subtle: handle doubled quote character in a literal by including
         // the first in the output, then treating the second as the start
         // of another character literal.
       } else {
         --chars;
       }
       context.Emit(format_ + start, chars);
     } else if (ch == 'H') {
       // 9HHOLLERITH
       if (!repeat || *repeat < 1 || offset_ + *repeat > formatLength_) {
-        context.SignalError(
-            IostatErrorInFormat, "Invalid width on Hollerith in FORMAT");
+        ReportBadFormat(context, "Invalid width on Hollerith in FORMAT",
+            maybeReversionPoint);
         return 0;
       }
       context.Emit(format_ + offset_, static_cast<std::size_t>(*repeat));
       offset_ += *repeat;
     } else if (ch >= 'A' && ch <= 'Z') {
       int start{offset_ - 1};
       CharType next{'\0'};
       if (ch != 'P') { // 1PE5.2 - comma not required (C1302)
if (ch == '(') {
       context.AdvanceRecord(repeat && *repeat > 0 ? *repeat : 1);
     } else if (ch == '$' || ch == '\\') {
       context.mutableModes().nonAdvancing = true;
     } else if (ch == '\t' || ch == '\v') {
       // Tabs (extension)
       // TODO: any other raw characters?
       context.Emit(format_ + offset_ - 1, 1);
     } else {
-      context.SignalError(IostatErrorInFormat,
-          "Invalid character '%c' in FORMAT", static_cast<char>(ch));
+      ReportBadFormat(
+          context, "Invalid character in FORMAT", maybeReversionPoint);
       return 0;
     }
   }
 }
 
 // Returns the next data edit descriptor
 template <typename CONTEXT>
 DataEdit FormatControl<CONTEXT>::GetNextDataEdit(
if (auto quote{static_cast<char>(PeekNext())};
         } else {
           edit.ioType[edit.ioTypeChars++] = ch;
           if (ch == quote) {
             ++offset_;
           }
         }
       }
       if (!ok) {
-        context.SignalError(
-            IostatErrorInFormat, "Unclosed DT'iotype' in FORMAT");
+        ReportBadFormat(context, "Unclosed DT'iotype' in FORMAT", start);
       } else if (tooLong) {
-        context.SignalError(
-            IostatErrorInFormat, "Excessive DT'iotype' in FORMAT");
+        ReportBadFormat(context, "Excessive DT'iotype' in FORMAT", start);
       }
     }
     if (PeekNext() == '(') {
       // Capture the v_list arguments
       bool ok{false}, tooLong{false};
       for (++offset_; offset_ < formatLength_;) {
         int n{GetIntField(context)};
         if (edit.vListEntries >= edit.maxVListEntries) {
           tooLong = true;
         } else {
           edit.vList[edit.vListEntries++] = n;
         }
         auto ch{static_cast<char>(GetNextChar(context))};
         if (ch != ',') {
           ok = ch == ')';
           break;
         }
       }
       if (!ok) {
-        context.SignalError(
-            IostatErrorInFormat, "Unclosed DT(v_list) in FORMAT");
+        ReportBadFormat(context, "Unclosed DT(v_list) in FORMAT", start);
       } else if (tooLong) {
-        context.SignalError(
-            IostatErrorInFormat, "Excessive DT(v_list) in FORMAT");
+        ReportBadFormat(context, "Excessive DT(v_list) in FORMAT", start);
       }
     }
   }
   if (edit.descriptor == 'A') { // width is optional for A[w]
     auto ch{PeekNext()};
     if (ch >= '0' && ch <= '9') {
       edit.width = GetIntField(context);
     }
   } else if (edit.descriptor != DataEdit::DefinedDerivedType) {
     edit.width = GetIntField(context);
   }
   if (edit.descriptor != DataEdit::DefinedDerivedType && PeekNext() == '.') {
     ++offset_;
     edit.digits = GetIntField(context);
     CharType ch{PeekNext()};
     if (ch == 'e' || ch == 'E' || ch == 'd' || ch == 'D') {
       ++offset_;
       edit.expoDigits = GetIntField(context);
     }
   }
   edit.modes = context.mutableModes();
-
   // Handle repeated nonparenthesized edit descriptors
+  edit.repeat = std::min(repeat, maxRepeat); // 0 if maxRepeat==0
   if (repeat > maxRepeat) {
     stack_[height_].start = start; // after repeat count
-    stack_[height_].remaining = repeat; // full count
+    stack_[height_].remaining = repeat - edit.repeat;
     ++height_;
   }
-  edit.repeat = std::min(1, maxRepeat); // 0 if maxRepeat==0
-  if (height_ > 1) { // Subtle: stack_[0].start doesn't necessarily point to '('
-    int start{stack_[height_ - 1].start};
-    if (format_[start] != '(') {
-      if (stack_[height_ - 1].remaining > maxRepeat) {
-        edit.repeat = maxRepeat;
-        stack_[height_ - 1].remaining -= maxRepeat;
-        offset_ = start; // repeat same edit descriptor next time
-      } else {
-        edit.repeat = stack_[height_ - 1].remaining;
-        --height_;
-      }
-    }
-  }
   return edit;
 }
 
 template <typename CONTEXT>
 void FormatControl<CONTEXT>::Finish(Context &context) {
   CueUpNextDataEdit(context, true /* stop at colon or end of FORMAT */);
 }
 } // namespace Fortran::runtime::io
 #endif // FORTRAN_RUNTIME_FORMAT_IMPLEMENTATION_H_

flang/runtime/format.h

 public:
   using Context = CONTEXT;
   using CharType = typename Context::CharType;
 
   FormatControl() {}
   FormatControl(const Terminator &, const CharType *format,
       std::size_t formatLength, int maxHeight = maxMaxHeight);
 
-  // Determines the max parenthesis nesting level by scanning and validating
-  // the FORMAT string.
-  static int GetMaxParenthesisNesting(
-      IoErrorHandler &, const CharType *format, std::size_t formatLength);
-
   // For attempting to allocate in a user-supplied stack area
   static std::size_t GetNeededSize(int maxHeight) {
     return sizeof(FormatControl) -
         sizeof(Iteration) * (maxMaxHeight - maxHeight);
   }
 
   // Extracts the next data edit descriptor, handling control edit descriptors
   // along the way.  If maxRepeat==0, this is a peek at the next data edit
private:
   // before the descriptor (defaulting to 1) and leaves offset_
   // pointing to the data edit.
   int CueUpNextDataEdit(Context &, bool stop = false);
 
   static constexpr CharType Capitalize(CharType ch) {
     return ch >= 'a' && ch <= 'z' ? ch + 'A' - 'a' : ch;
   }
 
+  void ReportBadFormat(Context &context, const char *msg, int offset) const {
+    if constexpr (std::is_same_v<CharType, char>) {
+      context.SignalError(IostatErrorInFormat,
+          "%s; at offset %d in format '%s'", msg, offset, format_);
+    } else {
+      context.SignalError(IostatErrorInFormat, "%s; at offset %d", msg, offset);
+    }
+  }
+
   // Data members are arranged and typed so as to reduce size.
   // This structure may be allocated in stack space loaned by the
   // user program for internal I/O.
   const std::uint8_t maxHeight_{maxMaxHeight};
   std::uint8_t height_{0};
   const CharType *format_{nullptr};
   int formatLength_{0};
   int offset_{0}; // next item is at format_[offset_]
 
   // must be last, may be incomplete
   Iteration stack_[maxMaxHeight];
 };
 } // namespace Fortran::runtime::io
 #endif // FORTRAN_RUNTIME_FORMAT_H_

flang/runtime/io-stmt.h

public:
   bool Emit(const char16_t *, std::size_t chars);
   bool Emit(const char32_t *, std::size_t chars);
   bool Receive(char *, std::size_t, std::size_t elementBytes = 0);
   std::size_t GetNextInputBytes(const char *&);
   bool AdvanceRecord(int = 1);
   void BackspaceRecord();
   void HandleRelativePosition(std::int64_t);
   void HandleAbsolutePosition(std::int64_t); // for r* in list I/O
-  std::optional<DataEdit> GetNextDataEdit(int = 1);
+  std::optional<DataEdit> GetNextDataEdit(int maxRepeat = 1);
   ExternalFileUnit *GetExternalFileUnit() const; // null if internal unit
   bool BeginReadingRecord();
   void FinishReadingRecord();
   bool Inquire(InquiryKeywordHash, char *, std::size_t);
   bool Inquire(InquiryKeywordHash, bool &);
   bool Inquire(InquiryKeywordHash, std::int64_t, bool &); // PENDING=
   bool Inquire(InquiryKeywordHash, std::int64_t &);
   void GotChar(signed int = 1); // for READ(SIZE=); can be <0
struct IoStatementBase : public IoErrorHandler {
   bool Emit(const char16_t *, std::size_t chars);
   bool Emit(const char32_t *, std::size_t chars);
   bool Receive(char *, std::size_t, std::size_t elementBytes = 0);
   std::size_t GetNextInputBytes(const char *&);
   bool AdvanceRecord(int);
   void BackspaceRecord();
   void HandleRelativePosition(std::int64_t);
   void HandleAbsolutePosition(std::int64_t);
-  std::optional<DataEdit> GetNextDataEdit(IoStatementState &, int = 1);
+  std::optional<DataEdit> GetNextDataEdit(
+      IoStatementState &, int maxRepeat = 1);
   ExternalFileUnit *GetExternalFileUnit() const;
   bool BeginReadingRecord();
   void FinishReadingRecord();
   bool Inquire(InquiryKeywordHash, char *, std::size_t);
   bool Inquire(InquiryKeywordHash, bool &);
   bool Inquire(InquiryKeywordHash, std::int64_t, bool &);
   bool Inquire(InquiryKeywordHash, std::int64_t &);