[Bug c++/105351] New: [concepts] Constraint checking does correctly match static member attributes

[Bug c++/105351] New: [concepts] Constraint checking does correctly match static member attributes

[gawain.bolton at free dot fr]

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

            Bug ID: 105351
           Summary: [concepts] Constraint checking does correctly match
                    static member attributes
           Product: gcc
           Version: 11.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: gawain.bolton at free dot fr
  Target Milestone: ---

Created attachment 52852
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52852&action=edit
Concept requirements violated when attr2 is not static

The attached code is shows how a concept requiring a static attribute matches a
structure which has the attribute with a non-static data member.

The code also shows that the checking of static vs. non-static functions works
as expected.

Tested with https://godbolt.org/ using versions: x86-64 gcc v11.2 and trunk
Compiler options: -std=c++20  -Wall -Wextra -O2

[Bug c++/105351] [concepts] Constraint checking does correctly match static member attributes

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Are you sure it should be == 2?
Even clang is != 2.

The expressions inside requires is not a normal expression and all.

[Bug c++/105351] [concepts] Constraint checking does correctly match static member attributes

[gawain.bolton at free dot fr]

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

--- Comment #2 from Gawain Bolton <gawain.bolton at free dot fr> ---
Yes I believe this is a bug and strangely enough clang also seems to have this
issue.

>From the draft C++20 standard concerning "simple requirements" (cf.
https://isocpp.org/files/papers/N4860.pdf page 109):

A simple-requirement asserts the validity of an expression. [Note: The
enclosing requires-expression will evaluate to false if substitution of
template arguments into the expression fails. The expression is an unevaluated
operand (7.2). — end note] [Example:
template<typename T> concept C =
requires (T a, T b) {
a + b; // C<T> is true if a + b is a valid expression
};
— end example]

Clearly T::attr2 is not a valid expression for struct T { int attr2; }

[Bug c++/105351] [concepts] Constraint checking does correctly match static member attributes

[ensadc at mailnesia dot com]

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

ensadc at mailnesia dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ensadc at mailnesia dot com

--- Comment #3 from ensadc at mailnesia dot com ---
I believe that `T::attr2` is a valid expression when it appears in a
requirement. So I think GCC and Clang are correct.

[expr.prim.id.general]/3:

An id-expression that denotes a non-static data member or non-static member
function of a class can only be used: 

- [...]
- if that id-expression denotes a non-static data member and it appears in an
unevaluated operand. 

[expr.prim.req.general]/2:

Expressions appearing within a requirement-body are unevaluated operands.

[Bug c++/105351] [concepts] Constraint checking does correctly match static member attributes

[gawain.bolton at free dot fr]

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

--- Comment #4 from Gawain Bolton <gawain.bolton at free dot fr> ---
I am confused as to how the requirement expression be valid and yet not be
evaluated.

This is also not consistent with how the requirement check is done for
functions.

Finally, this also begs the question as to how one could write a requirement to
ensure a class has a static attribute.

[Bug c++/105351] [concepts] Constraint checking does correctly match static member attributes

[ppalka at gcc dot gnu.org]

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

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

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

--- Comment #5 from Patrick Palka <ppalka at gcc dot gnu.org> ---
(In reply to Gawain Bolton from comment #4)
> Finally, this also begs the question as to how one could write a requirement
> to ensure a class has a static attribute.

One way is to replace the requirement 'T::attr2' with 'typename
dummy<&T::attr2>' where 'dummy' is the class template

  template<auto*> struct dummy;

This works because template arguments are constant expressions, and constant
expressions are evaluated even in unevaluated contexts, so the bullet point
that ensadc pointed out in comment #3 no longer applies.

This makes both Clang and GCC happy: https://godbolt.org/z/c8q355hfh

[Bug c++/105351] [concepts] Constraint checking does correctly match static member attributes

[ppalka at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-05-03
           Assignee|unassigned at gcc dot gnu.org      |ppalka at gcc dot gnu.org

--- Comment #6 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Er, the real reason the above works is because the type auto* of dummy's
template parameter demands that the argument &T::attr2 is an ordinary pointer,
not a pointer to member, which implies that it's a static member.

We should also be able to check that attr2 is a static member via

template<auto> struct dummy;

template<typename T>
concept C = requires(T v) {
    v.attr1;
    typename dummy<T::attr2>;
    v.fun1();
    T::fun2();
};

which Clang is happy with, but GCC incorrectly emits an error during SFINAE:

<source>: In substitution of 'template<class auto:1>  requires  C<auto:1> auto
f(auto:1) [with auto:1 = Z]':
<source>:48:5:   required from here
<source>:9:14: error: invalid use of non-static data member 'Z::attr2'
    9 |     typename dummy<T::attr2>;
      |     ~~~~~~~~~^~~~~~~~~~~~~~~~
<source>:39:15: note: declared here
   39 |     const int attr2 = 0;
      |               ^~~~~

This is definitely a bug.

[Bug c++/105351] [concepts] Constraint checking does correctly match static member attributes

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:a16fc9f1c96c7361d0b7a83a06f3649ace6c440d

commit r13-95-ga16fc9f1c96c7361d0b7a83a06f3649ace6c440d
Author: Patrick Palka <ppalka@redhat.com>
Date:   Tue May 3 15:21:26 2022 -0400

    c++: make finish_non_static_data_member SFINAE enabled [PR105351]

    Here since finish_non_static_data_member isn't SFINAE enabled, we
    incorrectly emit an error when considering the first overload rather
    than silently discarding it:

    sfinae33.C: In substitution of âtemplate<class T> A<T::value> f() [with T
= B]â:
    sfinae33.C:11:7:   required from here
    sfinae33.C:5:31: error: invalid use of non-static data member
âB::valueâ
        5 | template<class T> A<T::value> f();
          |                               ^

    This patch makes the function SFINAE enabled in the usual way: give it a
    complain parameter, check it before emitting an error, and pass it through
    appropriately.

            PR c++/105351

    gcc/cp/ChangeLog:

            * cp-tree.h (finish_non_static_data_member): Add defaulted
            complain parameter.
            * pt.cc (tsubst_copy_and_build): Pass complain to
            finish_non_static_data_member.
            * semantics.cc (finish_non_static_data_member): Respect complain
            parameter.
            (finish_qualified_id_expr): Pass complain to
            finish_non_static_data_member.

    gcc/testsuite/ChangeLog:

            * g++.dg/template/sfinae33.C: New test.

[Bug c++/105351] [concepts] Constraint checking does correctly match static member attributes

[ppalka at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.0
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #8 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Fixed for GCC 13, probably not suitable for backporting.  Thanks for the bug
report.