Add llvm::StringLiteral

[zturner]

Summary

As discussed on the mailing list, this allows us to create global tables of StringRefs that don't incur a global constructor.

[yaron.keren]

include/llvm/ADT/StringRef.h

839 ↗

(On Diff #81138)

wrapper around StringRef

[zturner]

This comment has been deleted.

include/llvm/ADT/StringRef.h

839 ↗

(On Diff #81138)

I'm open to better wording here, but I don't think this is accurate either. A "wrapper around a StringRef" implies that you need a StringRef in order to create one of these. But it's exactly the opposite, you really do need a string literal (i.e. object of the form "abcdefg") to create one of these.

[mehdi_amini]

include/llvm/ADT/StringRef.h

90 ↗

(On Diff #81138)

Too bad we're losing the assert, but I see why. An alternative is another (possibly protected) ctor that would be constexpr and used by StringLiteral

839 ↗

(On Diff #81138)

Nit, from the coding standards: Don’t duplicate function or class name at the beginning of the comment. For humans it is obvious which function or class is being documented; automatic documentation processing tools are smart enough to bind the comment to the correct declaration.

(Also technically it is more of a wrapper around a string literal more than around a StringLiteral)

843 ↗

(On Diff #81138)

May add a warning about null in the middle of a literal? (That's a behavior change from StringRef)

[mehdi_amini]

(Otherwise LGTM)

[efriedma]

include/llvm/ADT/StringRef.h

90 ↗

(On Diff #81138)

Stupid trick to save the assert:

/*implicit*/ constexpr StringRef(const char *data, long length)
    : Data(data), Length((data || length == 0) ? length : (assert(0 && "Bad StringRef"), 0)) { }

[zturner]

include/llvm/ADT/StringRef.h

90 ↗

(On Diff #81138)

Won't this still complain that assert is not a constant expression and thus can't be used in a constexpr function?

[malcolm.parsons]

unittests/ADT/StringRefTest.cpp

1007 ↗

(On Diff #81138)

There is no test for the length of the StrlingLiteral.
Maybe EXPECT_EQ(StringRef("Foo"), Strings[0]);

[efriedma]

include/llvm/ADT/StringRef.h

90 ↗

(On Diff #81138)

No... formally, [expr.const] in the C++11 standard says that when checking whether an expression is a constant expression, "conditional operations that are not evaluated are not considered". In practice, this means you'll get an error at compile time only if the condition is false, which is exactly the behavior you want.

I actually screwed up the implementation slightly; it's not a no-op if assertions are turned off, but that's easy to fix:

/*implicit*/ constexpr StringRef(const char *data, long length)
    : Data(data), Length((data || length == 0) ? length : (assert(0 && "Bad StringRef"), length)) { }

llvm/trunk/include/llvm/ADT/StringRef.h

public:
 
     /// Construct a string ref from a cstring.
     LLVM_ATTRIBUTE_ALWAYS_INLINE
     /*implicit*/ StringRef(const char *Str)
         : Data(Str), Length(Str ? ::strlen(Str) : 0) {}
 
     /// Construct a string ref from a pointer and length.
     LLVM_ATTRIBUTE_ALWAYS_INLINE
-    /*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");
-      }
+    /*implicit*/ constexpr StringRef(const char *data, size_t length)
+        : Data(data), Length(length) {}
 
     /// Construct a string ref from an std::string.
     LLVM_ATTRIBUTE_ALWAYS_INLINE
     /*implicit*/ StringRef(const std::string &Str)
       : Data(Str.data()), Length(Str.length()) {}
 
     static StringRef withNullAsEmpty(const char *data) {
       return StringRef(data ? data : "");
public:
     LLVM_NODISCARD
     StringRef trim(StringRef Chars = " \t\n\v\f\r") const {
       return ltrim(Chars).rtrim(Chars);
     }
 
     /// @}
   };
 
+  /// A wrapper around a string literal that serves as a proxy for constructing
+  /// global tables of StringRefs with the length computed at compile time.
+  /// Using this class with a non-literal char array is considered undefined
+  /// behavior.  To prevent this, it is recommended that StringLiteral *only*
+  /// be used in a constexpr context, as such:
+  ///
+  /// constexpr StringLiteral S("test");
+  ///
+  /// Note: There is a subtle behavioral difference in the constructor of
+  /// StringRef and StringLiteral, as illustrated below:
+  ///
+  /// constexpr StringLiteral S("a\0b");  // S.size() == 3
+  /// StringRef S("a\0b");  // S.size() == 1
+  ///
+  class StringLiteral : public StringRef {
+  public:
+    template <size_t N>
+    constexpr StringLiteral(const char (&Str)[N]) : StringRef(Str, N - 1) {}
+  };
+
   /// @name StringRef Comparison Operators
   /// @{
 
   LLVM_ATTRIBUTE_ALWAYS_INLINE
   inline bool operator==(StringRef LHS, StringRef RHS) {
     return LHS.equals(RHS);
   }
 

llvm/trunk/unittests/ADT/StringRefTest.cpp

TEST(StringRefTest, DropWhileUntil) {
   Taken = Test.drop_until([](char c) { return true; });
   EXPECT_EQ(Test, Taken);
 
   StringRef EmptyString = "";
   Taken = EmptyString.drop_while([](char c) { return true; });
   EXPECT_EQ("", Taken);
 }
 
+TEST(StringRefTest, StringLiteral) {
+  constexpr StringLiteral Strings[] = {"Foo", "Bar"};
+  EXPECT_EQ(StringRef("Foo"), Strings[0]);
+  EXPECT_EQ(StringRef("Bar"), Strings[1]);
+}
+
 } // end anonymous namespace