<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Feb 19, 2014 at 9:17 AM, Marshall Clow <span dir="ltr"><<a href="mailto:mclow.lists@gmail.com" target="_blank">mclow.lists@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div class="">On Feb 14, 2014, at 4:31 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
</div><div><div class=""><blockquote type="cite"><div class="gmail_extra"><div class="gmail_quote">On Fri, Feb 14, 2014 at 4:21 PM, Marshall Clow <span dir="ltr"><<a href="mailto:mclow.lists@gmail.com" target="_blank">mclow.lists@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Users can write<br>        for(sregex_iterator i(s.begin(), s.end(), regex("meow")), end; i != end; ++i)<br>
<br>binding a temporary regex to const regex& and storing a pointer to it.<br>This will compile silently, triggering undefined behavior at runtime.<br><br>Fixing this involves defining constructors for the various regex iterator types from rvalue regexes, and then marking them as “deleted”.<br>
And tests.<br></blockquote></div></div></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"><div dir="ltr">Would this break (what I assume is) correct code doing things like:<br><br>std::find(sregex_iterator(s.begin(), s.end(), regex("meow")), end, "foo”);  ?</div>
</blockquote><div><br></div></div><div>(Recapping a discussion on IRC)</div><div><br></div>Yes, it would.</div><div>And though that code is "technically correct” it (a) is dangerous, and (b) doesn’t do anything useful.</div>
<div><br></div><div>The return result from std::find is an interator into a destroyed object, so you can’t really do anything with that.</div><div><br></div><div>The “fix” for this code is simple:</div><div><br></div><div>
<span style="white-space:pre-wrap">     </span>regex re{“meow”}</div><div><div><div dir="ltr"><span style="white-space:pre-wrap">   </span>std::find(sregex_iterator(s.begin(), s.end(), re), end, "foo”); </div><div dir="ltr">
<br></div><div dir="ltr">and now the return value of std::find is valid (until the end of the lifetime of “re”).</div></div></div></div></blockquote><div><br></div><div>Fair point about find, though it's not too hard to find examples that take iterators and don't return temporaries from them.<br>
<br>There are lots of APIs that take references to things and return references with the same lifetime as the parameter and would break in similar situations, but perhaps these particular APIs are substantially more prone to this kind of problem and worthy of a specific safety guard.<br>
</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"><div dir="ltr">See <br></div><div dir="ltr"><span style="white-space:pre-wrap">     </span><a href="http://cplusplus.github.io/LWG/lwg-active.html#2329" target="_blank">http://cplusplus.github.io/LWG/lwg-active.html#2329</a></div>
<div dir="ltr">and<span style="white-space:pre-wrap">   </span><a href="http://cplusplus.github.io/LWG/lwg-active.html#2332" target="_blank">http://cplusplus.github.io/LWG/lwg-active.html#2332</a></div><div><div dir="ltr"><br>
</div></div><div dir="ltr">for the changes to the (C++14) standard.</div></div></blockquote><div><br>In any case, this was entirely a misunderstanding on my part - not realizing (though you'd mentioned in the subject) that this was a new, deliberately non-strictly-backwards-compatible change mandated by the next standard. I'd mistakenly thought this was an attempt to add some standard-allowable-but-not-mandated safety so I was confused why it would break existing correct callers. I'm still a little surprised at this being worth a non-backward-compatible change in the standard, but that's up to you and the rest of the committee and I didn't mean to get in the way of whatever decision came out of that.<br>
<br>A few minor, optional, comments on the patch itself:<br><br>* An extra explicit scope in each of the test cases seems strange, but perhaps there's a reason for it given how consistently you've done that.<br><br>
* The "= delete" ctors you've added aren't preprocessor protected (just "// C++14") - why's that? Won't this break C++11? Or was this change accepted as a DR to '11?<br><br>* The version tests of "> 11" and "<= 11" seem a bit harder to read (to me) than ">= 14" and "< 14" which speaks to the actual version the feature was introduced in rather than having to mentally compute that from the prior version.</div>
</div></div></div>