public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jonathan Wakely <jwakely@redhat.com>
To: Stephan Bergmann <sbergman@redhat.com>
Cc: "libstdc++" <libstdc++@gcc.gnu.org>,
	gcc Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [committed] libstdc++: Reduce template instantiations in <regex>
Date: Thu, 6 Jan 2022 14:59:48 +0000	[thread overview]
Message-ID: <CACb0b4n5d_Qrnf8WCqsdQ50V41S=Y2rd4L7Cxkdkh--rDS+uMA@mail.gmail.com> (raw)
In-Reply-To: <CACb0b4kTJiM8GD-dz4cxJRKa0p01Bzi=NRRFC0v0xvKe7_wbhA@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 5197 bytes --]

On Thu, 6 Jan 2022 at 10:43, Jonathan Wakely <jwakely@redhat.com> wrote:

>
>
> On Thu, 6 Jan 2022 at 10:33, Jonathan Wakely <jwakely@redhat.com> wrote:
>
>>
>>
>> On Thu, 6 Jan 2022 at 10:00, Stephan Bergmann <sbergman@redhat.com>
>> wrote:
>>
>>> On 05/01/2022 14:47, Jonathan Wakely via Libstdc++ wrote:
>>> > Tested powerpc64le-linux, pushed to trunk.
>>> >
>>> >
>>> > This moves the last two template parameters of __regex_algo_impl to be
>>> > runtime function parameters instead, so that we don't need four
>>> > different instantiations for the possible ways to call it. Most of the
>>> > function (and what it instantiates) is the same in all cases, so making
>>> > them compile-time choices doesn't really have much benefit.
>>> >
>>> > Use  'if constexpr' for conditions that check template parameters, so
>>> > that when we do depend on a compile-time condition we only instantiate
>>> > what we need to.
>>> >
>>> > libstdc++-v3/ChangeLog:
>>> >
>>> >       * include/bits/regex.h (__regex_algo_impl): Change __policy and
>>> >       __match_mode template parameters to be function parameters.
>>> >       (regex_match, regex_search): Pass policy and match mode as
>>> >       function arguments.
>>> >       * include/bits/regex.tcc (__regex_algo_impl): Change template
>>> >       parameters to function parameters.
>>> >       * include/bits/regex_compiler.h (_RegexTranslatorBase): Use
>>> >       'if constexpr' for conditions using template parameters.
>>> >       (_RegexTranslator): Likewise.
>>> >       * include/bits/regex_executor.tcc (_Executor::_M_handle_accept):
>>> >       Likewise.
>>> >       * testsuite/util/testsuite_regex.h (regex_match_debug)
>>> >       (regex_search_debug): Move template arguments to function
>>> >       arguments.
>>> > ---
>>> >   libstdc++-v3/include/bits/regex.h             | 33
>>> +++++++++----------
>>> >   libstdc++-v3/include/bits/regex.tcc           |  8 ++---
>>> >   libstdc++-v3/include/bits/regex_compiler.h    |  9 ++---
>>> >   libstdc++-v3/include/bits/regex_executor.tcc  |  2 +-
>>> >   libstdc++-v3/testsuite/util/testsuite_regex.h | 24 +++++++-------
>>> >   5 files changed, 37 insertions(+), 39 deletions(-)
>>>
>>> Clang now fails #include <regex> with
>>>
>>> > In file included from
>>> gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/12.0.0/../../../../include/c++/12.0.0/regex:66:
>>> >
>>> gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/12.0.0/../../../../include/c++/12.0.0/bits/regex.h:799:9:
>>> error: unknown type name '_RegexExecutorPolicy'; did you mean
>>> '__detail::_RegexExecutorPolicy'?
>>> >                                     _RegexExecutorPolicy, bool);
>>> >                                     ^
>>> >
>>> gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/12.0.0/../../../../include/c++/12.0.0/bits/regex.h:45:14:
>>> note: '__detail::_RegexExecutorPolicy' declared here
>>> >   enum class _RegexExecutorPolicy : int { _S_auto, _S_alternate };
>>> >              ^
>>> >
>>> gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/12.0.0/../../../../include/c++/12.0.0/bits/regex.h:2070:9:
>>> error: unknown type name '_RegexExecutorPolicy'; did you mean
>>> '__detail::_RegexExecutorPolicy'?
>>> >                                     _RegexExecutorPolicy, bool);
>>> >                                     ^
>>> >
>>> gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/12.0.0/../../../../include/c++/12.0.0/bits/regex.h:45:14:
>>> note: '__detail::_RegexExecutorPolicy' declared here
>>> >   enum class _RegexExecutorPolicy : int { _S_auto, _S_alternate };
>>> >              ^
>>>
>>> and
>>>
>>> > diff --git a/libstdc++-v3/include/bits/regex.h
>>> b/libstdc++-v3/include/bits/regex.h
>>> > index 7480b0a5f97..46c168010bf 100644
>>> > --- a/libstdc++-v3/include/bits/regex.h
>>> > +++ b/libstdc++-v3/include/bits/regex.h
>>> > @@ -796,7 +796,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>>> >         __detail::__regex_algo_impl(_Bp, _Bp, match_results<_Bp, _Ap>&,
>>> >                                     const basic_regex<_Cp, _Rp>&,
>>> >                                     regex_constants::match_flag_type,
>>> > -                                   _RegexExecutorPolicy, bool);
>>> > +                                   __detail::_RegexExecutorPolicy,
>>> bool);
>>> >
>>> >        template<typename, typename, typename, bool>
>>> >         friend class __detail::_Executor;
>>> > @@ -2067,7 +2067,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>>> >         __detail::__regex_algo_impl(_Bp, _Bp, match_results<_Bp, _Ap>&,
>>> >                                     const basic_regex<_Cp, _Rp>&,
>>> >                                     regex_constants::match_flag_type,
>>> > -                                   _RegexExecutorPolicy, bool);
>>> > +                                   __detail::_RegexExecutorPolicy,
>>> bool);
>>> >
>>> >        // Reset contents to __size unmatched sub_match objects
>>> >        // (plus additional objects for prefix, suffix and unmatched
>>> sub).
>>>
>>> would fix that.
>>>
>>
>>
>> I'll make the change, but this looks like a clang bug:
>> https://godbolt.org/z/bozxYErrc
>>
>
> Maybe this one: https://github.com/llvm/llvm-project/issues/38230
>

I've pushed the fix now, thanks for the report.

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 1536 bytes --]

commit ec12ddd1e7f7d6b48a593df865e7846039e7d62e
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Jan 6 11:11:52 2022

    libstdc++: Adjust friend declarations to work with Clang
    
    I think this code is valid but it fails with Clang, possibly due to
    https://llvm.org/PR38882
    
    Qualifying the names makes it work for all compilers.
    
    libstdc++-v3/ChangeLog:
    
            * include/bits/regex.h (basic_regex, match_results): Qualify
            name in friend declaration, to work around Clang bug.

diff --git a/libstdc++-v3/include/bits/regex.h b/libstdc++-v3/include/bits/regex.h
index 7480b0a5f97..46c168010bf 100644
--- a/libstdc++-v3/include/bits/regex.h
+++ b/libstdc++-v3/include/bits/regex.h
@@ -796,7 +796,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
 	__detail::__regex_algo_impl(_Bp, _Bp, match_results<_Bp, _Ap>&,
 				    const basic_regex<_Cp, _Rp>&,
 				    regex_constants::match_flag_type,
-				    _RegexExecutorPolicy, bool);
+				    __detail::_RegexExecutorPolicy, bool);
 
       template<typename, typename, typename, bool>
 	friend class __detail::_Executor;
@@ -2067,7 +2067,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
 	__detail::__regex_algo_impl(_Bp, _Bp, match_results<_Bp, _Ap>&,
 				    const basic_regex<_Cp, _Rp>&,
 				    regex_constants::match_flag_type,
-				    _RegexExecutorPolicy, bool);
+				    __detail::_RegexExecutorPolicy, bool);
 
       // Reset contents to __size unmatched sub_match objects
       // (plus additional objects for prefix, suffix and unmatched sub).

      reply	other threads:[~2022-01-06 15:00 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-05 13:47 Jonathan Wakely
2022-01-06 10:00 ` Stephan Bergmann
2022-01-06 10:33   ` Jonathan Wakely
2022-01-06 10:43     ` Jonathan Wakely
2022-01-06 14:59       ` Jonathan Wakely [this message]

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='CACb0b4n5d_Qrnf8WCqsdQ50V41S=Y2rd4L7Cxkdkh--rDS+uMA@mail.gmail.com' \
    --to=jwakely@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=libstdc++@gcc.gnu.org \
    --cc=sbergman@redhat.com \
    /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).