[llvm-dev] Is it time to allow StringRef to be constructed from nullptr?

Zachary Turner via llvm-dev llvm-dev at lists.llvm.org
Sun Sep 25 13:57:43 PDT 2016


For now, leaving it returning null seems the least risk, since in theory
someone could be relying on that (that would be horrible, of course, but
I've seen worse).

On Sun, Sep 25, 2016 at 1:49 PM Mehdi Amini <mehdi.amini at apple.com> wrote:

>
> On Sep 25, 2016, at 9:10 AM, Zachary Turner via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
> While porting LLDB over to StringRef, I am continuously running into
> difficulties caused by the fact that StringRef cannot be constructed from
> nullptr.  So I wanted to see peoples' thoughts on removing this restriction
> from StringRef.  To be clear, I'm only using LLDB as a motivating example,
> but I'm not requesting that it be done because LLDB is some kind of special
> case.  If it is to be done it should be on its own merits.  That said, here
> is some context:
>
> LLDB has a lot of functions that look like this:
>
> void foo(const char *, Bar, const char *).
>
> I'm trying to port these to functions that look like this:
>
> void foo(StringRef, Bar, StringRef).
>
> Often times the parameters are string literals or char arrays, but equally
> often they are another const char* that got passed into the calling
> function, or a return value from a CRT function like strstr(), or many
> other possible sources.  This latter category presents a problem for
> porting code to StringRef, because if I simply change the function
> signature and fix up compile errors, I will probably have introduced a bug
> because hundreds of callers will now be implicitly converting from const
> char* to StringRef, leaving open the possibility that one of those was null.
>
> To work around this, I've started doing the following every time I port a
> function:
>
> void foo(const char *, Bar, const char*) = delete;
>
> This is pretty hackish, but it gets the job done.  At least the compiler
> warns me and forces me to go inspect every callsite where there's an
> implicit conversion.  Unfortunately it also makes for extremely verbose
> code.  Now instead of:
>
> foo("bar", baz, "buzz")
>
> I have to write
>
> foo(StringRef("bar"), baz, StringRef("buzz"))
>
> even for string literals and char arrays, which will obviously never be
> null!  If StringRef would handle a null argument gracefully, it would make
> my life much easier.
>
>
> With that out of the way, here are some reasons I can see to allow
> StringRef accept null to its constructor which are independent of LLDB and
> stand on their own merit.
>
> 1) std::string_view<> can be constructed with null.  I don't know when we
> will be able to use std::string_view<>, but there's a chance that at some
> point in the future we may wish to remove StringRef in favor of
> string_view.  That day isn't soon, but in any case, it will be easier if
> our assumptions are the same.
>
> 2) [nullptr, nullptr+0) is a valid range.  Why shouldn't we be able to
> construct a StringRef from an otherwise perfectly valid range?
>
> 3) StringRef() can *already* be constructed from nullptr (!)  Surprised?
> That's what happens when you invoke the default constructor.  It happily
> initializes the internal Data with null.  So why not allow the same
> behavior when invoking the const char * constructor?
>
>
> Thoughts?
>
>
>
> As a tangent: I don’t like the fact that StringRef is implicitly built out
> of “const char *”, this is calling strlen() and because it is implicit
> folks don’t realize when they go from string -> char * -> StringRef.
> I rather have this constructor explicit, and provide an implicit one for
> string literal.
>
> To come back to your point, I’m not sure if we should leave the internal
> pointer null or always set it to “”? This would provide the guarantee that
> dereferencing a StringRef is always valid without checking.
>
>> Mehdi
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160925/2db251d5/attachment.html>


More information about the llvm-dev mailing list