Fix partial specialization problem for filesystem for Visual Studio (#…

…2957)

* #2954: Provide std::conjunction and std::disjunction substitutes

* #2954: Use conjunction and disjunction substitute to make formatter specializations for ranges and maps more robust (especially for Visual Studio compiler family)

* #2954: As workaround for older MSVC compilers split formatter<std::filesystem::path> partial template specialization into two explicit specialization.

* 2954: Add test case

* Provide simplified implementations of conjunction and disjunction

* Remove workaround explicit specializations if the partial specialization would cause an ambiguity error

* Eliminate extra-test and merge it into existing std-test instead. Add conditionals for filesystem::path testing that does not run into the ambiguity problem.

include/fmt/core.h

@@ -306,6 +306,24 @@ template <typename T> using type_identity_t = typename type_identity<T>::type;
 template <typename T>
 using underlying_t = typename std::underlying_type<T>::type;
 
+template <typename...>
+struct disjunction : std::false_type {};
+template <typename P>
+struct disjunction<P> : P {};
+template <typename P1, typename... Pn>
+struct disjunction<P1, Pn...> 
+  : conditional_t<bool(P1::value), P1, disjunction<Pn...>> {
+};
+
+template <typename...>
+struct conjunction : std::true_type {};
+template <typename P>
+struct conjunction<P> : P {};
+template <typename P1, typename... Pn>
+struct conjunction<P1, Pn...>
+  : conditional_t<bool(P1::value), conjunction<Pn...>, P1> {
+};
+
 struct monostate {
   constexpr monostate() {}
 };

include/fmt/ranges.h

@@ -246,7 +246,7 @@ template <class Tuple, class F> void for_each(Tuple&& tup, F&& f) {
   for_each(indexes, std::forward<Tuple>(tup), std::forward<F>(f));
 }
 
-#if FMT_MSC_VERSION
+#if FMT_MSC_VERSION && FMT_MSC_VERSION < 1920
 // Older MSVC doesn't get the reference type correctly for arrays.
 template <typename R> struct range_reference_type_impl {
   using type = decltype(*detail::range_begin(std::declval<R&>()));
@@ -396,15 +396,18 @@ template <typename R, typename Char>
 struct formatter<
     R, Char,
     enable_if_t<
-        fmt::is_range<R, Char>::value
-// Workaround a bug in MSVC 2019 and earlier.
-#if !FMT_MSC_VERSION
-        &&
-        (is_formattable<detail::uncvref_type<detail::maybe_const_range<R>>,
-                        Char>::value ||
-         detail::has_fallback_formatter<
-             detail::uncvref_type<detail::maybe_const_range<R>>, Char>::value)
+        conjunction<fmt::is_range<R, Char>
+// Workaround a bug in MSVC 2017 and earlier.
+#if !FMT_MSC_VERSION || FMT_MSC_VERSION >= 1920
+        ,
+        disjunction<
+          is_formattable<detail::uncvref_type<detail::maybe_const_range<R>>,
+                         Char>,
+          detail::has_fallback_formatter<
+             detail::uncvref_type<detail::maybe_const_range<R>>, Char>
+        >
 #endif
+        >::value
         >> {
 
   using range_type = detail::maybe_const_range<R>;
@@ -457,14 +460,20 @@ struct formatter<
 template <typename T, typename Char>
 struct formatter<
     T, Char,
-    enable_if_t<detail::is_map<T>::value
-// Workaround a bug in MSVC 2019 and earlier.
-#if !FMT_MSC_VERSION
-        && (is_formattable<detail::uncvref_first_type<T>, Char>::value ||
-            detail::has_fallback_formatter<detail::uncvref_first_type<T>, Char>::value)
-        && (is_formattable<detail::uncvref_second_type<T>, Char>::value ||
-            detail::has_fallback_formatter<detail::uncvref_second_type<T>, Char>::value)
+    enable_if_t<conjunction<detail::is_map<T>
+// Workaround a bug in MSVC 2017 and earlier.
+#if !FMT_MSC_VERSION || FMT_MSC_VERSION >= 1920
+        ,
+        disjunction<
+          is_formattable<detail::uncvref_first_type<T>, Char>,
+          detail::has_fallback_formatter<detail::uncvref_first_type<T>, Char>
+        >,
+        disjunction<
+          is_formattable<detail::uncvref_second_type<T>, Char>,
+          detail::has_fallback_formatter<detail::uncvref_second_type<T>, Char>
+        >
 #endif
+    >::value
     >> {
   template <typename ParseContext>
   FMT_CONSTEXPR auto parse(ParseContext& ctx) -> decltype(ctx.begin()) {

include/fmt/std.h

@@ -57,6 +57,10 @@ inline void write_escaped_path<std::filesystem::path::value_type>(
 
 }  // namespace detail
 
+#if !FMT_MSC_VERSION || FMT_MSC_VERSION >= 1920
+// For MSVC 2017 and earlier using the partial specialization
+// would cause an ambiguity error, therefore we provide it only
+// conditionally.
 template <typename Char>
 struct formatter<std::filesystem::path, Char>
     : formatter<basic_string_view<Char>> {
@@ -69,6 +73,7 @@ struct formatter<std::filesystem::path, Char>
         basic_string_view<Char>(quoted.data(), quoted.size()), ctx);
   }
 };
+#endif
 FMT_END_NAMESPACE
 #endif
 

test/std-test.cc

@@ -6,13 +6,18 @@
 // For the license information refer to format.h.
 
 #include "fmt/std.h"
+#include "fmt/ranges.h"
 
 #include <string>
+#include <vector>
 
 #include "gtest/gtest.h"
 
 TEST(std_test, path) {
-#ifdef __cpp_lib_filesystem
+// Test ambiguity problem described in #2954. We need to exclude compilers
+// where the ambiguity problem cannot be solved for now.
+#if defined(__cpp_lib_filesystem) && \
+    (!FMT_MSC_VERSION || FMT_MSC_VERSION >= 1920)
   EXPECT_EQ(fmt::format("{:8}", std::filesystem::path("foo")), "\"foo\"   ");
   EXPECT_EQ(fmt::format("{}", std::filesystem::path("foo\"bar.txt")),
             "\"foo\\\"bar.txt\"");
@@ -31,6 +36,18 @@ TEST(std_test, path) {
 #endif
 }
 
+TEST(ranges_std_test, format_vector_path) {
+// Test ambiguity problem described in #2954. We need to exclude compilers
+// where the ambiguity problem cannot be solved for now.
+#if defined(__cpp_lib_filesystem) && \
+    (!FMT_MSC_VERSION || FMT_MSC_VERSION >= 1920)
+  auto p = std::filesystem::path("foo/bar.txt");
+  auto c = std::vector<std::string>{"abc", "def"};
+  EXPECT_EQ(fmt::format("path={}, range={}", p, c),
+            "path=\"foo/bar.txt\", range=[\"abc\", \"def\"]");
+#endif
+}
+
 TEST(std_test, thread_id) {
   EXPECT_FALSE(fmt::format("{}", std::this_thread::get_id()).empty());
 }