[flang] Update format tests to use GTest

[ashermancinelli]

Summary

Update format tests to use GTest. Add some death tests for failure cases.

[awarzynski]

This patch refactors the original tests to use GTest and also adds some new ones. Thank you!

LGTM, but please look into the CI failures before merging.

[ashermancinelli]

Resolve warnings by removing utility function.

[awarzynski]

In D100765#2702519, @ashermancinelli wrote:

Resolve warnings by removing utility function.

Thank you for fixing the build. The new failures are clearly unrelated to this change and IMO it is safe to merge this.

[ashermancinelli]

Remove erroneous static qualifiers. Register all tests with crash handler.

[awarzynski]

flang/unittests/RuntimeGTest/Format.cpp

29–30

Could you copy the missing implementation across? Ta!

108

const auto& ? Otherwise there is going to be a warning, and all warnings are treated as errors in Flang :)

[ashermancinelli]

Resolve warnings against const auto without &.

[ashermancinelli]

Pull all of previous test harness into current implementation.

[ashermancinelli]

Thanks again for the rigorous review!

[awarzynski]

Looks good, thank you! The pre-commit CI failures are clearly unrelated to this patch.

flang/unittests/Runtime/CMakeLists.txt

 else()
   llvm_map_components_to_libnames(llvm_libs Support)
 endif()
 target_link_libraries(RuntimeTesting
   FortranRuntime
   ${llvm_libs}
 )
 
-add_flang_nongtest_unittest(format
-  RuntimeTesting
-  FortranRuntime
-)
-
 # This test is not run by default as it requires input.
 add_executable(external-hello-world
   external-hello.cpp
 )
 
 target_link_libraries(external-hello-world
   FortranRuntime
 )

flang/unittests/Runtime/format.cpp

-// Tests basic FORMAT string traversal
-
-#include "testing.h"
-#include "../runtime/format-implementation.h"
-#include "../runtime/io-error.h"
-#include <cstdarg>
-#include <cstring>
-#include <string>
-#include <vector>
-
-using namespace Fortran::runtime;
-using namespace Fortran::runtime::io;
-using namespace std::literals::string_literals;
-
-using Results = std::vector<std::string>;
-
-// A test harness context for testing FormatControl
-class TestFormatContext : public IoErrorHandler {
-public:
-  using CharType = char;
-  TestFormatContext() : IoErrorHandler{"format.cpp", 1} {}
-  bool Emit(const char *, std::size_t);
-  bool Emit(const char16_t *, std::size_t);
-  bool Emit(const char32_t *, std::size_t);
-  bool AdvanceRecord(int = 1);
-  void HandleRelativePosition(std::int64_t);
-  void HandleAbsolutePosition(std::int64_t);
-  void Report(const DataEdit &);
-  void Check(Results &);
-  Results results;
-  MutableModes &mutableModes() { return mutableModes_; }
-
-private:
-  MutableModes mutableModes_;
-};
-
-bool TestFormatContext::Emit(const char *s, std::size_t len) {
-  std::string str{s, len};
-  results.push_back("'"s + str + '\'');
-  return true;
-}
-bool TestFormatContext::Emit(const char16_t *, std::size_t) {
-  Crash("TestFormatContext::Emit(const char16_t *) called");
-  return false;
-}
-bool TestFormatContext::Emit(const char32_t *, std::size_t) {
-  Crash("TestFormatContext::Emit(const char32_t *) called");
-  return false;
-}
-
-bool TestFormatContext::AdvanceRecord(int n) {
-  while (n-- > 0) {
-    results.emplace_back("/");
-  }
-  return true;
-}
-
-void TestFormatContext::HandleAbsolutePosition(std::int64_t n) {
-  results.push_back("T"s + std::to_string(n));
-}
-
-void TestFormatContext::HandleRelativePosition(std::int64_t n) {
-  if (n < 0) {
-    results.push_back("TL"s + std::to_string(-n));
-  } else {
-    results.push_back(std::to_string(n) + 'X');
-  }
-}
-
-void TestFormatContext::Report(const DataEdit &edit) {
-  std::string str{edit.descriptor};
-  if (edit.repeat != 1) {
-    str = std::to_string(edit.repeat) + '*' + str;
-  }
-  if (edit.variation) {
-    str += edit.variation;
-  }
-  if (edit.width) {
-    str += std::to_string(*edit.width);
-  }
-  if (edit.digits) {
-    str += "."s + std::to_string(*edit.digits);
-  }
-  if (edit.expoDigits) {
-    str += "E"s + std::to_string(*edit.expoDigits);
-  }
-  // modes?
-  results.push_back(str);
-}
-
-void TestFormatContext::Check(Results &expect) {
-  if (expect != results) {
-    Fail() << "expected:";
-    for (const std::string &s : expect) {
-      llvm::errs() << ' ' << s;
-    }
-    llvm::errs() << "\ngot:";
-    for (const std::string &s : results) {
-      llvm::errs() << ' ' << s;
-    }
-    llvm::errs() << '\n';
-  }
-  expect.clear();
-  results.clear();
-}
-
-static void Test(int n, const char *format, Results &&expect, int repeat = 1) {
-  TestFormatContext context;
-  FormatControl<TestFormatContext> control{
-      context, format, std::strlen(format)};
-  try {
-    for (int j{0}; j < n; ++j) {
-      context.Report(control.GetNextDataEdit(context, repeat));
-    }
-    control.Finish(context);
-    if (int iostat{context.GetIoStat()}) {
-      context.Crash("GetIoStat() == %d", iostat);
-    }
-  } catch (const std::string &crash) {
-    context.results.push_back("Crash:"s + crash);
-  }
-  context.Check(expect);
-}
-
-int main() {
-  StartTests();
-  Test(1, "('PI=',F9.7)", Results{"'PI='", "F9.7"});
-  Test(1, "(3HPI=F9.7)", Results{"'PI='", "F9.7"});
-  Test(1, "(3HPI=/F9.7)", Results{"'PI='", "/", "F9.7"});
-  Test(2, "('PI=',F9.7)", Results{"'PI='", "F9.7", "/", "'PI='", "F9.7"});
-  Test(2, "(2('PI=',F9.7),'done')",
-      Results{"'PI='", "F9.7", "'PI='", "F9.7", "'done'"});
-  Test(2, "(3('PI=',F9.7,:),'tooFar')",
-      Results{"'PI='", "F9.7", "'PI='", "F9.7"});
-  Test(2, "(*('PI=',F9.7,:),'tooFar')",
-      Results{"'PI='", "F9.7", "'PI='", "F9.7"});
-  Test(1, "(3F9.7)", Results{"2*F9.7"}, 2);
-  return EndTests();
-}

flang/unittests/RuntimeGTest/CMakeLists.txt

 add_flang_unittest(FlangRuntimeTests
   CharacterTest.cpp
   CrashHandlerFixture.cpp
+  Format.cpp
   MiscIntrinsic.cpp
   Numeric.cpp
   NumericalFormatTest.cpp
   Random.cpp
   Reduction.cpp
   RuntimeCrashTest.cpp
   ListInputTest.cpp
 )
 
 target_link_libraries(FlangRuntimeTests
   PRIVATE
   FortranRuntime
 )

flang/unittests/RuntimeGTest/Format.cpp

+//===-- flang/unittests/RuntimeGTest/Format.cpp -----------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "CrashHandlerFixture.h"
+#include "../runtime/format-implementation.h"
+#include "../runtime/io-error.h"
+#include <string>
+#include <tuple>
+#include <vector>
+
+using namespace Fortran::runtime;
+using namespace Fortran::runtime::io;
+using namespace std::literals::string_literals;
+
+using ResultsTy = std::vector<std::string>;
+
+// A test harness context for testing FormatControl
+class TestFormatContext : public IoErrorHandler {
+public:
+  using CharType = char;
+  TestFormatContext() : IoErrorHandler{"format.cpp", 1} {}
+  bool Emit(const char *, std::size_t);
+  bool Emit(const char16_t *, std::size_t);
+  bool Emit(const char32_t *, std::size_t);
+  bool AdvanceRecord(int = 1);
+  void HandleRelativePosition(std::int64_t);
+  void HandleAbsolutePosition(std::int64_t);
+  void Report(const DataEdit &);
+  ResultsTy results;
+  MutableModes &mutableModes() { return mutableModes_; }
+
+private:
+  MutableModes mutableModes_;
+};
+
+bool TestFormatContext::Emit(const char *s, std::size_t len) {
+  std::string str{s, len};
+  results.push_back("'"s + str + '\'');
+  return true;
+}
+bool TestFormatContext::Emit(const char16_t *, std::size_t) {
+  Crash("TestFormatContext::Emit(const char16_t *) called");
+  return false;
+}
+bool TestFormatContext::Emit(const char32_t *, std::size_t) {
+  Crash("TestFormatContext::Emit(const char32_t *) called");
+  return false;
+}
+
+bool TestFormatContext::AdvanceRecord(int n) {
+  while (n-- > 0) {
+    results.emplace_back("/");
+  }
+  return true;
+}
+
+void TestFormatContext::HandleAbsolutePosition(std::int64_t n) {
+  results.push_back("T"s + std::to_string(n));
+}
+
+void TestFormatContext::HandleRelativePosition(std::int64_t n) {
+  if (n < 0) {
+    results.push_back("TL"s + std::to_string(-n));
+  } else {
+    results.push_back(std::to_string(n) + 'X');
+  }
+}
+
+void TestFormatContext::Report(const DataEdit &edit) {
+  std::string str{edit.descriptor};
+  if (edit.repeat != 1) {
+    str = std::to_string(edit.repeat) + '*' + str;
+  }
+  if (edit.variation) {
+    str += edit.variation;
+  }
+  if (edit.width) {
+    str += std::to_string(*edit.width);
+  }
+  if (edit.digits) {
+    str += "."s + std::to_string(*edit.digits);
+  }
+  if (edit.expoDigits) {
+    str += "E"s + std::to_string(*edit.expoDigits);
+  }
+  // modes?
+  results.push_back(str);
+}
+
+struct FormatTests : public CrashHandlerFixture {};
+
+TEST(FormatTests, FormatStringTraversal) {
+
+  using ParamsTy = std::tuple<int, const char *, ResultsTy, int>;
+
+  static const std::vector<ParamsTy> params{
+      {1, "('PI=',F9.7)", ResultsTy{"'PI='", "F9.7"}, 1},
+      {1, "(3HPI=F9.7)", ResultsTy{"'PI='", "F9.7"}, 1},
+      {1, "(3HPI=/F9.7)", ResultsTy{"'PI='", "/", "F9.7"}, 1},
+      {2, "('PI=',F9.7)", ResultsTy{"'PI='", "F9.7", "/", "'PI='", "F9.7"}, 1},
+      {2, "(2('PI=',F9.7),'done')",
+          ResultsTy{"'PI='", "F9.7", "'PI='", "F9.7", "'done'"}, 1},
+      {2, "(3('PI=',F9.7,:),'tooFar')",
+          ResultsTy{"'PI='", "F9.7", "'PI='", "F9.7"}, 1},
+      {2, "(*('PI=',F9.7,:),'tooFar')",
+          ResultsTy{"'PI='", "F9.7", "'PI='", "F9.7"}, 1},
+      {1, "(3F9.7)", ResultsTy{"2*F9.7"}, 2},
+  };
+
+  for (const auto &[n, format, expect, repeat] : params) {
+    TestFormatContext context;
+    FormatControl<decltype(context)> control{
+        context, format, std::strlen(format)};
+
+    for (auto i{0}; i < n; i++) {
+      context.Report(/*edit=*/control.GetNextDataEdit(context, repeat));
+    }
+    control.Finish(context);
+
+    auto iostat{context.GetIoStat()};
+    ASSERT_EQ(iostat, 0) << "Expected iostat == 0, but GetIoStat() == "
+                         << iostat;
+
+    // Create strings of the expected/actual results for printing errors
+    std::string allExpectedResults{""}, allActualResults{""};
+    for (const auto &res : context.results) {
+      allActualResults += " "s + res;
+    }
+    for (const auto &res : expect) {
+      allExpectedResults += " "s + res;
+    }
+
+    const auto &results = context.results;
+    ASSERT_EQ(expect, results) << "Expected '" << allExpectedResults
+                               << "' but got '" << allActualResults << "'";
+  }
+}
+
+struct InvalidFormatFailure : CrashHandlerFixture {};
+
+TEST(InvalidFormatFailure, ParenMismatch) {
+  static constexpr const char *format{"("};
+  static constexpr int repeat{1};
+
+  TestFormatContext context;
+  FormatControl<decltype(context)> control{
+      context, format, std::strlen(format)};
+
+  ASSERT_DEATH(
+      context.Report(/*edit=*/control.GetNextDataEdit(context, repeat)),
+      "FORMAT missing at least one ')'");
+}
+
+TEST(InvalidFormatFailure, MissingPrecision) {
+  static constexpr const char *format{"(F9.)"};
+  static constexpr int repeat{1};
+
+  TestFormatContext context;
+  FormatControl<decltype(context)> control{
+      context, format, std::strlen(format)};
+
+  ASSERT_DEATH(
+      context.Report(/*edit=*/control.GetNextDataEdit(context, repeat)),
+      "Invalid FORMAT: integer expected at ')'");
+}
+
+TEST(InvalidFormatFailure, MissingFormatWidth) {
+  static constexpr const char *format{"(F.9)"};
+  static constexpr int repeat{1};
+
+  TestFormatContext context;
+  FormatControl<decltype(context)> control{
+      context, format, std::strlen(format)};
+
+  ASSERT_DEATH(
+      context.Report(/*edit=*/control.GetNextDataEdit(context, repeat)),
+      "Invalid FORMAT: integer expected at '.'");
+}