[Bug libstdc++/109751] New: boost interface_interface fails concept check starting in gcc-13

[Bug libstdc++/109751] New: boost interface_interface fails concept check starting in gcc-13

[ldalessandro at gmail dot com]

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

            Bug ID: 109751
           Summary: boost interface_interface fails concept check starting
                    in gcc-13
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ldalessandro at gmail dot com
  Target Milestone: ---

The following example is reduced from
https://github.com/boostorg/stl_interfaces/blob/boost-1.82.0/example/node_iterator.cpp.
It compiles fine in gcc-12 but fails a concept check in gcc-13.


  #include <boost/stl_interfaces/iterator_interface.hpp>

  template<typename T>
  struct iterator : boost::stl_interfaces::iterator_interface<iterator<T>, 
  std::forward_iterator_tag, T>
  {
    using base_type = boost::stl_interfaces::iterator_interface<iterator<T>,
std::forward_iterator_tag, T>;
    using base_type::operator++;

    T& operator*() const;
    iterator& operator++();
    bool operator==(iterator) const;
  };

  static_assert(std::incrementable<iterator<int>>);

Live: https://godbolt.org/z/6qsx1narj

The error tells us that this requires clause changes from false to true.

      friend constexpr bool operator<(D lhs, D rhs)
        requires std::equality_comparable<D> {
          return (lhs - rhs) < typename D::difference_type(0);
        }

https://github.com/boostorg/stl_interfaces/blob/boost-1.82.0/include/boost/stl_interfaces/iterator_interface.hpp#L779

I don't think that requires clause makes any sense, but I also don't know why
the behavior would have changed from gcc-12 to gcc-13.

[Bug c++/109751] boost iterator_interface fails concept check starting in gcc-13

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|libstdc++                   |c++

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Can you provide the preprocessed source?

[Bug c++/109751] boost iterator_interface fails concept check starting in gcc-13

[ldalessandro at gmail dot com]

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

--- Comment #2 from Luke Dalessandro <ldalessandro at gmail dot com> ---
Created attachment 55009
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55009&action=edit
Preprocessed source copied from the godbolt live link included in the original
text.

[Bug c++/109751] boost iterator_interface fails concept check starting in gcc-13

[ldalessandro at gmail dot com]

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

--- Comment #3 from Luke Dalessandro <ldalessandro at gmail dot com> ---
One thing to note about this failure, the concept evaluation changes in gcc-13.
The when I try with current trunk it actually ICEs in addition.

I reported that ICE as https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109752.

[Bug c++/109751] boost iterator_interface fails concept check starting in gcc-13

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Luke Dalessandro from comment #3)
> One thing to note about this failure, the concept evaluation changes in
> gcc-13. The when I try with current trunk it actually ICEs in addition.

The ICE on the trunk is because extra checking is enabled by default, it most
likely also ICEs in GCC 13 also (maybe even with -fchecking=2).

[Bug c++/109751] boost iterator_interface fails concept check starting in gcc-13

[ldalessandro at gmail dot com]

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

--- Comment #5 from Luke Dalessandro <ldalessandro at gmail dot com> ---
Ah, I didn't realize that. If you want to close one of the two issues that's
fine with me. I sort of thought two different things were going on.

[Bug c++/109751] boost iterator_interface fails concept check starting in gcc-13

[pinskia at gcc dot gnu.org]

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

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Luke Dalessandro from comment #5)
> Ah, I didn't realize that. If you want to close one of the two issues that's
> fine with me. I sort of thought two different things were going on.

The ICE might not be related so keeping both open until it is figured out is
good.

[Bug c++/109751] boost iterator_interface fails concept check starting in gcc-13

[pinskia at gcc dot gnu.org]

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

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I think the error message is correct.

In the original code we have:
```
...
  template<typename _Iter>
    concept incrementable = regular<_Iter> && requires(_Iter __i) { { __i++ }
-> same_as<_Iter>; };
...

namespace boost { namespace stl_interfaces { inline namespace v2 {
    template<      typename D,      typename IteratorConcept,     typename
DifferenceType>
    struct iterator_interface
    {
      friend constexpr bool operator>=(D lhs, D rhs)
        requires std::equality_comparable<D> {        }
    };
...

template<typename T>
struct iterator : boost::stl_interfaces::iterator_interface<iterator<T>,
std::forward_iterator_tag, T>
{
    bool operator==(iterator) const;
};

```
iterator<T> is not complete until after the instantiation is finished but we
have a std::equality_comparable which does the confusion

[Bug c++/109751] boost iterator_interface fails concept check starting in gcc-13

[ldalessandro at gmail dot com]

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

--- Comment #8 from Luke Dalessandro <ldalessandro at gmail dot com> ---
(In reply to Andrew Pinski from comment #7)
> I think the error message is correct.
> 
> In the original code we have:
> ```
> ...
>   template<typename _Iter>
>     concept incrementable = regular<_Iter> && requires(_Iter __i) { { __i++
> } -> same_as<_Iter>; };
> ...
> 
> namespace boost { namespace stl_interfaces { inline namespace v2 {
>     template<      typename D,      typename IteratorConcept,     typename
> DifferenceType>
>     struct iterator_interface
>     {
>       friend constexpr bool operator>=(D lhs, D rhs)
>         requires std::equality_comparable<D> {        }
>     };
> ...
> 
> template<typename T>
> struct iterator : boost::stl_interfaces::iterator_interface<iterator<T>,
> std::forward_iterator_tag, T>
> {
>     bool operator==(iterator) const;
> };
> 
> ```
> iterator<T> is not complete until after the instantiation is finished but we
> have a std::equality_comparable which does the confusion

Right.

I was sort of surprised that that boost code could get away with a requires on
any of those CRTP functions where it uses the CRTP "D" type, but I tested
gcc-12 and all the clangs, and nvc++, and msvc 19, and gcc-13 was the only one
reporting problems here, so I went with consensus.

[Bug c++/109751] boost iterator_interface fails concept check starting in gcc-13

[pinskia at gcc dot gnu.org]

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

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Luke Dalessandro from comment #8)
> 
> Right.
> 
> I was sort of surprised that that boost code could get away with a requires
> on any of those CRTP functions where it uses the CRTP "D" type, but I tested
> gcc-12 and all the clangs, and nvc++, and msvc 19, and gcc-13 was the only
> one reporting problems here, so I went with consensus.

Note I could be wrong too. Which is why I kept it open until someone else who
is more familar with the C++ standard makes a judgement.

[Bug c++/109751] boost iterator_interface fails concept check starting in gcc-13

[pinskia at gcc dot gnu.org]

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

--- Comment #10 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note is_constructible_v changes answers between the two places I think.

[Bug c++/109751] boost iterator_interface fails concept check starting in gcc-13

[pinskia at gcc dot gnu.org]

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

--- Comment #11 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Reduced down to:
```
template<typename _Tp> concept cmpeq
  = requires(_Tp __t, _Tp __u) { { __u != __t } ; };
template<typename D>
struct iterator_interface
{
  friend constexpr bool operator>=(D lhs, D rhs) requires cmpeq<D> { return
true; }
};
template<typename T>
struct iterator : iterator_interface<iterator<T>>
{
    bool operator==(iterator) const;
    iterator &operator++();
    iterator &operator++(int);
};
static_assert(cmpeq<iterator<int>>);
```
Which does make it look invalid for the reasons why I mentioned. clang compiles
the above just fine and so does GCC 12.x.

[Bug c++/109751] boost iterator_interface fails concept check starting in gcc-13

[pinskia at gcc dot gnu.org]

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

--- Comment #12 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #11)

> Which does make it look invalid for the reasons why I mentioned. clang
> compiles the above just fine and so does GCC 12.x.

I should note the ICE does not happen with this fully reduced testcase, I put a
less reduced testcase in the bug report for the ICE.

[Bug c++/109751] boost iterator_interface fails concept check starting in gcc-13

[redi at gcc dot gnu.org]

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

--- Comment #13 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Yes, I think GCC 13 is correct to reject it (but not correct to ICE!)

Either way, I don't see how it can be a libstdc++ bug, the concept is defined
the same way, it's the compiler that diagnoses the bug. So I agree with
Andrew's change to component=c++ as well.

[Bug c++/109751] boost iterator_interface fails concept check starting in gcc-13

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jason at gcc dot gnu.org

--- Comment #14 from Jonathan Wakely <redi at gcc dot gnu.org> ---
This started to be rejected with r13-4035

c++: only declare satisfied friends

A friend declaration can only have constraints if it is defined.  If
multiple instantiations of a class template define the same friend function
signature, it's an error, but that shouldn't happen if it's constrained to
only be declared in one instantiation.

Currently we don't mangle requirements, so the foos all mangle the same and
actually instantiating #1 will break, but for now we can test that they're
considered distinct.

gcc/cp/ChangeLog:

* pt.cc (tsubst_friend_function): Check satisfaction.

[Bug c++/109751] boost iterator_interface fails concept check starting in gcc-13

[ldalessandro at gmail dot com]

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

--- Comment #15 from Luke Dalessandro <ldalessandro at gmail dot com> ---
Thanks for looking at this. I'd like to report it back to boost as an issue,
but I want to make sure I understand what to tell them.

1. The error produce by Andrew's reduction ("error: satisfaction of atomic
constraint ... depends on itself") is different than the one produced in the
original code ("error: satisfaction value of atomic constraint ...  changed)
from 'false' to 'true'"). I'm to understand that this isn't important here,
it's basically reacting to the same thing?

2. The fundamental problem with the boost code is the use of a constrained
`friend` template that depends on the template parameter D, that is not yet
complete when it is evaluated?

3. A fix for this issue in boost and the iterator_interface is to use a
constrained member function rather than attempting to use a constrained friend
function?


Also, from what I understand, I should report Andrew's reduced case as a bug in
clang, and msvc (and maybe nvc++ with its different front-end), which I will
do.

Thanks for looking into this.

[Bug c++/109751] boost iterator_interface fails concept check starting in gcc-13

[ldalessandro at gmail dot com]

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

--- Comment #16 from Luke Dalessandro <ldalessandro at gmail dot com> ---
(In reply to Luke Dalessandro from comment #15)
> Thanks for looking at this. 
> [...]
>
> Also, from what I understand, I should report Andrew's reduced case as a bug
> in clang, and msvc (and maybe nvc++ with its different front-end), which I
> will do.

I realized that this is almost certainly IFNDR so there isn't any need for me
to create these reports.

[Bug c++/109751] boost iterator_interface fails concept check starting in gcc-13

[redi at gcc dot gnu.org]

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

--- Comment #17 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Yep, [temp.constr.atomic] p3:

"If, at different points in the program, the satisfaction result is different
for identical atomic constraints and template arguments, the program is
ill-formed, no diagnostic required."

[Bug c++/109751] boost iterator_interface fails concept check starting in gcc-13

[ppalka at gcc dot gnu.org]

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

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=108303
                 CC|                            |ppalka at gcc dot gnu.org

--- Comment #18 from Patrick Palka <ppalka at gcc dot gnu.org> ---
A similar issue with constrained hidden friends + CRTP + CWG2596 was reported
previously as PR108303

[Bug c++/109751] [13/14 Regression] boost iterator_interface fails concept check starting in gcc-13

[ppalka at gcc dot gnu.org]

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

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|ice-on-invalid-code         |rejects-valid
   Target Milestone|---                         |13.2
            Summary|boost iterator_interface    |[13/14 Regression] boost
                   |fails concept check         |iterator_interface fails
                   |starting in gcc-13          |concept check starting in
                   |                            |gcc-13

--- Comment #19 from Patrick Palka <ppalka at gcc dot gnu.org> ---
We may want to consider reverting / adjusting the r13-4035 change for GCC 13.2,
since CWG2596 hasn't been formally adopted (and so strictly speaking this is a
rejects-valid bug IIUC).

[Bug c++/109751] [13/14 Regression] boost iterator_interface fails concept check starting in gcc-13

[ppalka at gcc dot gnu.org]

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

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|https://gcc.gnu.org/bugzill |
                   |a/show_bug.cgi?id=108303    |
                 CC|                            |ted at lyncon dot se

--- Comment #20 from Patrick Palka <ppalka at gcc dot gnu.org> ---
*** Bug 108303 has been marked as a duplicate of this bug. ***

[Bug c++/109751] [13/14 Regression] boost iterator_interface fails concept check starting in gcc-13

[ppalka at gcc dot gnu.org]

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

--- Comment #21 from Patrick Palka <ppalka at gcc dot gnu.org> ---
(In reply to Luke Dalessandro from comment #15)
> 3. A fix for this issue in boost and the iterator_interface is to use a
> constrained member function rather than attempting to use a constrained
> friend function?

IIUC yes, or another less invasive workaround would be to convert the
constrained hidden friend into a template, e.g.

template<typename _Tp> concept cmpeq
  = requires(_Tp __t, _Tp __u) { { __u != __t } ; };
template<typename D>
struct iterator_interface
{
  template<class...>
  friend constexpr bool operator>=(D lhs, D rhs) requires cmpeq<D> { return
true; }
};
template<typename T>
struct iterator : iterator_interface<iterator<T>>
{
    bool operator==(iterator) const;
    iterator &operator++();
    iterator &operator++(int);
};
static_assert(cmpeq<iterator<int>>);

[Bug c++/109751] [13/14 Regression] boost iterator_interface fails concept check starting in gcc-13

[ppalka at gcc dot gnu.org]

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

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |h2+bugs at fsfe dot org

--- Comment #22 from Patrick Palka <ppalka at gcc dot gnu.org> ---
*** Bug 110765 has been marked as a duplicate of this bug. ***

[Bug c++/109751] [13/14 Regression] boost iterator_interface fails concept check starting in gcc-13

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|13.2                        |13.3

--- Comment #23 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 13.2 is being released, retargeting bugs to GCC 13.3.

[Bug c++/109751] [13/14 Regression] boost iterator_interface fails concept check starting in gcc-13

[jason at gcc dot gnu.org]

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

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |jason at gcc dot gnu.org
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2023-08-17
     Ever confirmed|0                           |1

[Bug c++/109751] [13/14 Regression] boost iterator_interface fails concept check starting in gcc-13

[cvs-commit at gcc dot gnu.org]

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

--- Comment #24 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Jason Merrill <jason@gcc.gnu.org>:

https://gcc.gnu.org/g:810bcc00156cefce7ad40fc9d8de6e43c3a04450

commit r14-3389-g810bcc00156cefce7ad40fc9d8de6e43c3a04450
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Aug 17 11:36:23 2023 -0400

    c++: constrained hidden friends [PR109751]

    r13-4035 avoided a problem with overloading of constrained hidden friends
by
    checking satisfaction, but checking satisfaction early is inconsistent with
    the usual late checking and can lead to hard errors, so let's not do that
    after all.

    We were wrongly treating the different instantiations of the same friend
    template as the same function because maybe_substitute_reqs_for was failing
    to actually substitute in the case of a non-template friend.  But we don't
    actually need to do the substitution anyway, because [temp.friend] says
that
    such a friend can't be the same as any other declaration.

    After fixing that, instead of a redefinition error we got an ambiguous
    overload error, fixed by allowing constrained hidden friends to coexist
    until overload resolution, at which point they probably won't be in the
same
    ADL overload set anyway.

    And we avoid mangling collisions by following the proposed mangling for
    these friends as a member function with an extra 'F' before the name.  I
    demangle this by just adding [friend] to the name of the function because
    it's not feasible to reconstruct the actual scope of the function since the
    mangling ABI doesn't distinguish between class and namespace scopes.

            PR c++/109751

    gcc/cp/ChangeLog:

            * cp-tree.h (member_like_constrained_friend_p): Declare.
            * decl.cc (member_like_constrained_friend_p): New.
            (function_requirements_equivalent_p): Check it.
            (duplicate_decls): Check it.
            (grokfndecl): Check friend template constraints.
            * mangle.cc (decl_mangling_context): Check it.
            (write_unqualified_name): Check it.
            * pt.cc (uses_outer_template_parms_in_constraints): Fix for
friends.
            (tsubst_friend_function): Don't check satisfaction.

    include/ChangeLog:

            * demangle.h (enum demangle_component_type): Add
            DEMANGLE_COMPONENT_FRIEND.

    libiberty/ChangeLog:

            * cp-demangle.c (d_make_comp): Handle DEMANGLE_COMPONENT_FRIEND.
            (d_count_templates_scopes): Likewise.
            (d_print_comp_inner): Likewise.
            (d_unqualified_name): Handle member-like friend mangling.
            * testsuite/demangle-expected: Add test.

    gcc/testsuite/ChangeLog:

            * g++.dg/cpp2a/concepts-friend11.C: Now works.  Add template.
            * g++.dg/cpp2a/concepts-friend15.C: New test.

[Bug c++/109751] [13/14 Regression] boost iterator_interface fails concept check starting in gcc-13

[cvs-commit at gcc dot gnu.org]

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

--- Comment #25 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Jason Merrill
<jason@gcc.gnu.org>:

https://gcc.gnu.org/g:6396e37d451b2ef6d3cb9c11edbc06deeaccd808

commit r13-7771-g6396e37d451b2ef6d3cb9c11edbc06deeaccd808
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Aug 17 11:36:23 2023 -0400

    Revert "c++: only declare satisfied friends" [PR109751]

    The earlier patch was only an incremental step toward making this sort of
    code work, and broke code that had been working.  So let's revert it.

    This reverts commit r13-4035-gc41bbfcaf9d6ef.

            PR c++/109751

    gcc/cp/ChangeLog:

            * pt.cc (tsubst_friend_function): Don't check constraints.

    gcc/testsuite/ChangeLog:

            * g++.dg/cpp2a/concepts-friend11.C: Xfail.
            * g++.dg/cpp2a/concepts-friend15.C: New test.

[Bug c++/109751] [13/14 Regression] boost iterator_interface fails concept check starting in gcc-13

[jason at gcc dot gnu.org]

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

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #26 from Jason Merrill <jason at gcc dot gnu.org> ---
Fixed for 13.3/14.

[Bug c++/109751] [13/14 Regression] boost iterator_interface fails concept check starting in gcc-13

[ppalka at gcc dot gnu.org]

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

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |janezz55 at gmail dot com

--- Comment #27 from Patrick Palka <ppalka at gcc dot gnu.org> ---
*** Bug 111831 has been marked as a duplicate of this bug. ***