[PATCH] D26332: Add a user-defined literal for StringRef

[Mehdi AMINI via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: include/llvm/ADT/StringRef.h:74
     /// Construct an empty string ref.
-    /*implicit*/ StringRef() : Data(nullptr), Length(0) {}
+    /*implicit*/ constexpr StringRef() : Data(nullptr), Length(0) {}
 
----------------
Is this related to this patch?


================
Comment at: include/llvm/ADT/StringRef.h:88
     /*implicit*/ StringRef(const char *data, size_t length)
-      : Data(data), Length(length) {
-        assert((data || length == 0) &&
-        "StringRef cannot be built from a NULL argument with non-null length");
-      }
+        : Data(data), Length(data ? length : 0) {}
 
----------------
same.


================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1221
     MCSymbol *AddrSymbol =
-        OutContext.getOrCreateSymbol(StringRef("__morestack_addr"));
+        OutContext.getOrCreateSymbol("__morestack_addr"_sr);
     OutStreamer->EmitLabel(AddrSymbol);
----------------
idlecode wrote:
> `getOrCreateSymbol` takes Twine as argument so `StringRef` conversion is not needed.
> There is few other such use-cases but it would be quite tedious to fix them all manually - maybe clang-tidy could take care of it in the future.
Are you sure that using a `Twine` with a `const char *` is equivalent? I wonder if there won't be an extra call to strlen at the point where the `Twine` is serialized.


================
Comment at: lib/CodeGen/AsmPrinter/DIEHash.cpp:41
 
-  return StringRef("");
+  return ""_sr;
 }
----------------
Why not just `return "";`?


================
Comment at: lib/IR/Value.cpp:197
   if (!hasName())
-    return StringRef("", 0);
+    return ""_sr;
   return getValueName()->getKey();
----------------
same.


================
Comment at: lib/MC/MCAsmStreamer.cpp:350
     return;
-  if (c.startswith(StringRef("//"))) {
+  if (c.startswith("//"_sr)) {
     ExplicitCommentToEmit.append("\t");
----------------
Same in this context.


https://reviews.llvm.org/D26332