On 21/10/14 09:45 -0700, Tim Shen wrote:
On Tue, Oct 21, 2014 at 3:25 AM, Jonathan Wakely <jwak...@redhat.com> wrote:
Did you manage to produce a testcase that crashed on trunk?
Oh I forgot to mention that I've tried my best to make a testcase that
crash the trunk, but failed :).
I'm not sure if I should directly put an assert in the code and make a
testcase to explode it. Now I think it's better to do it.
Only if it's likely to catch problems in future. If you'd be putting
it in only to make a testcase fail then it's not worth it.
Is it really necessary to modify _M_current here?
Couldn't you do:
auto __pre = _M_current;
if (_M_is_word(*--__pre))
__left_is_word = true;
Then the function could remain const, couldn't it?
That's exactly what I did in the early version of this patch. But
later I changed because I assume that copying an iterator is
potentially expensive, but mutating is cheaper.
In general iterators are always passed by value and should be cheap to
copy. Inside regex the iterator is usually a const char* so is very
cheap to copy.
Making this function const may bring some optimization, doesn't it?
But I have no idea how much it will bring and if it's worthy.
It's unlikely (see http://www.gotw.ca/gotw/081.htm).
I just don't see the point in making it a non-const function just to
perform a micro-optimisation.
If you were passing an integer to a function would you do
f(i-1);
or
--i;
f(i);
++i;
?
The first form seems obviously better to me.
You could even simplify it further using std::prev:
if (_M_is_word(*std::prev(__pre)))
__left_is_word = true;
