From 8e575bd507a6f4eb5770707e092d9ffccafa01f3 Mon Sep 17 00:00:00 2001
From: Walter Gray <walter.gray@getcruise.com>
Date: Sat, 12 Dec 2020 16:55:02 -0800
Subject: [PATCH] address comments

---
 include/fmt/chrono.h                          | 22 ------
 test/CMakeLists.txt                           | 10 ++-
 ...test.cc => enforce-compile-string-test.cc} | 67 ++++---------------
 3 files changed, 19 insertions(+), 80 deletions(-)
 rename test/{enforce-compiletime-test.cc => enforce-compile-string-test.cc} (60%)

diff --git a/include/fmt/chrono.h b/include/fmt/chrono.h
index 8e0cca56..417ff992 100644
--- a/include/fmt/chrono.h
+++ b/include/fmt/chrono.h
@@ -770,14 +770,8 @@ template <typename Char, typename Rep, typename OutputIt,
 OutputIt format_duration_value(OutputIt out, Rep val, int) {
   static FMT_CONSTEXPR_DECL const Char format[] = {'{', '}', 0};
 
-// Note(12/3/2020): Workaround an as-of-yet unfixed compiler error in MSVC.
-// See https://developercommunity.visualstudio.com/content/problem/1277597/internal-compiler-c0001-error-on-complex-nested-la.html
-#if FMT_MSC_VER
   return vformat_to(out, to_string_view(format),
                     make_format_args<buffer_context<Char>>(val));
-#else
-  return format_to(out, FMT_STRING(format), val);
-#endif
 }
 
 template <typename Char, typename Rep, typename OutputIt,
@@ -786,21 +780,13 @@ OutputIt format_duration_value(OutputIt out, Rep val, int precision) {
   static FMT_CONSTEXPR_DECL const Char pr_f[] = {'{', ':', '.', '{',
                                                  '}', 'f', '}', 0};
   if (precision >= 0) {
-#if FMT_MSC_VER
     return vformat_to(out, to_string_view(pr_f),
                       make_format_args<buffer_context<Char>>(val, precision));
-#else
-    return format_to(out, FMT_STRING(pr_f), val, precision);
-#endif
   }
   static FMT_CONSTEXPR_DECL const Char fp_f[] = {'{', ':', 'g', '}', 0};
 
-#if FMT_MSC_VER
   return vformat_to(out, to_string_view(fp_f),
                     make_format_args<buffer_context<Char>>(val));
-#else
-  return format_to(out, FMT_STRING(fp_f), val);
-#endif
 }
 
 template <typename Char, typename OutputIt>
@@ -822,22 +808,14 @@ OutputIt format_duration_unit(OutputIt out) {
     return copy_unit(string_view(unit), out, Char());
   static FMT_CONSTEXPR_DECL const Char num_f[] = {'[', '{', '}', ']', 's', 0};
   if (const_check(Period::den == 1)) {
-#if FMT_MSC_VER
     return vformat_to(out, to_string_view(num_f),
                       make_format_args<buffer_context<Char>>(Period::num));
-#else
-    return format_to(out, FMT_STRING(num_f), Period::num);
-#endif
   }
   static FMT_CONSTEXPR_DECL const Char num_def_f[] = {'[', '{', '}', '/', '{',
                                                       '}', ']', 's', 0};
-#if FMT_MSC_VER
   return vformat_to(
       out, to_string_view(num_def_f),
       make_format_args<buffer_context<Char>>(Period::num, Period::den));
-#else
-  return format_to(out, FMT_STRING(num_def_f), Period::num, Period::den);
-#endif
 }
 
 template <typename FormatContext, typename OutputIt, typename Rep,
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index e6f95733..f34a71ee 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -106,9 +106,13 @@ add_fmt_test(printf-test)
 add_fmt_test(ranges-test)
 add_fmt_test(scan-test)
 
-
-add_fmt_test(enforce-compiletime-test)
-target_compile_definitions(enforce-compiletime-test PRIVATE "-DFMT_ENFORCE_COMPILE_STRING")
+if(NOT MSVC)
+  # FMT_ENFORCE_COMPILE_STRING not supported under MSVC
+  # See https://developercommunity.visualstudio.com/content/problem/1277597/internal-compiler-c0001-error-on-complex-nested-la.html
+  add_fmt_test(enforce-compile-string-test)
+  target_compile_definitions(enforce-compile-string-test PRIVATE
+    "-DFMT_ENFORCE_COMPILE_STRING")
+endif()
 
 if (NOT DEFINED MSVC_STATIC_RUNTIME AND MSVC)
   foreach (flag_var
diff --git a/test/enforce-compiletime-test.cc b/test/enforce-compile-string-test.cc
similarity index 60%
rename from test/enforce-compiletime-test.cc
rename to test/enforce-compile-string-test.cc
index 4ae943e0..79a5cf8d 100644
--- a/test/enforce-compiletime-test.cc
+++ b/test/enforce-compile-string-test.cc
@@ -14,10 +14,6 @@
 #include <utility>
 #include <vector>
 
-#ifdef WIN32
-#  define _CRT_SECURE_NO_WARNINGS
-#endif
-
 #include "fmt/chrono.h"
 #include "fmt/color.h"
 #include "fmt/format.h"
@@ -26,20 +22,16 @@
 #include "fmt/ranges.h"
 
 // Exercise the API to verify that everything we expect to can compile.
-void TestFormatApi() {
+void test_format_api() {
   (void)fmt::format(FMT_STRING("{}"), 42);
   (void)fmt::format(FMT_STRING(L"{}"), 42);
-#if !FMT_GCC_VERSION
   (void)fmt::format(FMT_STRING("noop"));
-#endif
 
   (void)fmt::to_string(42);
   (void)fmt::to_wstring(42);
 
   std::list<char> out;
   fmt::format_to(std::back_inserter(out), FMT_STRING("{}"), 42);
-  std::stringstream s;
-  fmt::format_to(std::ostream_iterator<char>(s), FMT_STRING("{}"), 42);
 
   char buffer[4];
   (void)fmt::format_to_n(buffer, 3, FMT_STRING("{}"), 12345);
@@ -47,12 +39,9 @@ void TestFormatApi() {
   wchar_t wbuffer[4];
   (void)fmt::format_to_n(wbuffer, 3, FMT_STRING(L"{}"), 12345);
 }
-void TestLiteralsApi() {
-#if FMT_USE_UDL_TEMPLATE
-  // Passing user-defined literals directly to EXPECT_EQ causes problems
-  // with macro argument stringification (#) on some versions of GCC.
-  // Workaround: Assing the UDL result to a variable before the macro.
 
+void test_literals_api() {
+#if FMT_USE_UDL_TEMPLATE
   using namespace fmt::literals;
 
   auto udl_format = "{}c{}"_format("ab", 1);
@@ -62,40 +51,12 @@ void TestLiteralsApi() {
 #endif
 }
 
-struct test_output_iterator {
-  char* data;
-
-  using iterator_category = std::output_iterator_tag;
-  using value_type = void;
-  using difference_type = void;
-  using pointer = void;
-  using reference = void;
-
-  test_output_iterator& operator++() {
-    ++data;
-    return *this;
-  }
-  test_output_iterator operator++(int) {
-    auto tmp = *this;
-    ++data;
-    return tmp;
-  }
-  char& operator*() { return *data; }
-};
-
-void FormatToNOutputIteratorTest() {
-  char buf[10] = {};
-  fmt::format_to_n(test_output_iterator{buf}, 10, FMT_STRING("{}"), 42);
-}
-
-void TestChrono() {
-#ifndef FMT_STATIC_THOUSANDS_SEPARATOR
+void test_chrono() {
   (void)fmt::format(FMT_STRING("{}"), std::chrono::seconds(42));
   (void)fmt::format(FMT_STRING(L"{}"), std::chrono::seconds(42));
-#endif  // FMT_STATIC_THOUSANDS_SEPARATOR
 }
 
-void TestTextStyle() {
+void test_text_style() {
   fmt::print(fg(fmt::rgb(255, 20, 30)), FMT_STRING("{}"), "rgb(255,20,30)");
   (void)fmt::format(fg(fmt::rgb(255, 20, 30)), FMT_STRING("{}"),
                     "rgb(255,20,30)");
@@ -117,19 +78,15 @@ struct zstring {
   zstring_sentinel end() const { return {}; }
 };
 
-void TestZString() {
+void test_zstring() {
   zstring hello{"hello"};
   (void)fmt::format(FMT_STRING("{}"), hello);
-  (void)fmt::format(FMT_STRING("{}"), fmt::join(hello, "_"));
 }
 
-int main(int, char**) {
-  TestFormatApi();
-  TestLiteralsApi();
-  FormatToNOutputIteratorTest();
-  TestChrono();
-  TestTextStyle();
-  TestZString();
-
-  return 0;
+int main() {
+  test_format_api();
+  test_literals_api();
+  test_chrono();
+  test_text_style();
+  test_zstring();
 }