[flang] Improve runtime crash messages

[PeteSteinfeld]

Summary

Where possible, I added additional information to the messages to help
programmers figure out what went wrong. I also removed all uses of the word
"bad" from the messages since (to me) that implies a moral judgement rather
than a programming error. I replaced it with either "invalid" or "unsupported"
where appropriate.

[klausler]

I prefer the current usage of "bad".

[PeteSteinfeld]

@klausler, do you also prefer "bad" in all contexts, including for messages related to invalid/unsupported KIND specifications?

[klausler]

In D121493#3376173, @PeteSteinfeld wrote:

@klausler, do you also prefer "bad" in all contexts, including for messages related to invalid/unsupported KIND specifications?

Yes.

[PeteSteinfeld]

Responding to Peter's review comments.

flang/runtime/descriptor-io.h

case TypeCategory::Logical:
             "DescriptorIO: Unimplemented LOGICAL kind (%d) in descriptor",
             kind);
         return false;
       }
     case TypeCategory::Derived:
       return FormattedDerivedTypeIO<DIR>(io, descriptor);
     }
   }
-  handler.Crash("DescriptorIO: Bad type code (%d) in descriptor",
+  handler.Crash("DescriptorIO: bad type code (%d) in descriptor",
       static_cast<int>(descriptor.type().raw()));
   return false;
 }
 } // namespace Fortran::runtime::io::descr
 #endif // FORTRAN_RUNTIME_DESCRIPTOR_IO_H_

flang/runtime/extrema.cpp

inline void TypedMaxOrMinLoc(const char *intrinsic, Descriptor &result,
   case TypeCategory::Character:
     ApplyCharacterKind<TypedMaxOrMinLocHelper<TypeCategory::Character,
                            IS_MAX>::template Functor,
         void>(catKind->second, terminator, intrinsic, result, x, kind, source,
         line, mask, back);
     break;
   default:
     terminator.Crash(
-        "%s: Bad data type code (%d) for array", intrinsic, x.type().raw());
+        "%s: bad data type code (%d) for array", intrinsic, x.type().raw());
   }
 }
 
 extern "C" {
 void RTNAME(Maxloc)(Descriptor &result, const Descriptor &x, int kind,
     const char *source, int line, const Descriptor *mask, bool back) {
   TypedMaxOrMinLoc<true>("MAXLOC", result, x, kind, source, line, mask, back);
 }
inline void TypedPartialMaxOrMinLoc(const char *intrinsic, Descriptor &result,
   case TypeCategory::Character:
     ApplyCharacterKind<DoPartialMaxOrMinLocHelper<TypeCategory::Character,
                            IS_MAX, CharacterCompare>::template Functor,
         void>(catKind->second, terminator, intrinsic, result, x, kind, dim,
         mask, back, terminator);
     break;
   default:
     terminator.Crash(
-        "%s: Bad data type code (%d) for array", intrinsic, x.type().raw());
+        "%s: bad data type code (%d) for array", intrinsic, x.type().raw());
   }
 }
 
 extern "C" {
 void RTNAME(MaxlocDim)(Descriptor &result, const Descriptor &x, int kind,
     int dim, const char *source, int line, const Descriptor *mask, bool back) {
   TypedPartialMaxOrMinLoc<true>(
       "MAXLOC", result, x, kind, dim, source, line, mask, back);

flang/runtime/findloc.cpp

ApplyCharacterKind<CharacterFindlocHelper, void>(xType->second, terminator,
         result, x, target, kind, mask, back, terminator);
     break;
   case TypeCategory::Logical:
     RUNTIME_CHECK(terminator, targetType->first == TypeCategory::Logical);
     LogicalFindlocHelper(result, x, target, kind, mask, back, terminator);
     break;
   default:
     terminator.Crash(
-        "FINDLOC: Bad data type code (%d) for array", x.type().raw());
+        "FINDLOC: bad data type code (%d) for array", x.type().raw());
   }
 }
 } // extern "C"
 
 // FINDLOC with DIM=
 
 template <TypeCategory XCAT, int XKIND, TypeCategory TARGET_CAT>
 struct PartialNumericFindlocHelper {
case TypeCategory::Character:
     break;
   case TypeCategory::Logical:
     RUNTIME_CHECK(terminator, targetType->first == TypeCategory::Logical);
     PartialLogicalFindlocHelper(
         result, x, target, kind, dim, mask, back, terminator);
     break;
   default:
     terminator.Crash(
-        "FINDLOC: Bad data type code (%d) for array", x.type().raw());
+        "FINDLOC: bad data type code (%d) for array", x.type().raw());
   }
 }
 } // extern "C"
 } // namespace Fortran::runtime

flang/runtime/inquiry.cpp

 
 namespace Fortran::runtime {
 
 extern "C" {
 std::int64_t RTNAME(LboundDim)(
     const Descriptor &array, int dim, const char *sourceFile, int line) {
   if (dim < 1 || dim > array.rank()) {
     Terminator terminator{sourceFile, line};
-    terminator.Crash("SIZE: bad DIM=%d", dim);
+    terminator.Crash(
+        "SIZE: bad DIM=%d for ARRAY with rank=%d", dim, array.rank());
   }
   const Dimension &dimension{array.GetDimension(dim - 1)};
   return static_cast<std::int64_t>(dimension.LowerBound());
 }
 
 void RTNAME(Ubound)(Descriptor &result, const Descriptor &array, int kind,
     const char *sourceFile, int line) {
   SubscriptValue extent[1]{array.rank()};
std::int64_t RTNAME(Size)(
   }
   return result;
 }
 
 std::int64_t RTNAME(SizeDim)(
     const Descriptor &array, int dim, const char *sourceFile, int line) {
   if (dim < 1 || dim > array.rank()) {
     Terminator terminator{sourceFile, line};
-    terminator.Crash("SIZE: bad DIM=%d", dim);
+    terminator.Crash(
+        "SIZE: bad DIM=%d for ARRAY with rank=%d", dim, array.rank());
   }
   const Dimension &dimension{array.GetDimension(dim - 1)};
   return static_cast<std::int64_t>(dimension.Extent());
 }
 
 } // extern "C"
 } // namespace Fortran::runtime

flang/runtime/io-api.cpp

bool IONAME(SetPos)(Cookie cookie, std::int64_t pos) {
   if (pos < 1) { // POS=1 is beginning of file (12.6.2.11)
     handler.SignalError("POS=%zd is invalid", static_cast<std::intmax_t>(pos));
     return false;
   }
   if (auto *unit{io.GetExternalFileUnit()}) {
     unit->SetPosition(pos - 1, handler);
     return true;
   }
-  io.GetIoErrorHandler().Crash("SetPos() on internal unit");
+  io.GetIoErrorHandler().Crash("SetPos() called on internal unit");
   return false;
 }
 
 bool IONAME(SetRec)(Cookie cookie, std::int64_t rec) {
   IoStatementState &io{*cookie};
   ConnectionState &connection{io.GetConnectionState()};
   IoErrorHandler &handler{io.GetIoErrorHandler()};
   if (connection.access != Access::Direct) {
bool IONAME(GetNewUnit)(Cookie cookie, int &unit, int kind) {
   IoStatementState &io{*cookie};
   auto *open{io.get_if<OpenStatementState>()};
   if (!open) {
     io.GetIoErrorHandler().Crash(
         "GetNewUnit() called when not in an OPEN statement");
   }
   std::int64_t result{open->unit().unitNumber()};
   if (!SetInteger(unit, kind, result)) {
-    open->SignalError("GetNewUnit(): Bad INTEGER kind(%d) or out-of-range "
+    open->SignalError("GetNewUnit(): bad INTEGER kind(%d) or out-of-range "
                       "value(%jd) for result",
         kind, static_cast<std::intmax_t>(result));
   }
   return true;
 }
 
 // Data transfers
 
bool IONAME(InquireInteger64)(
     Cookie cookie, InquiryKeywordHash inquiry, std::int64_t &result, int kind) {
   IoStatementState &io{*cookie};
   std::int64_t n;
   if (io.Inquire(inquiry, n)) {
     if (SetInteger(result, kind, n)) {
       return true;
     }
     io.GetIoErrorHandler().SignalError(
-        "InquireInteger64(): Bad INTEGER kind(%d) or out-of-range value(%jd) "
+        "InquireInteger64(): bad INTEGER kind(%d) or out-of-range "
+        "value(%jd) "
         "for result",
         kind, static_cast<std::intmax_t>(n));
   }
   return false;
 }
 
 enum Iostat IONAME(EndIoStatement)(Cookie cookie) {
   IoStatementState &io{*cookie};
   return static_cast<enum Iostat>(io.EndIoStatement());
 }
 } // namespace Fortran::runtime::io

flang/runtime/io-stmt.cpp

 
 bool IoStatementBase::Inquire(InquiryKeywordHash, std::int64_t &) {
   return false;
 }
 
 void IoStatementBase::BadInquiryKeywordHashCrash(InquiryKeywordHash inquiry) {
   char buffer[16];
   const char *decode{InquiryKeywordHashDecode(buffer, sizeof buffer, inquiry)};
-  Crash("bad InquiryKeywordHash 0x%x (%s)", inquiry,
+  Crash("Bad InquiryKeywordHash 0x%x (%s)", inquiry,
       decode ? decode : "(cannot decode)");
 }
 
 template <Direction DIR, typename CHAR>
 InternalIoStatementState<DIR, CHAR>::InternalIoStatementState(
     Buffer scalar, std::size_t length, const char *sourceFile, int sourceLine)
     : IoStatementBase{sourceFile, sourceLine}, unit_{scalar, length} {}
 

flang/runtime/iostat.cpp

case IostatGenericError:
     return "I/O error"; // dummy value, there's always a message
   case IostatRecordWriteOverrun:
     return "Excessive output to fixed-size record";
   case IostatRecordReadOverrun:
     return "Excessive input from fixed-size record";
   case IostatInternalWriteOverrun:
     return "Internal write overran available records";
   case IostatErrorInFormat:
-    return "Invalid FORMAT";
+    return "Bad FORMAT";
   case IostatErrorInKeyword:
     return "Bad keyword argument value";
   case IostatEndfileDirect:
     return "ENDFILE on direct-access file";
   case IostatEndfileUnwritable:
     return "ENDFILE on read-only file";
   case IostatOpenBadRecl:
     return "OPEN with bad RECL= value";

flang/runtime/random.cpp

case 10:
     Generate<CppTypeFor<TypeCategory::Real, 10>, 64>(harvest);
     break;
 #elif LONG_DOUBLE == 128
   case 16:
     Generate<CppTypeFor<TypeCategory::Real, 16>, 113>(harvest);
     break;
 #endif
   default:
-    terminator.Crash("RANDOM_NUMBER(): unsupported REAL kind %d", kind);
+    terminator.Crash("RANDOM_NUMBER(): bad REAL kind %d", kind);
   }
 }
 
 void RTNAME(RandomSeedSize)(
     const Descriptor &size, const char *source, int line) {
   Terminator terminator{source, line};
   auto typeCode{size.type().GetCategoryAndKind()};
   RUNTIME_CHECK(terminator,

flang/runtime/reduction-templates.h

 // Total reduction of the array argument to a scalar (or to a vector in the
 // cases of FINDLOC, MAXLOC, & MINLOC).  These are the cases without DIM= or
 // cases where the argument has rank 1 and DIM=, if present, must be 1.
 template <typename TYPE, typename ACCUMULATOR>
 inline void DoTotalReduction(const Descriptor &x, int dim,
     const Descriptor *mask, ACCUMULATOR &accumulator, const char *intrinsic,
     Terminator &terminator) {
   if (dim < 0 || dim > 1) {
-    terminator.Crash(
-        "%s: bad DIM=%d for argument with rank %d", intrinsic, dim, x.rank());
+    terminator.Crash("%s: bad DIM=%d for ARRAY argument with rank %d",
+        intrinsic, dim, x.rank());
   }
   SubscriptValue xAt[maxRank];
   x.GetLowerBounds(xAt);
   if (mask) {
     CheckConformability(x, *mask, terminator, intrinsic, "ARRAY", "MASK");
     SubscriptValue maskAt[maxRank];
     mask->GetLowerBounds(maskAt);
     if (mask->rank() > 0) {

 
 // Utility: establishes & allocates the result array for a partial
 // reduction (i.e., one with DIM=).
 static void CreatePartialReductionResult(Descriptor &result,
     const Descriptor &x, int dim, Terminator &terminator, const char *intrinsic,
     TypeCode typeCode) {
   int xRank{x.rank()};
   if (dim < 1 || dim > xRank) {
-    terminator.Crash("%s: bad DIM=%d for rank %d", intrinsic, dim, xRank);
+    terminator.Crash(
+        "%s: bad DIM=%d for ARRAY with rank %d", intrinsic, dim, xRank);
   }
   int zeroBasedDim{dim - 1};
   SubscriptValue resultExtent[maxRank];
   for (int j{0}; j < zeroBasedDim; ++j) {
     resultExtent[j] = x.GetDimension(j).Extent();
   }
   for (int j{zeroBasedDim + 1}; j < xRank; ++j) {
     resultExtent[j - 1] = x.GetDimension(j).Extent();
case TypeCategory::Real:
     break;
   case TypeCategory::Complex:
     ApplyFloatingPointKind<PartialFloatingReductionHelper<TypeCategory::Complex,
                                COMPLEX_ACCUM>::template Functor,
         void>(catKind->second, terminator, result, x, dim, mask, terminator,
         intrinsic);
     break;
   default:
-    terminator.Crash("%s: invalid type code %d", intrinsic, x.type().raw());
+    terminator.Crash("%s: bad type code %d", intrinsic, x.type().raw());
   }
 }
 
 template <typename ACCUMULATOR> struct LocationResultHelper {
   template <int KIND> struct Functor {
     void operator()(ACCUMULATOR &accumulator, const Descriptor &result) const {
       accumulator.GetResult(
           result.OffsetElement<CppTypeFor<TypeCategory::Integer, KIND>>());

flang/runtime/reduction.cpp

 };
 
 template <typename ACCUMULATOR>
 inline auto GetTotalLogicalReduction(const Descriptor &x, const char *source,
     int line, int dim, ACCUMULATOR &&accumulator, const char *intrinsic) ->
     typename ACCUMULATOR::Type {
   Terminator terminator{source, line};
   if (dim < 0 || dim > 1) {
-    terminator.Crash("%s: bad DIM=%d", intrinsic, dim);
+    terminator.Crash("%s: bad DIM=%d for ARRAY with rank=1", intrinsic, dim);
   }
   SubscriptValue xAt[maxRank];
   x.GetLowerBounds(xAt);
   for (auto elements{x.Elements()}; elements--; x.IncrementSubscripts(xAt)) {
     if (!accumulator.AccumulateAt(xAt)) {
       break; // cut short, result is known
     }
   }

flang/runtime/transformational.cpp

RUNTIME_CHECK(terminator,
       vector.type() == field.type() && vector.ElementBytes() == elementLen);
   SubscriptValue resultAt[maxRank], maskAt[maxRank], fieldAt[maxRank],
       vectorAt{vector.GetDimension(0).LowerBound()};
   for (int j{0}; j < rank; ++j) {
     resultAt[j] = 1;
   }
   mask.GetLowerBounds(maskAt);
   field.GetLowerBounds(fieldAt);
-  SubscriptValue vectorLeft{vector.GetDimension(0).Extent()};
+  SubscriptValue vectorElements{vector.GetDimension(0).Extent()};
+  SubscriptValue vectorLeft{vectorElements};
   for (std::size_t n{result.Elements()}; n-- > 0;) {
     if (IsLogicalElementTrue(mask, maskAt)) {
       if (vectorLeft-- == 0) {
-        terminator.Crash("UNPACK: VECTOR= argument has fewer elements than "
-                         "MASK= has .TRUE. entries");
+        terminator.Crash(
+            "UNPACK: VECTOR= argument has fewer elements (%d) than "
+            "MASK= has .TRUE. entries",
+            vectorElements);
       }
       CopyElement(result, resultAt, vector, &vectorAt, terminator);
       ++vectorAt;
     } else {
       CopyElement(result, resultAt, field, fieldAt, terminator);
     }
     result.IncrementSubscripts(resultAt);
     mask.IncrementSubscripts(maskAt);
     field.IncrementSubscripts(fieldAt);
   }
 }
 
 } // extern "C"
 } // namespace Fortran::runtime

flang/runtime/unit.cpp

 ExternalFileUnit *ExternalFileUnit::LookUp(int unit) {
   return GetUnitMap().LookUp(unit);
 }
 
 ExternalFileUnit &ExternalFileUnit::LookUpOrCrash(
     int unit, const Terminator &terminator) {
   ExternalFileUnit *file{LookUp(unit)};
   if (!file) {
-    terminator.Crash("Not an open I/O unit number: %d", unit);
+    terminator.Crash("%d is not an open I/O unit number", unit);
   }
   return *file;
 }
 
 ExternalFileUnit &ExternalFileUnit::LookUpOrCreate(
     int unit, const Terminator &terminator, bool &wasExtant) {
   return GetUnitMap().LookUpOrCreate(unit, terminator, wasExtant);
 }
void ExternalFileUnit::CommitWrites() {
   recordOffsetInFrame_ = 0;
   BeginRecord();
 }
 
 bool ExternalFileUnit::CheckDirectAccess(IoErrorHandler &handler) {
   if (access == Access::Direct) {
     RUNTIME_CHECK(handler, openRecl);
     if (!directAccessRecWasSet_) {
-      handler.SignalError("No REC= was specified for a data transfer with ACCESS='DIRECT'");
+      handler.SignalError(
+          "No REC= was specified for a data transfer with ACCESS='DIRECT'");
       return false;
     }
   }
   return true;
 }
 
 ChildIo &ExternalFileUnit::PushChildIo(IoStatementState &parent) {
   OwningPtr<ChildIo> current{std::move(child_)};