diff mbox series

c++: 'A op B' constraint-expression is unchecked [PR125490]

Message ID 20260602185753.1801445-1-ppalka@redhat.com
State New
Headers
Series c++: 'A op B' constraint-expression is unchecked [PR125490] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-aarch64-bootstrap success Build passed
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap success Build passed

Commit Message

Patrick Palka June 2, 2026, 6:57 p.m. UTC 
  Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
OK for trunk/16?

-- >8 --

Here the nonsensical constraint-expression f() == 42 isn't getting
rejected ahead of time ultimately because we pass no_toplevel_fold_p=true
to cp_parser_binary_expression which makes it use build_min instead of
build_x_binary_op to avoid undesirable folding/canonicalization for sake
of OpenMP for loop parsing (r145014).  But this certainly doesn't happen
during templated parsing, and constraint-expressions are always
templated, so we might as well pass no_toplevel_fold_p=false here.

	PR c++/125490

gcc/cp/ChangeLog:

	* parser.cc (cp_parser_constraint_expression): Pass
	no_toplevel_fold_p=false instead of =true to
	cp_parser_binary_expression.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/concepts-pr125490.C: New test.
---
 gcc/cp/parser.cc                               |  2 +-
 gcc/testsuite/g++.dg/cpp2a/concepts-pr125490.C | 10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr125490.C

Comments

Jason Merrill June 2, 2026, 7:41 p.m. UTC | #1 
On 6/2/26 2:57 PM, Patrick Palka wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
> OK for trunk/16?
> 
> -- >8 --
> 
> Here the nonsensical constraint-expression f() == 42 isn't getting
> rejected ahead of time ultimately because we pass no_toplevel_fold_p=true
> to cp_parser_binary_expression which makes it use build_min instead of
> build_x_binary_op to avoid undesirable folding/canonicalization for sake
> of OpenMP for loop parsing (r145014).  But this certainly doesn't happen
> during templated parsing, and constraint-expressions are always
> templated, so we might as well pass no_toplevel_fold_p=false here.

In this testcase the concept is never checked for satisfaction.  In the 
testcase in the PR there is a similarly nonsensical concept that is 
used, and is diagnosed at that point.  So this patch is about diagnosing 
more IFNDR code, yes?

With that clarification to the commit message, OK for trunk but not 16.

> 	PR c++/125490
> 
> gcc/cp/ChangeLog:
>  that is used
> 	* parser.cc (cp_parser_constraint_expression): Pass
> 	no_toplevel_fold_p=false instead of =true to
> 	cp_parser_binary_expression.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/concepts-pr125490.C: New test.
> ---
>   gcc/cp/parser.cc                               |  2 +-
>   gcc/testsuite/g++.dg/cpp2a/concepts-pr125490.C | 10 ++++++++++
>   2 files changed, 11 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr125490.C
> 
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index b5c8c62118cc..c1797191b833 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -34761,7 +34761,7 @@ cp_parser_constraint_expression (cp_parser *parser)
>   {
>     processing_constraint_expression_sentinel parsing_constraint;
>     ++processing_template_decl;
> -  cp_expr expr = cp_parser_binary_expression (parser, false, true,
> +  cp_expr expr = cp_parser_binary_expression (parser, false, false,
>   					      PREC_NOT_OPERATOR, NULL);
>     --processing_template_decl;
>     if (check_for_bare_parameter_packs (expr))
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr125490.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr125490.C
> new file mode 100644
> index 000000000000..a33416b232d1
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr125490.C
> @@ -0,0 +1,10 @@
> +// PR c++/125490
> +// { dg-do compile { target c++20 } }
> +
> +void f();
> +
> +template<class T>
> +concept C = f() == 42; // { dg-error "invalid operands" }
> +
> +template<class T>
> +concept D = requires { requires f() == 42; }; // { dg-error "invalid operands" }
Patrick Palka June 2, 2026, 7:53 p.m. UTC | #2 
On Tue, 2 Jun 2026, Jason Merrill wrote:

> On 6/2/26 2:57 PM, Patrick Palka wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
> > OK for trunk/16?
> > 
> > -- >8 --
> > 
> > Here the nonsensical constraint-expression f() == 42 isn't getting
> > rejected ahead of time ultimately because we pass no_toplevel_fold_p=true
> > to cp_parser_binary_expression which makes it use build_min instead of
> > build_x_binary_op to avoid undesirable folding/canonicalization for sake
> > of OpenMP for loop parsing (r145014).  But this certainly doesn't happen
> > during templated parsing, and constraint-expressions are always
> > templated, so we might as well pass no_toplevel_fold_p=false here.
> 
> In this testcase the concept is never checked for satisfaction.  In the
> testcase in the PR there is a similarly nonsensical concept that is used, and
> is diagnosed at that point.  So this patch is about diagnosing more IFNDR
> code, yes?

It's mainly about IFNDR, but there's also name lookup implications:

    struct A {
      constexpr operator int() { return 0; };
      constexpr friend bool operator+(A, int) { return true; } // #1
    };

    template<class T>
    concept C = A{} + A{}; // resolves to #1 with patch, #2 without

    constexpr bool operator+(A, A) { return false; } // #2

    static_assert(C<int>); // succeeds only with patch

> 
> With that clarification to the commit message, OK for trunk but not 16.
> 
> > 	PR c++/125490
> > 
> > gcc/cp/ChangeLog:
> >  that is used
> > 	* parser.cc (cp_parser_constraint_expression): Pass
> > 	no_toplevel_fold_p=false instead of =true to
> > 	cp_parser_binary_expression.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/cpp2a/concepts-pr125490.C: New test.
> > ---
> >   gcc/cp/parser.cc                               |  2 +-
> >   gcc/testsuite/g++.dg/cpp2a/concepts-pr125490.C | 10 ++++++++++
> >   2 files changed, 11 insertions(+), 1 deletion(-)
> >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr125490.C
> > 
> > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> > index b5c8c62118cc..c1797191b833 100644
> > --- a/gcc/cp/parser.cc
> > +++ b/gcc/cp/parser.cc
> > @@ -34761,7 +34761,7 @@ cp_parser_constraint_expression (cp_parser *parser)
> >   {
> >     processing_constraint_expression_sentinel parsing_constraint;
> >     ++processing_template_decl;
> > -  cp_expr expr = cp_parser_binary_expression (parser, false, true,
> > +  cp_expr expr = cp_parser_binary_expression (parser, false, false,
> >   					      PREC_NOT_OPERATOR, NULL);
> >     --processing_template_decl;
> >     if (check_for_bare_parameter_packs (expr))
> > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr125490.C
> > b/gcc/testsuite/g++.dg/cpp2a/concepts-pr125490.C
> > new file mode 100644
> > index 000000000000..a33416b232d1
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr125490.C
> > @@ -0,0 +1,10 @@
> > +// PR c++/125490
> > +// { dg-do compile { target c++20 } }
> > +
> > +void f();
> > +
> > +template<class T>
> > +concept C = f() == 42; // { dg-error "invalid operands" }
> > +
> > +template<class T>
> > +concept D = requires { requires f() == 42; }; // { dg-error "invalid
> > operands" }
> 
>
Jason Merrill June 2, 2026, 8:07 p.m. UTC | #3 
On 6/2/26 3:53 PM, Patrick Palka wrote:
> On Tue, 2 Jun 2026, Jason Merrill wrote:
> 
>> On 6/2/26 2:57 PM, Patrick Palka wrote:
>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
>>> OK for trunk/16?
>>>
>>> -- >8 --
>>>
>>> Here the nonsensical constraint-expression f() == 42 isn't getting
>>> rejected ahead of time ultimately because we pass no_toplevel_fold_p=true
>>> to cp_parser_binary_expression which makes it use build_min instead of
>>> build_x_binary_op to avoid undesirable folding/canonicalization for sake
>>> of OpenMP for loop parsing (r145014).  But this certainly doesn't happen
>>> during templated parsing, and constraint-expressions are always
>>> templated, so we might as well pass no_toplevel_fold_p=false here.
>>
>> In this testcase the concept is never checked for satisfaction.  In the
>> testcase in the PR there is a similarly nonsensical concept that is used, and
>> is diagnosed at that point.  So this patch is about diagnosing more IFNDR
>> code, yes?
> 
> It's mainly about IFNDR, but there's also name lookup implications:
> 
>      struct A {
>        constexpr operator int() { return 0; };
>        constexpr friend bool operator+(A, int) { return true; } // #1
>      };
> 
>      template<class T>
>      concept C = A{} + A{}; // resolves to #1 with patch, #2 without
> 
>      constexpr bool operator+(A, A) { return false; } // #2
> 
>      static_assert(C<int>); // succeeds only with patch

Fair enough.  It also seems that apart from here, currently =true is 
only passed by the omp code so this is simply wrong.  So, OK for 16 as well.

>> With that clarification to the commit message, OK for trunk but not 16.
>>
>>> 	PR c++/125490
>>>
>>> gcc/cp/ChangeLog:
>>>   that is used
>>> 	* parser.cc (cp_parser_constraint_expression): Pass
>>> 	no_toplevel_fold_p=false instead of =true to
>>> 	cp_parser_binary_expression.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	* g++.dg/cpp2a/concepts-pr125490.C: New test.
>>> ---
>>>    gcc/cp/parser.cc                               |  2 +-
>>>    gcc/testsuite/g++.dg/cpp2a/concepts-pr125490.C | 10 ++++++++++
>>>    2 files changed, 11 insertions(+), 1 deletion(-)
>>>    create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr125490.C
>>>
>>> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
>>> index b5c8c62118cc..c1797191b833 100644
>>> --- a/gcc/cp/parser.cc
>>> +++ b/gcc/cp/parser.cc
>>> @@ -34761,7 +34761,7 @@ cp_parser_constraint_expression (cp_parser *parser)
>>>    {
>>>      processing_constraint_expression_sentinel parsing_constraint;
>>>      ++processing_template_decl;
>>> -  cp_expr expr = cp_parser_binary_expression (parser, false, true,
>>> +  cp_expr expr = cp_parser_binary_expression (parser, false, false,
>>>    					      PREC_NOT_OPERATOR, NULL);
>>>      --processing_template_decl;
>>>      if (check_for_bare_parameter_packs (expr))
>>> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr125490.C
>>> b/gcc/testsuite/g++.dg/cpp2a/concepts-pr125490.C
>>> new file mode 100644
>>> index 000000000000..a33416b232d1
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr125490.C
>>> @@ -0,0 +1,10 @@
>>> +// PR c++/125490
>>> +// { dg-do compile { target c++20 } }
>>> +
>>> +void f();
>>> +
>>> +template<class T>
>>> +concept C = f() == 42; // { dg-error "invalid operands" }
>>> +
>>> +template<class T>
>>> +concept D = requires { requires f() == 42; }; // { dg-error "invalid
>>> operands" }
>>
>>
>
diff mbox series

Patch

diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index b5c8c62118cc..c1797191b833 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -34761,7 +34761,7 @@  cp_parser_constraint_expression (cp_parser *parser)
 {
   processing_constraint_expression_sentinel parsing_constraint;
   ++processing_template_decl;
-  cp_expr expr = cp_parser_binary_expression (parser, false, true,
+  cp_expr expr = cp_parser_binary_expression (parser, false, false,
 					      PREC_NOT_OPERATOR, NULL);
   --processing_template_decl;
   if (check_for_bare_parameter_packs (expr))
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr125490.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr125490.C
new file mode 100644
index 000000000000..a33416b232d1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr125490.C
@@ -0,0 +1,10 @@ 
+// PR c++/125490
+// { dg-do compile { target c++20 } }
+
+void f();
+
+template<class T>
+concept C = f() == 42; // { dg-error "invalid operands" }
+
+template<class T>
+concept D = requires { requires f() == 42; }; // { dg-error "invalid operands" }