public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "redi at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug libstdc++/107376] regex executor requires allocator to be default constructible
Date: Tue, 25 Oct 2022 10:18:45 +0000	[thread overview]
Message-ID: <bug-107376-4-03WFdGUQJl@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-107376-4@http.gcc.gnu.org/bugzilla/>

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107376

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Thanks for the report. The patch is necessary, but not sufficient.

We should probably have used the same allocator for _M_rep_count but doing that
now would be an ABI break.

We should definitely have used the same allocator for _M_states, and for its
_ResultsVec members, which would also be an ABI break, but we can mitigate that
with:

--- a/libstdc++-v3/include/bits/regex_executor.tcc
+++ b/libstdc++-v3/include/bits/regex_executor.tcc
@@ -124,9 +124,10 @@ namespace __detail
            break;
          std::fill_n(_M_states._M_visited_states, _M_nfa.size(), false);
          auto __old_queue = std::move(_M_states._M_match_queue);
+         auto __alloc = _M_cur_results.get_allocator();
          for (auto& __task : __old_queue)
            {
-             _M_cur_results = std::move(__task.second);
+             _M_cur_results = _ResultsVec(std::move(__task.second), __alloc);
              _M_dfs(__match_mode, __task.first);
            }
          if (__match_mode == _Match_mode::_Prefix)

With this change and the original patch suggestion, _M_cur_results always has
the same allocator as _M_results and so _Executor::_M_handle_accept won't
replace the allocator in _M_results when assigning the new results to it.

We could also use the same allocator for the local vector here:

    bool _Executor<_BiIter, _Alloc, _TraitsT, __dfs_mode>::
    _M_lookahead(_StateIdT __next)
    {
      // Backreferences may refer to captured content.
      // We may want to make this faster by not copying,
      // but let's not be clever prematurely.
      _ResultsVec __what(_M_cur_results);

Or we can rely on select_on_container_copy_construction() to do "the right
thing". Users of non-propagating allocators would probably prefer that, but it
means that matching with std::pmr::match_results will always use new/delete for
allocating intermediate results. But that's just one of the costs of the PMR
design.

  parent reply	other threads:[~2022-10-25 10:18 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-24 12:46 [Bug libstdc++/107376] New: " listcrawler at gmail dot com
2022-10-24 23:33 ` [Bug libstdc++/107376] " redi at gcc dot gnu.org
2022-10-25 10:18 ` redi at gcc dot gnu.org [this message]
2022-10-28 14:27 ` cvs-commit at gcc dot gnu.org
2022-10-28 14:48 ` redi at gcc dot gnu.org
2024-03-18 14:06 ` cvs-commit at gcc dot gnu.org
2024-03-18 14:15 ` redi at gcc dot gnu.org

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bug-107376-4-03WFdGUQJl@http.gcc.gnu.org/bugzilla/ \
    --to=gcc-bugzilla@gcc.gnu.org \
    --cc=gcc-bugs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).