From 883b40886558ca250fe1377abc4fc4b9af3dd7e8 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Thu, 8 Dec 2016 11:31:19 +0000 Subject: [PATCH] Improve format member detection in llvm::formatv Summary: The existing detection of a format member function has a couple of deficiencies: - the member function does not get detected if one calls formatv with an lvalue, because the template parameter gets deduced as T&, which fails the is_class check. - it also did not work if the function was called with a const variable because the template parameter would get deduced as const T&, again failing the is_class check. This fixes the problem by stripping the references in the uses_format_member template, to make sure the type is correctly detected as class. It also provides specializations of the has_FormatMember template for const and non-const members of the types in order to enable declaring the format member as a "const" function. I have added tests that verify that formatv can be now called in these scenarios. As some scenarios could not be verified at runtime (e.g. making sure that calling a non-const format member on a const object does *not* compile), I have also added some static_asserts which test the behaviour of the template classes used internally by formatv(). Reviewers: zturner Subscribers: llvm-commits Differential Revision: https://reviews.llvm.org/D27525 llvm-svn: 289040 --- include/llvm/Support/FormatVariadicDetails.h | 42 ++++++++++++--- unittests/Support/FormatVariadicTest.cpp | 56 +++++++++++++++++++- 2 files changed, 88 insertions(+), 10 deletions(-) diff --git a/include/llvm/Support/FormatVariadicDetails.h b/include/llvm/Support/FormatVariadicDetails.h index edde6db162f..77655373fa7 100644 --- a/include/llvm/Support/FormatVariadicDetails.h +++ b/include/llvm/Support/FormatVariadicDetails.h @@ -56,6 +56,7 @@ template class missing_format_wrapper; // // void format(raw_ostream &, StringRef); // +// It is assumed T is a non-reference type. template class has_FormatMember { public: static bool const value = false; @@ -63,8 +64,11 @@ public: template class has_FormatMember::value>::type> { - using Signature_format = void (T::*)(llvm::raw_ostream &S, StringRef Options); + typename std::enable_if::value && + std::is_const::value>::type> { + using CleanT = typename std::remove_volatile::type; + using Signature_format = void (CleanT::*)(llvm::raw_ostream &S, + StringRef Options) const; template static char test2(SameType *); @@ -72,7 +76,25 @@ class has_FormatMember static double test2(...); public: - static bool const value = (sizeof(test2(nullptr)) == 1); + static bool const value = (sizeof(test2(nullptr)) == 1); +}; + +template +class has_FormatMember< + T, typename std::enable_if::value && + !std::is_const::value>::type> { + using CleanT = typename std::remove_cv::type; + using Signature_format = void (CleanT::*)(llvm::raw_ostream &S, + StringRef Options); + + template + static char test2(SameType *); + + template static double test2(...); + +public: + static bool const value = + (sizeof(test2(nullptr)) == 1) || has_FormatMember::value; }; // Test if format_provider is defined on T and contains a member function @@ -98,15 +120,18 @@ public: // based format() invocation. template struct uses_format_member - : public std::integral_constant::value> {}; + : public std::integral_constant< + bool, + has_FormatMember::type>::value> {}; // Simple template that decides whether a type T should use the format_provider // based format() invocation. The member function takes priority, so this test // will only be true if there is not ALSO a format member. template struct uses_format_provider - : public std::integral_constant::value && - has_FormatProvider::value> {}; + : public std::integral_constant< + bool, !uses_format_member::value && has_FormatProvider::value> { +}; // Simple template that decides whether a type T has neither a member-function // nor format_provider based implementation that it can use. Mostly used so @@ -114,8 +139,9 @@ struct uses_format_provider // implementation can be located. template struct uses_missing_provider - : public std::integral_constant::value && - !has_FormatProvider::value> {}; + : public std::integral_constant::value && + !uses_format_provider::value> {}; template typename std::enable_if::value, diff --git a/unittests/Support/FormatVariadicTest.cpp b/unittests/Support/FormatVariadicTest.cpp index 7dd23cfa2f8..9c87c9e53b5 100644 --- a/unittests/Support/FormatVariadicTest.cpp +++ b/unittests/Support/FormatVariadicTest.cpp @@ -13,6 +13,35 @@ using namespace llvm; +// Compile-time tests for the uses_format_member template +namespace { +struct ConstFormat { + void format(raw_ostream &OS, StringRef Opt) const { OS << "ConstFormat"; } +}; + +struct Format { + void format(raw_ostream &OS, StringRef Opt) { OS << "Format"; } +}; + +using detail::uses_format_member; + +static_assert(uses_format_member::value, ""); +static_assert(uses_format_member::value, ""); +static_assert(uses_format_member::value, ""); +static_assert(not uses_format_member::value, ""); +static_assert(not uses_format_member::value, ""); +static_assert(not uses_format_member::value, ""); +static_assert(not uses_format_member::value, ""); + +static_assert(uses_format_member::value, ""); +static_assert(uses_format_member::value, ""); +static_assert(uses_format_member::value, ""); +static_assert(uses_format_member::value, ""); +static_assert(uses_format_member::value, ""); +static_assert(uses_format_member::value, ""); +static_assert(uses_format_member::value, ""); +} + TEST(FormatVariadicTest, EmptyFormatString) { auto Replacements = formatv_object_base::parseFormatString(""); EXPECT_EQ(0U, Replacements.size()); @@ -511,7 +540,7 @@ TEST(FormatVariadicTest, Adapter) { public: explicit Negative(int N) : N(N) {} - void format(raw_ostream &S, StringRef Options) { S << -N; } + void format(raw_ostream &S, StringRef Options) const { S << -N; } }; EXPECT_EQ("-7", formatv("{0}", Negative(7)).str()); @@ -535,4 +564,27 @@ TEST(FormatVariadicTest, ImplicitConversions) { SmallString<4> S2 = formatv("{0} {1}", 1, 2); EXPECT_EQ("1 2", S2); -} \ No newline at end of file +} + +TEST(FormatVariadicTest, FormatMember) { + EXPECT_EQ("Format", formatv("{0}", Format()).str()); + + Format var; + EXPECT_EQ("Format", formatv("{0}", var).str()); + EXPECT_EQ("Format", formatv("{0}", std::move(var)).str()); + + // Not supposed to compile + // const Format cvar{}; + // EXPECT_EQ("Format", formatv("{0}", cvar).str()); +} + +TEST(FormatVariadicTest, FormatMemberConst) { + EXPECT_EQ("ConstFormat", formatv("{0}", ConstFormat()).str()); + + ConstFormat var; + EXPECT_EQ("ConstFormat", formatv("{0}", var).str()); + EXPECT_EQ("ConstFormat", formatv("{0}", std::move(var)).str()); + + const ConstFormat cvar{}; + EXPECT_EQ("ConstFormat", formatv("{0}", cvar).str()); +}