<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Oct 17, 2016 at 2:25 PM Mehdi Amini <<a href="mailto:mehdi.amini@apple.com">mehdi.amini@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg">On Oct 17, 2016, at 1:51 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="gmail_msg" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br class="m_5479295637845851072Apple-interchange-newline gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg"><br class="gmail_msg"><br class="gmail_msg"><div class="gmail_quote gmail_msg"><div dir="ltr" class="gmail_msg">On Mon, Oct 17, 2016 at 12:38 PM Mehdi Amini <<a href="mailto:mehdi.amini@apple.com" class="gmail_msg" target="_blank">mehdi.amini@apple.com</a>> wrote:<br class="gmail_msg"></div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg">On Oct 17, 2016, at 10:12 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="gmail_msg" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br class="gmail_msg m_5479295637845851072m_2465380976656664214Apple-interchange-newline"><div class="gmail_msg"><div dir="ltr" class="gmail_msg">(for my money the readability loss of adding StringRef(...) all over the place doesn't seem quite worth it, but I'm not going to stand in the way of the change or anything.)<br class="gmail_msg"></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg">The balance is between the readability, and the “error prone” current behavior.</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">For example we introduced all these redundant .c_str() in the past: <a href="https://reviews.llvm.org/D25667" class="gmail_msg" target="_blank">https://reviews.llvm.org/D25667</a></div><div class="gmail_msg">Ideally we shouldn’t have much explicit calls to StringRef, if all the APIs are converted to work with StringRef most of the time. </div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">But for now we do have a fair few (judging by the size of this change).</div></div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg">Sure, at least it will be easy to spot which APIs / code can benefit from an upgrade.</div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"> </div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg"><br class="gmail_msg">Are these changes necessarily joined - or is the explicitness of the StringRef(...) ctor independent of the addition of the template?<br class="gmail_msg"></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg">It is not independent: the template is never selected if the other constructor is not made explicit.</div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg">Ah, because the const char* is a close enough match that the template isn't chosen.<br class="gmail_msg"><br class="gmail_msg">Can workaround that with something like:<br class="gmail_msg"><br class="gmail_msg"> template<typename T></div><div class="gmail_msg">  StringRef(T *Str, typename std::enable_if<std::is_same<T, const char>::value>::type = 0) = delete;</div></div></div></div></blockquote><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"><br class="gmail_msg">(now it's a template, so it's not a better match than the other one - but wouldn't trigger other implicit conversions - could generalize it to T Str, is_convertible_to<T, const char*>, etc)</div></div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg">I’m not sure what you’re looking to achieve here?</div><div class="gmail_msg">Assuming your template above replaces the one I made explicit, you’re forbidding creating a StringRef from a const char *?</div></div></div></div></blockquote><div><br></div><div>The template replaces the explicit const char* ctor with the intention of providing essentially the same behavior (implicit construction) while still deferring the array cases to the array ctor template.<br><br>By making the const char* ctor a template, it competes on more even ground against the template case. So you don't have to make the const char* one const to get the template benefit.<br><br>Alternatively - if it's just in particular global array initializers built by tblgen, we could skip all of this and provide a factory function we only use in those cases (that is constexpr, etc). Then there's no ctor issue. (we could mark the const char*+length ctor constexpr so it can be used by the factory).</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div></div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg"><div class="gmail_quote gmail_msg"><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><br class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg">Do we need to worry about the recursive strlen in the template ctor? Or is it clear that even in a non-constexpr context the compilers we care about produce reasonable performance?</div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg">Can you clarify what you mean by "non-constexpr context” here?</div><div class="gmail_msg">The recursive template is not intended to be used with anything else than literal, I am not expecting this to generate any code ever (if it does, it is not intended).</div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg">Constexpr functions don't guarantee that they will be evaluated at compile time. They only provide that they /can/ be used in a place where compile time evaluation is required (& taht they will conform to the constexpr requirements (not writing mutating state, throwing exceptions, etc) in whatever places they're used that require constexpr (eg: a constexpr function can throw under some condition, so long as all the uses of the function in a constexpr context don't satisfy that condition))<br class="gmail_msg"></div></div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg">Right, I was expecting that calling this function when instantiating using a `constexpr StringRef` constructor would be enough to trigger it, but not in all cases apparently.</div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><br class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"><br class="gmail_msg">For example, look at the IR generated from this code:<br class="gmail_msg"><br class="gmail_msg"><div class="gmail_msg">struct StringRef {</div><div class="gmail_msg">  constexpr static unsigned getInitLength(char const *str, unsigned count = 0) {</div><div class="gmail_msg">    return ('\0' == str[0]) ? count : getInitLength(str + 1, count + 1);</div><div class="gmail_msg">  }</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">  template <int N></div><div class="gmail_msg">  constexpr StringRef(const char (&Str)[N]) </div><div class="gmail_msg">    : Length(getInitLength(Str)) {</div><div class="gmail_msg">  }</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">  int Length;</div><div class="gmail_msg">};</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">int main() {</div><div class="gmail_msg">  StringRef S = "foo";</div><div class="gmail_msg">}<br class="gmail_msg"><br class="gmail_msg"><div class="gmail_msg">$ grep getInitLength str.ll</div><div class="gmail_msg">$_ZN9StringRef13getInitLengthEPKcj = comdat any</div><div class="gmail_msg">  %call = call i32 @_ZN9StringRef13getInitLengthEPKcj(i8* %arraydecay, i32 0)</div><div class="gmail_msg">define linkonce_odr i32 @_ZN9StringRef13getInitLengthEPKcj(i8* %str, i32 %count) #1 comdat align 2 {</div><div class="gmail_msg">  %call = call i32 @_ZN9StringRef13getInitLengthEPKcj(i8* %add.ptr, i32 %add)<br class="gmail_msg"><br class="gmail_msg">the getInitLength function exists and is recursive (the only way to force this not to happen is if the S variable itself is declared constexpr, then there's no mention of getInitLength in the generated IR).<br class="gmail_msg"><br class="gmail_msg">Granted, if it's always a compile time constant string and everything is inlinable, etc, the optimizer will get rid of it - but LLVM's -O0 build might suffer greatly with all this recursive string length evaluation.<br class="gmail_msg"></div></div></div></div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg">Right, we can use __builtin_strlen() instead of the recursive getInitLength. I didn’t go this route because I don’t know how to address it with MSVC  (and was expecting it be folded by the frontend…).</div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg">& of course any code like this wouldn't ever be optimized away & would still hit the recursive version (which might be optimized to a loop - /maybe/ even to strlen?):<br class="gmail_msg"><br class="gmail_msg">char x[N];<br class="gmail_msg">x[0] = 'a' + (rand() % 26); // or otherwise compute string contents at runtime<br class="gmail_msg">StringRef S = x;<br class="gmail_msg"><br class="gmail_msg">(essentially what Zach was asking about/pointing out)<br class="gmail_msg"><br class="gmail_msg">Does all of that make sense? I may've done a poor job explaining it.</div></div></div></div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg">You did a good job here :)</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">— </div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg">Mehdi</div><div class="gmail_msg"><br class="gmail_msg"></div></div></div></blockquote></div></div>