[flang] Improve runtime crash messages

psteinfeld · jeanPerier · commit 04cdacf0f071 · 2022-03-17T10:09:07.000+01:00
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. Differential Revision: https://reviews.llvm.org/D121493
diff --git a/flang/runtime/descriptor-io.h b/flang/runtime/descriptor-io.h
@@ -480,7 +480,7 @@ static bool DescriptorIO(IoStatementState &io, const Descriptor &descriptor) {
       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;
 }
diff --git a/flang/runtime/extrema.cpp b/flang/runtime/extrema.cpp
@@ -176,7 +176,7 @@ inline void TypedMaxOrMinLoc(const char *intrinsic, Descriptor &result,
     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());
   }
 }
 
@@ -262,7 +262,7 @@ inline void TypedPartialMaxOrMinLoc(const char *intrinsic, Descriptor &result,
     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());
   }
 }
 
diff --git a/flang/runtime/findloc.cpp b/flang/runtime/findloc.cpp
@@ -248,7 +248,7 @@ void RTNAME(Findloc)(Descriptor &result, const Descriptor &x,
     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"
@@ -335,7 +335,7 @@ void RTNAME(FindlocDim)(Descriptor &result, const Descriptor &x,
     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"
diff --git a/flang/runtime/inquiry.cpp b/flang/runtime/inquiry.cpp
@@ -23,7 +23,8 @@ 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());
@@ -68,7 +69,8 @@ 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());
diff --git a/flang/runtime/io-api.cpp b/flang/runtime/io-api.cpp
@@ -564,7 +564,7 @@ bool IONAME(SetPos)(Cookie cookie, std::int64_t pos) {
     unit->SetPosition(pos - 1, handler);
     return true;
   }
-  io.GetIoErrorHandler().Crash("SetPos() on internal unit");
+  io.GetIoErrorHandler().Crash("SetPos() called on internal unit");
   return false;
 }
 
@@ -937,7 +937,7 @@ bool IONAME(GetNewUnit)(Cookie cookie, int &unit, int kind) {
   }
   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));
   }
@@ -1209,7 +1209,8 @@ bool IONAME(InquireInteger64)(
       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));
   }
diff --git a/flang/runtime/io-stmt.cpp b/flang/runtime/io-stmt.cpp
@@ -78,7 +78,7 @@ bool IoStatementBase::Inquire(InquiryKeywordHash, std::int64_t &) {
 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)");
 }
 
diff --git a/flang/runtime/iostat.cpp b/flang/runtime/iostat.cpp
@@ -30,7 +30,7 @@ const char *IostatErrorString(int iostat) {
   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:
diff --git a/flang/runtime/random.cpp b/flang/runtime/random.cpp
@@ -108,7 +108,7 @@ void RTNAME(RandomNumber)(
     break;
 #endif
   default:
-    terminator.Crash("RANDOM_NUMBER(): unsupported REAL kind %d", kind);
+    terminator.Crash("RANDOM_NUMBER(): bad REAL kind %d", kind);
   }
 }
 
diff --git a/flang/runtime/reduction-templates.h b/flang/runtime/reduction-templates.h
@@ -44,8 +44,8 @@ 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);
@@ -166,7 +166,8 @@ static void CreatePartialReductionResult(Descriptor &result,
     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];
@@ -295,7 +296,7 @@ inline void TypedPartialNumericReduction(Descriptor &result,
         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());
   }
 }
 
diff --git a/flang/runtime/reduction.cpp b/flang/runtime/reduction.cpp
@@ -228,7 +228,7 @@ inline auto GetTotalLogicalReduction(const Descriptor &x, const char *source,
     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);
diff --git a/flang/runtime/transformational.cpp b/flang/runtime/transformational.cpp
@@ -525,12 +525,15 @@ void RTNAME(Unpack)(Descriptor &result, const Descriptor &vector,
   }
   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;
diff --git a/flang/runtime/unit.cpp b/flang/runtime/unit.cpp
@@ -48,7 +48,7 @@ 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;
 }
@@ -855,7 +855,8 @@ 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;
     }
   }

Original file line number	Diff line number	Diff line change
`@@ -480,7 +480,7 @@ static bool DescriptorIO(IoStatementState &io, const Descriptor &descriptor) {`
`480`	`480`	`return FormattedDerivedTypeIO<DIR>(io, descriptor);`
`481`	`481`	`}`
`482`	`482`	`}`
`483`		`- handler.Crash("DescriptorIO: Bad type code (%d) in descriptor",`
	`483`	`+ handler.Crash("DescriptorIO: bad type code (%d) in descriptor",`
`484`	`484`	`static_cast<int>(descriptor.type().raw()));`
`485`	`485`	`return false;`
`486`	`486`	`}`
Original file line number	Diff line number	Diff line change
`@@ -176,7 +176,7 @@ inline void TypedMaxOrMinLoc(const char *intrinsic, Descriptor &result,`
`176`	`176`	`break;`
`177`	`177`	`default:`
`178`	`178`	`terminator.Crash(`
`179`		`- "%s: Bad data type code (%d) for array", intrinsic, x.type().raw());`
	`179`	`+ "%s: bad data type code (%d) for array", intrinsic, x.type().raw());`
`180`	`180`	`}`
`181`	`181`	`}`
`182`	`182`
`@@ -262,7 +262,7 @@ inline void TypedPartialMaxOrMinLoc(const char *intrinsic, Descriptor &result,`
`262`	`262`	`break;`
`263`	`263`	`default:`
`264`	`264`	`terminator.Crash(`
`265`		`- "%s: Bad data type code (%d) for array", intrinsic, x.type().raw());`
	`265`	`+ "%s: bad data type code (%d) for array", intrinsic, x.type().raw());`
`266`	`266`	`}`
`267`	`267`	`}`
`268`	`268`
Original file line number	Diff line number	Diff line change
`@@ -248,7 +248,7 @@ void RTNAME(Findloc)(Descriptor &result, const Descriptor &x,`
`248`	`248`	`break;`
`249`	`249`	`default:`
`250`	`250`	`terminator.Crash(`
`251`		`- "FINDLOC: Bad data type code (%d) for array", x.type().raw());`
	`251`	`+ "FINDLOC: bad data type code (%d) for array", x.type().raw());`
`252`	`252`	`}`
`253`	`253`	`}`
`254`	`254`	`} // extern "C"`
`@@ -335,7 +335,7 @@ void RTNAME(FindlocDim)(Descriptor &result, const Descriptor &x,`
`335`	`335`	`break;`
`336`	`336`	`default:`
`337`	`337`	`terminator.Crash(`
`338`		`- "FINDLOC: Bad data type code (%d) for array", x.type().raw());`
	`338`	`+ "FINDLOC: bad data type code (%d) for array", x.type().raw());`
`339`	`339`	`}`
`340`	`340`	`}`
`341`	`341`	`} // extern "C"`
Original file line number	Diff line number	Diff line change
`@@ -564,7 +564,7 @@ bool IONAME(SetPos)(Cookie cookie, std::int64_t pos) {`
`564`	`564`	`unit->SetPosition(pos - 1, handler);`
`565`	`565`	`return true;`
`566`	`566`	`}`
`567`		`- io.GetIoErrorHandler().Crash("SetPos() on internal unit");`
	`567`	`+ io.GetIoErrorHandler().Crash("SetPos() called on internal unit");`
`568`	`568`	`return false;`
`569`	`569`	`}`
`570`	`570`
`@@ -937,7 +937,7 @@ bool IONAME(GetNewUnit)(Cookie cookie, int &unit, int kind) {`
`937`	`937`	`}`
`938`	`938`	`std::int64_t result{open->unit().unitNumber()};`
`939`	`939`	`if (!SetInteger(unit, kind, result)) {`
`940`		`- open->SignalError("GetNewUnit(): Bad INTEGER kind(%d) or out-of-range "`
	`940`	`+ open->SignalError("GetNewUnit(): bad INTEGER kind(%d) or out-of-range "`
`941`	`941`	`"value(%jd) for result",`
`942`	`942`	`kind, static_cast<std::intmax_t>(result));`
`943`	`943`	`}`
`@@ -1209,7 +1209,8 @@ bool IONAME(InquireInteger64)(`
`1209`	`1209`	`return true;`
`1210`	`1210`	`}`
`1211`	`1211`	`io.GetIoErrorHandler().SignalError(`
`1212`		`- "InquireInteger64(): Bad INTEGER kind(%d) or out-of-range value(%jd) "`
	`1212`	`+ "InquireInteger64(): bad INTEGER kind(%d) or out-of-range "`
	`1213`	`+ "value(%jd) "`
`1213`	`1214`	`"for result",`
`1214`	`1215`	`kind, static_cast<std::intmax_t>(n));`
`1215`	`1216`	`}`
Original file line number	Diff line number	Diff line change
`@@ -78,7 +78,7 @@ bool IoStatementBase::Inquire(InquiryKeywordHash, std::int64_t &) {`
`78`	`78`	`void IoStatementBase::BadInquiryKeywordHashCrash(InquiryKeywordHash inquiry) {`
`79`	`79`	`char buffer[16];`
`80`	`80`	`const char *decode{InquiryKeywordHashDecode(buffer, sizeof buffer, inquiry)};`
`81`		`- Crash("bad InquiryKeywordHash 0x%x (%s)", inquiry,`
	`81`	`+ Crash("Bad InquiryKeywordHash 0x%x (%s)", inquiry,`
`82`	`82`	`decode ? decode : "(cannot decode)");`
`83`	`83`	`}`
`84`	`84`
Original file line number	Diff line number	Diff line change
`@@ -108,7 +108,7 @@ void RTNAME(RandomNumber)(`
`108`	`108`	`break;`
`109`	`109`	`#endif`
`110`	`110`	`default:`
`111`		`- terminator.Crash("RANDOM_NUMBER(): unsupported REAL kind %d", kind);`
	`111`	`+ terminator.Crash("RANDOM_NUMBER(): bad REAL kind %d", kind);`
`112`	`112`	`}`
`113`	`113`	`}`
`114`	`114`
Original file line number	Diff line number	Diff line change
`@@ -228,7 +228,7 @@ inline auto GetTotalLogicalReduction(const Descriptor &x, const char *source,`
`228`	`228`	`typename ACCUMULATOR::Type {`
`229`	`229`	`Terminator terminator{source, line};`
`230`	`230`	`if (dim < 0 \|\| dim > 1) {`
`231`		`- terminator.Crash("%s: bad DIM=%d", intrinsic, dim);`
	`231`	`+ terminator.Crash("%s: bad DIM=%d for ARRAY with rank=1", intrinsic, dim);`
`232`	`232`	`}`
`233`	`233`	`SubscriptValue xAt[maxRank];`
`234`	`234`	`x.GetLowerBounds(xAt);`