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

[Jonathan Wakely <jwakely at redhat dot com>]

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.