std::regex: inserting std::wregex to std::vector loses some std::wregex values

[Jonathan Wakely]

On 14/09/14 11:12 -0700, Tim Shen wrote:
>On Sun, Sep 14, 2014 at 3:16 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
>> Thus, are you going to send an actual patch? I'm wondering if we could
>> arrange it in such a way not to break the binary compatibility in the
>> release branch (thus fix the bug there too) perhaps by not removing any data
>> members, simply leaving them unused?! In mainline I suppose we are going to
>> stabilize the ABI for the 5.0 release, thus we can do that. While we are at
>> it, check that the order of the final remaining data members is still
>> optimal size-wise.
>
>I've made a patch that changes basic_regex's member only, by making
>basic_regex::_M_traits heap allocated.

With this patch basic_regex::_M_traits becames a pointer, but that
pointer becomes null after a move - is that safe?

It looks wrong to allocate a traits object in the default constructor
but not in the move constructor, and to allow _M_traits to be null
following assign(), e.g. does this work OK after your patch?

  std::regex r;
  std::regex rr = std::move(r);
  r.assign("a");
  regex_match("a", r);

Would it make sense to store the traits object inside the NFA, instead
of having it separate but tightly coupled to the NFA by references?

>I believe that I didn't get it, but I'm still a little bit confused,
>since we haven't instantiated and exported any regex class (except
>regex_error, I suppose) to the binary, right?
>
>Anyway, the patch is bootstrapped and tested with debug flag. Can it
>be modified slightly to keep the binary compatibility?

I think for the 4.9 branch we could fix the copy constructor so it
doesn't cause sharing of the NFA and then change the move constructor
to also perform a copy, like so:

diff --git a/libstdc++-v3/include/bits/regex.h b/libstdc++-v3/include/bits/regex.h
index 9dc83fd..5a9dd78 100644
--- a/libstdc++-v3/include/bits/regex.h
+++ b/libstdc++-v3/include/bits/regex.h
@@ -474,16 +474,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        *
        * @param __rhs A @p regex object.
        */
-      basic_regex(const basic_regex& __rhs) = default;
+      basic_regex(const basic_regex& __rhs)
+      : basic_regex(__rhs._M_original_str, __rhs._M_flags)
+      { }
 
       /**
        * @brief Move-constructs a basic regular expression.
        *
        * @param __rhs A @p regex object.
        */
-      basic_regex(const basic_regex&& __rhs) noexcept
-      : _M_flags(__rhs._M_flags), _M_traits(__rhs._M_traits),
-       _M_automaton(std::move(__rhs._M_automaton))
+      basic_regex(basic_regex&& __rhs)
+      : basic_regex(__rhs)
       { }
 
       /**

This will be slower, and not noexcept, but avoids the dangling
references.