Re: [Patch] Small refactor on <regex> _State<>

[Tim Shen <timshen at google dot com>]

On Sat, Jul 25, 2015 at 8:31 AM, Jonathan Wakely <jwakely@redhat.com> wrote:
> On 25/07/15 00:11 -0700, Tim Shen wrote:
>>
>> It's not a very necessary refactoring, but simply can't resist. :)
>>
>> I'm not sure of the ::memcpy calls. It looks not very idiomatic, but
>> std::copy on char* looks even more weird? :/
>
>
> The ::memcpy(this, &rhs, sizeof(rhs))...) makes me quite
> uncomfortable, because _State is not trivially-copyable, although its
> _State_base base class _is_ trivially-copyable, and is at the same
> address and has the same size ... so I think it's safe.
>
> But couldn't you replace that memcpy with an assignment?
>
>    _State_base::operator=(__rhs);

Done.

> The implicitly defined assignment operator should do the same thing as
> a memcpy.
>
> _State should have a deleted copy assignment operator though (or a
> user-provided one that correctly handles the _S_opcode_match case, but
> since it's not needed it should just be deleted).

Actually it's needed in _StateSeq::_M_clone, but I defined a normal
member function _State::_M_clone to avoid unexpected copying.


-- 
Regards,
Tim Shen

Attachment: b.diff
Description: Text document