[PATCH] D29898: [Support] Add format_provider for StringLiteral

Pavel Labath via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 14 03:40:32 PST 2017


labath updated this revision to Diff 88342.
labath added a comment.

Using is_convertible will work right now. What will this prevent is someone
defining their own format_provider on a class which has a conversion operator
for StringRef.  I've debated this with myself a bit. On one hand, it adds more
predictability (if something behaves enough like StringRef that in can be
converted to one, it should also be formatted like it). On the other hand, I'm
sure somebody somewhere will invent a case where it still makes sense to do
that.

But I suppose we can cross that bridge when we come to it.


https://reviews.llvm.org/D29898

Files:
  include/llvm/Support/FormatProviders.h
  unittests/Support/FormatVariadicTest.cpp


Index: unittests/Support/FormatVariadicTest.cpp
===================================================================
--- unittests/Support/FormatVariadicTest.cpp
+++ unittests/Support/FormatVariadicTest.cpp
@@ -324,11 +324,13 @@
   const char FooArray[] = "FooArray";
   const char *FooPtr = "FooPtr";
   llvm::StringRef FooRef("FooRef");
+  constexpr StringLiteral FooLiteral("FooLiteral");
   std::string FooString("FooString");
   // 1. Test that we can print various types of strings.
   EXPECT_EQ(FooArray, formatv("{0}", FooArray).str());
   EXPECT_EQ(FooPtr, formatv("{0}", FooPtr).str());
   EXPECT_EQ(FooRef, formatv("{0}", FooRef).str());
+  EXPECT_EQ(FooLiteral, formatv("{0}", FooLiteral).str());
   EXPECT_EQ(FooString, formatv("{0}", FooString).str());
 
   // 2. Test that the precision specifier prints the correct number of
Index: include/llvm/Support/FormatProviders.h
===================================================================
--- include/llvm/Support/FormatProviders.h
+++ include/llvm/Support/FormatProviders.h
@@ -45,9 +45,8 @@
 
 template <typename T>
 struct use_string_formatter
-    : public std::integral_constant<
-          bool, is_one_of<T, llvm::StringRef, std::string>::value ||
-                    is_cstring<T>::value> {};
+    : public std::integral_constant<bool,
+                                    std::is_convertible<T, llvm::StringRef>::value> {};
 
 template <typename T>
 struct use_pointer_formatter
@@ -205,7 +204,7 @@
     if (!Style.empty() && Style.getAsInteger(10, N)) {
       assert(false && "Style is not a valid integer");
     }
-    llvm::StringRef S(V);
+    llvm::StringRef S = V;
     Stream << S.substr(0, N);
   }
 };


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D29898.88342.patch
Type: text/x-patch
Size: 1688 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170214/0291c66e/attachment.bin>


More information about the llvm-commits mailing list