<div dir="ltr">Incidentally, it appears the version of regcomp() on apple supports <span style="font-family:monaco,courier,monospace;font-size:12px;letter-spacing:-0.23px;color:rgb(102,102,102)"><a href="https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man3/regcomp.3.html">REG_PEND</a>, </span>although that is not a standard. And apple is already the only platform where enhanced regexes are a thing. So if you want to keep enhanced regexes, one thing we could do is:<div><br></div><div>a) Have all non Apple platforms delegate to the LLVM implementation.</div><div>b) Have Apple's implementation use the system library but set the end pointer.</div><div><br></div><div>That said, if you don't care about enhanced mode, let's just have everyone use LLVM's.</div></div><br><div class="gmail_quote"><div dir="ltr">On Wed, Sep 21, 2016 at 5:03 PM Zachary Turner <<a href="mailto:zturner@google.com">zturner@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="gmail_msg">It supports extended, just not *enhanced*. Enhanced appears to be an apple specific thing, and even the developer documentation recommends not using it in portable code.</div><br class="gmail_msg"><div class="gmail_quote gmail_msg"><div dir="ltr" class="gmail_msg">On Wed, Sep 21, 2016 at 5:00 PM Jim Ingham <<a href="mailto:jingham@apple.com" class="gmail_msg" target="_blank">jingham@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">IIRC you said the llvm one doesn't support extended regular expressions. If that's true, please don't do this till that is fixed. I think pretty much everybody who is using reg expressions expects to be able to use extended regular expressions.<br class="gmail_msg">
<br class="gmail_msg">
Jim<br class="gmail_msg">
<br class="gmail_msg">
> On Sep 21, 2016, at 4:58 PM, Zachary Turner via lldb-commits <<a href="mailto:lldb-commits@lists.llvm.org" class="gmail_msg" target="_blank">lldb-commits@lists.llvm.org</a>> wrote:<br class="gmail_msg">
><br class="gmail_msg">
> Incidentally even the STL regex implementation supports specifying the end pointer. So I would call the system one deficient in this regard and advocate for replacing it sooner rather than later.<br class="gmail_msg">
><br class="gmail_msg">
> On Wed, Sep 21, 2016 at 4:52 PM Zachary Turner <<a href="mailto:zturner@google.com" class="gmail_msg" target="_blank">zturner@google.com</a>> wrote:<br class="gmail_msg">
> Looks like llvm's regex is better than LLDB's in this regard, since it supports explicitly setting the end pointer. I can see a couple options:<br class="gmail_msg">
><br class="gmail_msg">
> 1) Check if it's null terminated by peeking one past the end, and copying if it's not. This is pretty hackish, not crazy about this idea.<br class="gmail_msg">
> 2) Un-delete the const char * version of the function but leave the StringRef overload, find all places where I added the explicit conversion and remove them so they invoke the const char* overload.<br class="gmail_msg">
> 3) Change lldb::RegularExpression to just delegate to llvm under the hood and set the end pointer.<br class="gmail_msg">
><br class="gmail_msg">
> On Wed, Sep 21, 2016 at 4:44 PM Zachary Turner <<a href="mailto:zturner@google.com" class="gmail_msg" target="_blank">zturner@google.com</a>> wrote:<br class="gmail_msg">
> Actually wait, it doesn't. It just explicitly sets the end pointer.<br class="gmail_msg">
><br class="gmail_msg">
> On Wed, Sep 21, 2016 at 4:44 PM Zachary Turner <<a href="mailto:zturner@google.com" class="gmail_msg" target="_blank">zturner@google.com</a>> wrote:<br class="gmail_msg">
> Worth noting that llvm::Regex has this constructor:<br class="gmail_msg">
><br class="gmail_msg">
><br class="gmail_msg">
> Regex::Regex(StringRef regex, unsigned Flags) {<br class="gmail_msg">
> unsigned flags = 0;<br class="gmail_msg">
> preg = new llvm_regex();<br class="gmail_msg">
> preg->re_endp = regex.end();<br class="gmail_msg">
> if (Flags & IgnoreCase)<br class="gmail_msg">
> flags |= REG_ICASE;<br class="gmail_msg">
> if (Flags & Newline)<br class="gmail_msg">
> flags |= REG_NEWLINE;<br class="gmail_msg">
> if (!(Flags & BasicRegex))<br class="gmail_msg">
> flags |= REG_EXTENDED;<br class="gmail_msg">
> error = llvm_regcomp(preg, regex.data(), flags|REG_PEND);<br class="gmail_msg">
> }<br class="gmail_msg">
><br class="gmail_msg">
> So it assumes null termination even though you have a StringRef.<br class="gmail_msg">
><br class="gmail_msg">
> On Wed, Sep 21, 2016 at 4:43 PM Zachary Turner <<a href="mailto:zturner@google.com" class="gmail_msg" target="_blank">zturner@google.com</a>> wrote:<br class="gmail_msg">
> You need to duplicate something on the heap once when you execute the regex. And in turn you save tens or hundreds or copies on the way there because of inefficient string usage.<br class="gmail_msg">
><br class="gmail_msg">
> We could also just un-delete the overload that takes a const char*, then the duplication would only ever happen when you explicitly use a StringRef.<br class="gmail_msg">
><br class="gmail_msg">
> I don't agree this should be reverted. In the process of doing this conversion I eliminated numerous careless string copies.<br class="gmail_msg">
><br class="gmail_msg">
> On Wed, Sep 21, 2016 at 4:38 PM Greg Clayton <<a href="mailto:gclayton@apple.com" class="gmail_msg" target="_blank">gclayton@apple.com</a>> wrote:<br class="gmail_msg">
> This it the perfect example of why not to use a StringRef since the string needs to be null terminated. Why did we change this? Now even if you call this function:<br class="gmail_msg">
><br class="gmail_msg">
> RegularExpression r(...);<br class="gmail_msg">
><br class="gmail_msg">
> r.Execute(".......................", ...)<br class="gmail_msg">
><br class="gmail_msg">
> You will need to duplicate the string on the heap just to execute this. Please revert this. Anything that requires null terminate is not a candidate for converting to StringRef.<br class="gmail_msg">
><br class="gmail_msg">
><br class="gmail_msg">
> > On Sep 21, 2016, at 10:13 AM, Zachary Turner via lldb-commits <<a href="mailto:lldb-commits@lists.llvm.org" class="gmail_msg" target="_blank">lldb-commits@lists.llvm.org</a>> wrote:<br class="gmail_msg">
> ><br class="gmail_msg">
> > Author: zturner<br class="gmail_msg">
> > Date: Wed Sep 21 12:13:51 2016<br class="gmail_msg">
> > New Revision: 282090<br class="gmail_msg">
> ><br class="gmail_msg">
> > URL: <a href="http://llvm.org/viewvc/llvm-project?rev=282090&view=rev" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project?rev=282090&view=rev</a><br class="gmail_msg">
> > Log:<br class="gmail_msg">
> > Fix failing regex tests.<br class="gmail_msg">
> ><br class="gmail_msg">
> > r282079 converted the regular expression interface to accept<br class="gmail_msg">
> > and return StringRefs instead of char pointers. In one case<br class="gmail_msg">
> > a null pointer check was converted to an empty string check,<br class="gmail_msg">
> > but this was an incorrect conversion because an empty string<br class="gmail_msg">
> > is a valid regular expression. Removing this check should<br class="gmail_msg">
> > fix the test failures.<br class="gmail_msg">
> ><br class="gmail_msg">
> > Modified:<br class="gmail_msg">
> > lldb/trunk/source/Core/RegularExpression.cpp<br class="gmail_msg">
> ><br class="gmail_msg">
> > Modified: lldb/trunk/source/Core/RegularExpression.cpp<br class="gmail_msg">
> > URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/RegularExpression.cpp?rev=282090&r1=282089&r2=282090&view=diff" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/RegularExpression.cpp?rev=282090&r1=282089&r2=282090&view=diff</a><br class="gmail_msg">
> > ==============================================================================<br class="gmail_msg">
> > --- lldb/trunk/source/Core/RegularExpression.cpp (original)<br class="gmail_msg">
> > +++ lldb/trunk/source/Core/RegularExpression.cpp Wed Sep 21 12:13:51 2016<br class="gmail_msg">
> > @@ -102,7 +102,7 @@ bool RegularExpression::Compile(llvm::St<br class="gmail_msg">
> > //---------------------------------------------------------------------<br class="gmail_msg">
> > bool RegularExpression::Execute(llvm::StringRef str, Match *match) const {<br class="gmail_msg">
> > int err = 1;<br class="gmail_msg">
> > - if (!str.empty() && m_comp_err == 0) {<br class="gmail_msg">
> > + if (m_comp_err == 0) {<br class="gmail_msg">
> > // Argument to regexec must be null-terminated.<br class="gmail_msg">
> > std::string reg_str = str;<br class="gmail_msg">
> > if (match) {<br class="gmail_msg">
> ><br class="gmail_msg">
> ><br class="gmail_msg">
> > _______________________________________________<br class="gmail_msg">
> > lldb-commits mailing list<br class="gmail_msg">
> > <a href="mailto:lldb-commits@lists.llvm.org" class="gmail_msg" target="_blank">lldb-commits@lists.llvm.org</a><br class="gmail_msg">
> > <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits</a><br class="gmail_msg">
><br class="gmail_msg">
> _______________________________________________<br class="gmail_msg">
> lldb-commits mailing list<br class="gmail_msg">
> <a href="mailto:lldb-commits@lists.llvm.org" class="gmail_msg" target="_blank">lldb-commits@lists.llvm.org</a><br class="gmail_msg">
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits</a><br class="gmail_msg">
<br class="gmail_msg">
</blockquote></div></blockquote></div>