Fix a bug in the character runtime

[rovka]

Summary

The number of bytes copied in CopyAndPad should depend on the size of
the type being copied, not on its shift value (which in the case of char
is 0, leading to no bytes at all being copied).

Add unit tests for CharacterMin and CharacterMax, which exercise this
code path.

[rovka]

• Enabled the tests for all character types (and added dependencies for the fixes that made that possible)
• Simplified test expectation (no need to dump raw pointer values)

flang/runtime/character.cpp

if constexpr (sizeof(TO) != sizeof(FROM)) {
     std::size_t copyChars{std::min(toChars, fromChars)};
     for (std::size_t j{0}; j < copyChars; ++j) {
       to[j] = from[j];
     }
     for (std::size_t j{copyChars}; j < toChars; ++j) {
       to[j] = static_cast<TO>(' ');
     }
   } else if (toChars <= fromChars) {
-    std::memcpy(to, from, toChars * shift<TO>);
+    std::memcpy(to, from, toChars * sizeof(TO));
   } else {
-    std::memcpy(to, from, fromChars * shift<TO>);
+    std::memcpy(to, from, fromChars * sizeof(TO));
     for (std::size_t j{fromChars}; j < toChars; ++j) {
       to[j] = static_cast<TO>(' ');
     }
   }
 }
 
 template <typename CHAR, bool ISMIN>
 static void MaxMinHelper(Descriptor &accumulator, const Descriptor &x,

flang/unittests/RuntimeGTest/CharacterTest.cpp

 //===-- flang/unittests/RuntimeGTest/CharacterTest.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
 //
 //===----------------------------------------------------------------------===//
 
 // Basic sanity tests of CHARACTER API; exhaustive testing will be done
 // in Fortran.
 
 #include "../../runtime/character.h"
+#include "../../runtime/descriptor.h"
 #include "gtest/gtest.h"
 #include <cstring>
 #include <functional>
 #include <tuple>
 #include <vector>
 
 using namespace Fortran::runtime;
 
 TEST(CharacterTests, AppendAndPad) {
for (auto &[x, y, xBytes, yBytes, expect] : this->parameters) {
     std::swap(xBytes, yBytes);
     expect = -expect;
 
     cmp = this->characterComparisonFunc(x, y, xBytes, yBytes);
     std::memset(buf, 0, sizeof buf);
     std::memcpy(buf[0], x, xBytes);
     std::memcpy(buf[1], y, yBytes);
     ASSERT_EQ(cmp, expect) << "compare '" << x << "'(" << xBytes << ") to '"
-                           << y << "'(" << yBytes << "), got " << cmp
+                           << y << "'(" << yBytes << "'), got " << cmp
                            << ", should be " << expect << '\n';
   }
 }
 
+// Test MIN() and MAX()
+struct ExtremumTestCase {
+  const char *x, *y, *expect;
+};
+
+template <typename CHAR>
+void RunExtremumTests(const char *which,
+    std::function<void(Descriptor &, const Descriptor &, const char *, int)>
+        function,
+    const std::vector<ExtremumTestCase> &testCases) {
+
+  // Helper for creating, allocating and filling up a descriptor with data from
+  // a raw character literal, converted to the CHAR type used by the test.
+  auto CreateDescriptor = [](const char *raw) -> OwningPtr<Descriptor> {
+    std::size_t length{std::strlen(raw)};
+    std::basic_string<CHAR> converted{raw, raw + length};
+
+    OwningPtr<Descriptor> descriptor{Descriptor::Create(
+        sizeof(CHAR), length, nullptr, 0, nullptr, CFI_attribute_allocatable)};
+    if (descriptor->Allocate() != 0) {
+      return nullptr;
+    }
+
+    std::copy(
+        converted.begin(), converted.end(), descriptor->OffsetElement<CHAR>());
+
+    return descriptor;
+  };
+
+  for (const auto &t : testCases) {
+    OwningPtr<Descriptor> x = CreateDescriptor(t.x);
+    OwningPtr<Descriptor> y = CreateDescriptor(t.y);
+
+    ASSERT_NE(x, nullptr);
+    ASSERT_TRUE(x->IsAllocated());
+    ASSERT_NE(y, nullptr);
+    ASSERT_TRUE(y->IsAllocated());
+    function(*x, *y, nullptr, 0);
+
+    std::basic_string<CHAR> got{
+        x->OffsetElement<CHAR>(), x->Elements() * x->ElementBytes()};
+    std::basic_string<CHAR> expect{t.expect, t.expect + std::strlen(t.expect)};
+    EXPECT_EQ(expect, got) << which << "('" << t.x << "','" << t.y
+                           << "') for CHARACTER(kind=" << sizeof(CHAR)
+                           << "): got " << got.data() << ", expected "
+                           << expect.data();
+  }
+}
+
+template <typename CHAR> struct ExtremumTests : public ::testing::Test {};
+TYPED_TEST_CASE(ExtremumTests, char); // FIXME: Enable for all CharacterTypes.
+
+TYPED_TEST(ExtremumTests, MinTests) {
+  static std::vector<ExtremumTestCase> tests{
+      {"a", "z", "a"},
+      {"zaaa", "aa", "aa  "},
+      {"aaz", "aaaaa", "aaaaa"},
+  };
+  RunExtremumTests<TypeParam>("MIN", RTNAME(CharacterMin), tests);
+}
+
+TYPED_TEST(ExtremumTests, MaxTests) {
+  static std::vector<ExtremumTestCase> tests{
+      {"a", "z", "z"},
+      {"zaa", "aaaaa", "zaa  "},
+      {"aaaaa", "aazaa", "aazaa"},
+  };
+  RunExtremumTests<TypeParam>("MAX", RTNAME(CharacterMax), tests);
+}
+
 // Test search functions INDEX(), SCAN(), and VERIFY()
 
 template <typename CHAR>
 using SearchFunction = std::function<std::size_t(
     const CHAR *, std::size_t, const CHAR *, std::size_t, bool)>;
 template <template <typename> class FUNC>
 using CharTypedFunctions =
     std::tuple<FUNC<char>, FUNC<char16_t>, FUNC<char32_t>>;