<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 Oct 4, 2016, at 4:46 PM, Zachary Turner <<a href="mailto:zturner@google.com" class="">zturner@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div style="white-space:pre-wrap" class="">I mean "how many callsites are there”?</div></div></blockquote><div><br class=""></div><div>I haven’t audited / counted, and I’m not done with all the conversions yet.</div><div><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Tue, Oct 4, 2016 at 4:23 PM Mehdi Amini <<a href="mailto:mehdi.amini@apple.com" class="">mehdi.amini@apple.com</a>> wrote:<br class=""></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 4, 2016, at 4:17 PM, Zachary Turner <<a href="mailto:zturner@google.com" class="gmail_msg" target="_blank">zturner@google.com</a>> wrote:</div><br class="gmail_msg m_-1048385252294974526Apple-interchange-newline"><div class="gmail_msg"><div dir="ltr" class="gmail_msg">How frequently would it need to be done at each callsite? </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">I don’t understand the question. Basically “always”?</div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"><br class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg"> My concern is just that it sounds like we're adding a potentially dangerous function to a very generic and widely used API for the purposes of one client.</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">Well the alternative to this API is .data(), the goal is to add a safer API than .data() :)</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">I'd inspect closely any client that is using .data() instead of this one.</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><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"> If you make the function available, people are going to use it without understanding the implications. Another possibility is that if this all takes place within a single common translation unit, you could write a private function inside there that looks like this:<div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">const char *Unsafe_cstr(StringRef S) {</div><div class="gmail_msg"> assert(strlen(S.data()) == S.size());</div><div class="gmail_msg"> return S.data();</div><div class="gmail_msg">}</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">That way the unsafe function is not exposed to general clients of StringRef?</div></div></div></blockquote><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"><div class="gmail_quote gmail_msg"><div dir="ltr" class="gmail_msg">On Tue, Oct 4, 2016 at 4:10 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">The use case is that MachineInstruction have operands, but the size of their operands is limited. Right now you have the space for one-pointer, but not for a StringRef. So if you create a MachineInstruction that takes a symbol name as an operand, we can only store the “const chat *” from the StringRef.</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">So what you’re suggesting to do is roughly what I’m trying to achieve here as well, but instead of replicating the logic at each call sites, I try to abstract it behind the API.</div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg">On Oct 4, 2016, at 3:56 PM, Zachary Turner <<a href="mailto:zturner@google.com" class="gmail_msg" target="_blank">zturner@google.com</a>> wrote:</div><br class="gmail_msg m_-1048385252294974526m_-4338203111005648878Apple-interchange-newline"><div class="gmail_msg"><div dir="ltr" class="gmail_msg">I'm not familiar with those cases, but could you do something like this:<div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">assert(strlen(s.data()) == s.size() && "Str is not null terminated!");</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">at each call site?</div></div></div></blockquote><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"><div class="gmail_quote gmail_msg"><div dir="ltr" class="gmail_msg">On Tue, Oct 4, 2016 at 3:48 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">Neat, I didn’t know this one!</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">That does not solve cases like the round-trip StringRef -> MO -> StringRef though.</div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg">On Oct 4, 2016, at 3:38 PM, Zachary Turner <<a href="mailto:zturner@google.com" class="gmail_msg" target="_blank">zturner@google.com</a>> wrote:</div><br class="m_-1048385252294974526m_-4338203111005648878m_-4260667698418455263Apple-interchange-newline gmail_msg"><div class="gmail_msg">Can you use %*s format specifier in those cases?<br class="gmail_msg"><br class="gmail_msg">printf("%*s", s.size(), s.data());<br class="gmail_msg"></div></blockquote><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div class="gmail_quote gmail_msg"><div dir="ltr" class="gmail_msg">On Tue, Oct 4, 2016 at 3:34 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">mehdi_amini added a comment.<br class="gmail_msg">
<br class="gmail_msg">
In <a href="https://reviews.llvm.org/D25255#561566" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D25255#561566</a>, @zturner wrote:<br class="gmail_msg">
<br class="gmail_msg">
> Not sure how I feel about this. It's convenient, but it has potential for abuse. Where did you run into issues porting code to `StringRef` that this solves?<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
I ran into this with "printf" like API at some point, or with places that can't be converted to StringRef because of space constraint, like MachineOperand. So you're out of the IR and you have to use .data() to initialize the MO. Later you may construct a StringRef from this "const char *" again.<br class="gmail_msg">
<br class="gmail_msg">
> I've done a lot of `StringRef` porting in LLDB by now, and I've always managed to find a solution to this. Usually it involves trickling the `StringRef` changes down further<br class="gmail_msg">
<br class="gmail_msg">
Usually that's what I do, yes. The problem is when you reach some cases like above.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<a href="https://reviews.llvm.org/D25255" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D25255</a><br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
</blockquote></div>
</div></blockquote></div><br class="gmail_msg"></div></blockquote></div>
</div></blockquote></div><br class="gmail_msg"></div></blockquote></div>
</div></blockquote></div></div></blockquote></div>
</div></blockquote></div><br class=""></body></html>