<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Sep 25, 2016, at 1:49 PM, Mehdi Amini via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" class=""><div class=""><br class="Apple-interchange-newline">On Sep 25, 2016, at 9:10 AM, Zachary Turner via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">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:<div class=""><br class=""></div><div class="">LLDB has a lot of functions that look like this:</div><div class=""><br class=""></div><div class="">void foo(const char *, Bar, const char *).</div><div class=""><br class=""></div><div class="">I'm trying to port these to functions that look like this:</div><div class=""><br class=""></div><div class="">void foo(StringRef, Bar, StringRef).</div><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">To work around this, I've started doing the following every time I port a function:</div><div class=""><br class=""></div><div class="">void foo(const char *, Bar, const char*) = delete;</div><div class=""><br class=""></div><div class="">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:</div><div class=""><br class=""></div><div class="">foo("bar", baz, "buzz")</div><div class=""><br class=""></div><div class="">I have to write</div><div class=""><br class=""></div><div class="">foo(StringRef("bar"), baz, StringRef("buzz"))<br class=""></div><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">2) [nullptr, nullptr+0) is a valid range. Why shouldn't we be able to construct a StringRef from an otherwise perfectly valid range?</div><div class=""><br class=""></div><div class="">3) StringRef() can<span class="Apple-converted-space"> </span><b class="">already</b> 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?</div><div class=""><br class=""></div><div class=""><br class=""></div><div class="">Thoughts?</div></div></div></blockquote><br class=""></div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class=""></div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">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. </div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">I rather have this constructor explicit, and provide an implicit one for string literal.</div></div></blockquote>I wonder if we could change that call site to be deleted (or at least explicit), and add support for literal strings with a StringRef version of this:</div><div><br class=""></div><div> /// Construct an ArrayRef from a C array.<br class=""> template <size_t N><br class=""> /*implicit*/ LLVM_CONSTEXPR ArrayRef(const T (&Arr)[N])<br class=""> : Data(Arr), Length(N) {}</div><div><br class=""></div><div>This way we’ll avoid the strlen on quoted strings which is the common case anyway, and then can see how many other cases we have from const char* remaining.</div><div><br class=""></div><div>Pete<br class=""><blockquote type="cite" class=""><div class=""><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class=""></div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">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.</div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class=""></div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">— </div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">Mehdi</div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class=""></div><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">_______________________________________________</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">LLVM Developers mailing list</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><a href="mailto:llvm-dev@lists.llvm.org" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">llvm-dev@lists.llvm.org</a><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a></div></blockquote></div><br class=""></body></html>