Re: [Patch, libstdc++/64584, libstdc++/64585] Clear basic_regex after imbue and make assign exception tolerant

[Jonathan Wakely]

@@ -516,14 +509,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
      template<typename _FwdIter>
        basic_regex(_FwdIter __first, _FwdIter __last,
                    flag_type __f = ECMAScript)
-       : _M_flags(__f),
-         _M_loc(),
-         _M_original_str(__first, __last),
-         _M_automaton(__detail::__compile_nfa<_Rx_traits>(
-           _M_original_str.c_str(),
-           _M_original_str.c_str() + _M_original_str.size(),
-           _M_loc,
-           _M_flags))
+       : basic_regex(std::move(__first), std::move(__last), locale_type(), __f)

Am I missing something here, why are you moving the iterators?

Iterators are cheap to copy, so moving them is not necessary.

@@ -764,7 +745,15 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
#endif

    private:
-      typedef std::shared_ptr<__detail::_NFA<_Rx_traits>> _AutomatonPtr;
+      typedef std::shared_ptr<const __detail::_NFA<_Rx_traits>> _AutomatonPtr;
+
+      template<typename _FwdIter>
+       basic_regex(_FwdIter __first, _FwdIter __last, locale_type __loc,
+                   flag_type __f)
+       : _M_flags(__f), _M_loc(std::move(__loc)),
+       _M_automaton(__detail::__compile_nfa<_FwdIter, _Rx_traits>(
+         std::move(__first), std::move(__last), _M_loc, _M_flags))

Again, I don't think std::move is needed for the iterators.

diff --git a/libstdc++-v3/include/bits/regex_compiler.h 
b/libstdc++-v3/include/bits/regex_compiler.h
index 81f8c8e..56962a9 100644
--- a/libstdc++-v3/include/bits/regex_compiler.h
+++ b/libstdc++-v3/include/bits/regex_compiler.h
@@ -59,9 +59,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
      _Compiler(_IterT __b, _IterT __e,
                const typename _TraitsT::locale_type& __traits, _FlagT __flags);

-      std::shared_ptr<_RegexT>
+      shared_ptr<const _RegexT>
      _M_get_nfa()
-      { return std::move(_M_nfa); }
+      { return shared_ptr<const _RegexT>(_M_nfa.release()); }

This could be:

     { return shared_ptr<const _RegexT>(std::move(_M_nfa)); }

or simply left as it was:

     { return std::move(_M_nfa); }

because shared_ptr has an implicit conversion from unique_ptr rvalues.

@@ -138,20 +138,65 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION

      _FlagT              _M_flags;
      _ScannerT           _M_scanner;
-      shared_ptr<_RegexT> _M_nfa;
+      unique_ptr<_RegexT> _M_nfa;
      _StringT            _M_value;
      _StackT             _M_stack;
      const _TraitsT&     _M_traits;
      const _CtypeT&      _M_ctype;
    };
diff --git a/libstdc++-v3/include/bits/regex_compiler.tcc 
b/libstdc++-v3/include/bits/regex_compiler.tcc
index 33d7118..20b3172 100644
--- a/libstdc++-v3/include/bits/regex_compiler.tcc
+++ b/libstdc++-v3/include/bits/regex_compiler.tcc
@@ -73,7 +73,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
               ? __flags
               : __flags | regex_constants::ECMAScript),
      _M_scanner(__b, __e, _M_flags, __loc),
-      _M_nfa(make_shared<_RegexT>(__loc, _M_flags)),
+      _M_nfa(new _RegexT(__loc, _M_flags)),

Am I right in thinking that this unique_ptr will always end up being
converted to a shared_ptr?

In that case, why bother using a unique_ptr initially, if it will just
have to allocate a shared_ptr control block later anyway? It's better
to use make_shared to reduce the number of allocations, isn't it?