llvm dev - Nov 2016 - RFC: Constructing StringRefs at compile time

On 29 November 2016 at 16:18, Zachary Turner <zturner at google.com>
wrote:
> I don't like the llvm_strlen approach as it is incompatible with
> std::string_view which we may eventually move to.
In what way is it incompatible?

constexpr StringRef(const char* s) : Data(s), Length(llvm_strlen(s)) {}
is equivalent to
constexpr string_view(const char* s) : Data(s),
Length(std::char_traits<char>::length(s)) {}

-- 
Malcolm Parsons

On Tue, Nov 29, 2016 at 8:32 AM Malcolm Parsons <malcolm.parsons at
gmail.com>
wrote:
> On 29 November 2016 at 16:18, Zachary Turner <zturner at google.com>
wrote:
> > I don't like the llvm_strlen approach as it is incompatible with
> > std::string_view which we may eventually move to.
>
> In what way is it incompatible?
>
> constexpr StringRef(const char* s) : Data(s), Length(llvm_strlen(s)) {}
> is equivalent to
> constexpr string_view(const char* s) : Data(s),
> Length(std::char_traits<char>::length(s)) {}
>
I see, but I looked over your proposed implementation from earlier in the
thread, and if I'm not mistaken I see this:

     /// Construct a string ref from a cstring.
     LLVM_ATTRIBUTE_ALWAYS_INLINE
+#if __has_builtin(__builtin_strlen)
+    /*implicit*/ constexpr StringRef(const char *Str)
+        : Data(Str), Length(Str ? __builtin_strlen(Str) : 0) {}
+#else
     /*implicit*/ StringRef(const char *Str)
         : Data(Str), Length(Str ? ::strlen(Str) : 0) {}
+#endif

So we've got a constexpr constructor if __builtin_strlen exists, and a
non-constexpr constructor if it doesn't exist.  This seems like a recipe
for disaster for me.  You can't ever construct one in a constexpr context,
because not all compilers will have a constexpr constructor, and if you
can't construct one in a constexpr context, then the __builtin_strlen()
wouldn't work as expected either, right?

I like making it explicit via a StringLiteral class because then everyone's
code looks the same:

constexpr StringLiteral Strings[] = {"a", "b",
"c"};

That said, what did you think about my other proposal of the complicated
UDL with macro?

#define LIT(x) x_string_ref_literal
constexpr StringRef Strings[] = {LIT("a"), LIT("b"),
LIT("c")};
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20161129/e1224e54/attachment.html>

On 29 November 2016 at 17:38, Zachary Turner <zturner at google.com>
wrote:
> I see, but I looked over your proposed implementation from earlier in the
> thread, and if I'm not mistaken I see this:
That's a different suggestion.
> That said, what did you think about my other proposal of the complicated
UDL
> with macro?
>
> #define LIT(x) x_string_ref_literal
> constexpr StringRef Strings[] = {LIT("a"), LIT("b"),
LIT("c")};
Why bother with the UDL?
#define LIT(x) StringRef((x), sizeof(x)-1)

-- 
Malcolm Parsons