Re: [PATCH] PING implement pre-c++20 contracts

[Jeff Chapman via Gcc-patches]

Hello again :)  Wanted to shoot a quick status update. Some github issues have
been created for points of feedback, and we've been working on addressing them.
A few changes have been pushed to the contracts-jac-alt branch, while there's
also an active more in depth rewrite branch. Some specific comments inline
below.

On 5/17/21, Jason Merrill  wrote:
> On 5/14/21 4:54 PM, Jason Merrill wrote:
>> On 4/30/21 1:44 PM, Jeff Chapman wrote:
>>> Hello! Looping back around to this. re:
>>> https://gcc.gnu.org/pipermail/gcc-patches/2021-March/567334.html
>>>
>>> On 3/25/21, Jason Merrill  wrote:
>>>> On 3/1/21 8:12 AM, Jeff Chapman wrote:
>>>>> On 1/18/21, Jason Merrill  wrote:
>>>>>> On 1/4/21 9:58 AM, Jeff Chapman wrote:
>>>>>>> Ping. re:
>>>>>>> https://gcc.gnu.org/pipermail/gcc-patches/2020-December/561135.html
>>>>>>> <https://gcc.gnu.org/pipermail/gcc-patches/2020-December/561135.html>
>>>>>>>
>>>>>>> https://github.com/lock3/gcc/tree/contracts-jac-alt
>>>>>>> <https://github.com/lock3/gcc/tree/contracts-jac-alt>
>>>>>>>
>>>>>> Why is some of the code in c-family?  From the modules merge there is
>>>>>> now a cp_handle_option function that you could add the option
>>>>>> handling
>>>>>> to, and I don't see a reason for cxx-contracts.c to be in c-family/
>>>>>> rather than cp/.
>>>>>
>>>>> I've been pushing changes that address the points raised and wanted to
>>>>> ping to see if there's more feedback and give a status summary. The
>>>>> notable change is making sure the implementation plays nicely with
>>>>> modules and a mangling change that did away with a good chunk of code.
>>>>>
>>>>> The contracts code outside of cp/ has been moved into it, and the
>>>>> contract attributes have been altered to work with language
>>>>> independent
>>>>> handling code. More of the contracts code can probably still be
>>>>> moved to
>>>>> cxx-contracts which I'll loop back to after other refactoring. The
>>>>> naming, spelling, and comments (or lack thereof) have been addressed.
>>>>
>>>> Sounds good.  I plan to get back to this when GCC 11 branches, which
>>>> should be mid-April.
>>>
>>> Please let me know if you see any more issues when you pick it back up.
>>> Particularly in modules interop, since I don't think you've had a chance
>>> to look at that yet.
>>>
>>> Completed another merge with master earlier this week which didn't bring
>>> to light any new issues or regressions, but I'll keep on that :)
>>>
>>>>>>> +  /* If we have contracts, check that they're valid in this
>>>>>>> context. */
>>>>>>> +  // FIXME: These aren't entirely correct.
>>>>>>
>>>>>> How not?  Can local extern function decls have contract attributes?
>>>>>>
>>>>>>> + /* FIXME when we instatiate a template class with
>>>>>>> guarded
>>>>>>> +  * members, particularly guarded template members,
>>>>>>> the resulting
>>>>>>> +  * pre/post functions end up being inaccessible
>>>>>>> because their
>>>>>>> +  * template info's context is the original
>>>>>>> uninstantiated class.
>>>>>>
>>>>>> This sounds like a significant functionality gap.  I'm guessing you
>>>>>> want
>>>>>> to reconsider the unshare_template approach.
>>
>> One approach would be to only do the pre/post/guarded/unguarded
>> transformation for a fully-instantiated function; a temploid function
>> would leave the contracts as attributes.
>>

There's an in progress rewrite which moves pre/post function generation until
much later which should resolve this. Like you suggested, these are not created
until we're completely out of template processing.

>>>>>>> +  /* FIXME do we need magic to perfectly forward this so we
>>>>>>> don't clobber
>>>>>>> +RVO/NRVO etc?  */
>>>>>>
>>>>>> Yes.  CALL_FROM_THUNK_P will probably get you some of the magic you
>>>>>> want.
>>>>>
>>

Re: [PATCH] PING implement pre-c++20 contracts

[Jeff Chapman via Gcc-patches]

Hello! Looping back around to this. re:
https://gcc.gnu.org/pipermail/gcc-patches/2021-March/567334.html

On 3/25/21, Jason Merrill  wrote:
> On 3/1/21 8:12 AM, Jeff Chapman wrote:
>> On 1/18/21, Jason Merrill  wrote:
>>> On 1/4/21 9:58 AM, Jeff Chapman wrote:
>>>> Ping. re:
>>>> https://gcc.gnu.org/pipermail/gcc-patches/2020-December/561135.html
>>>> <https://gcc.gnu.org/pipermail/gcc-patches/2020-December/561135.html>
>>>>
>>>> https://github.com/lock3/gcc/tree/contracts-jac-alt
>>>> <https://github.com/lock3/gcc/tree/contracts-jac-alt>
>>>>
>>> Why is some of the code in c-family?  From the modules merge there is
>>> now a cp_handle_option function that you could add the option handling
>>> to, and I don't see a reason for cxx-contracts.c to be in c-family/
>>> rather than cp/.
>>
>> I've been pushing changes that address the points raised and wanted to
>> ping to see if there's more feedback and give a status summary. The
>> notable change is making sure the implementation plays nicely with
>> modules and a mangling change that did away with a good chunk of code.
>>
>> The contracts code outside of cp/ has been moved into it, and the
>> contract attributes have been altered to work with language independent
>> handling code. More of the contracts code can probably still be moved to
>> cxx-contracts which I'll loop back to after other refactoring. The
>> naming, spelling, and comments (or lack thereof) have been addressed.
>
> Sounds good.  I plan to get back to this when GCC 11 branches, which
> should be mid-April.

Please let me know if you see any more issues when you pick it back up.
Particularly in modules interop, since I don't think you've had a chance
to look at that yet.

Completed another merge with master earlier this week which didn't bring
to light any new issues or regressions, but I'll keep on that :)

>>>> +  /* If we have contracts, check that they're valid in this context. */
>>>> +  // FIXME: These aren't entirely correct.
>>>
>>> How not?  Can local extern function decls have contract attributes?
>>>
>>>> + /* FIXME when we instatiate a template class with guarded
>>>> +  * members, particularly guarded template members, the 
>>>> resulting
>>>> +  * pre/post functions end up being inaccessible because their
>>>> +  * template info's context is the original uninstantiated 
>>>> class.
>>>
>>> This sounds like a significant functionality gap.  I'm guessing you want
>>> to reconsider the unshare_template approach.
>>>
>>>> +  /* FIXME do we need magic to perfectly forward this so we don't 
>>>> clobber
>>>> +RVO/NRVO etc?  */
>>>
>>> Yes.  CALL_FROM_THUNK_P will probably get you some of the magic you
>>> want.
>>
>> These points are still being investigated and addressed; including them
>> for reference.
>>
>>> More soon.
>>
>> Please let me know what other issues need work.

If there's anything I can do to make the process smoother please don't
hesitate to ask.

Thank you,
Jeff Chapman

Re: [PATCH] PING implement pre-c++20 contracts

[Jeff Chapman via Gcc-patches]

On 1/18/21, Jason Merrill  wrote:
> On 1/4/21 9:58 AM, Jeff Chapman wrote:
>> Ping. re:
>> https://gcc.gnu.org/pipermail/gcc-patches/2020-December/561135.html
>> <https://gcc.gnu.org/pipermail/gcc-patches/2020-December/561135.html>
>>
>> https://github.com/lock3/gcc/tree/contracts-jac-alt
>> <https://github.com/lock3/gcc/tree/contracts-jac-alt>
>>
> Why is some of the code in c-family?  From the modules merge there is
> now a cp_handle_option function that you could add the option handling
> to, and I don't see a reason for cxx-contracts.c to be in c-family/
> rather than cp/.

I've been pushing changes that address the points raised and wanted to
ping to see if there's more feedback and give a status summary. The
notable change is making sure the implementation plays nicely with
modules and a mangling change that did away with a good chunk of code.

The contracts code outside of cp/ has been moved into it, and the
contract attributes have been altered to work with language independent
handling code. More of the contracts code can probably still be moved to
cxx-contracts which I'll loop back to after other refactoring. The
naming, spelling, and comments (or lack thereof) have been addressed.

>> +set_decl_contracts (tree decl, tree contract_attrs)
> I think you want to use 'chainon' here.

>> +build_arg_list (tree fn)
> You can use is_this_parameter here.

Thanks; I knew of chainon but didn't think of it and is_this_parameter
is new to me. Always fun to delete code :)

>> +  /* If we have contracts, check that they're valid in this context.  */
>> +  // FIXME: These aren't entirely correct.
>
> How not?  Can local extern function decls have contract attributes?
>
>> + /* FIXME when we instatiate a template class with guarded
>> +  * members, particularly guarded template members, the 
>> resulting
>> +  * pre/post functions end up being inaccessible because their
>> +  * template info's context is the original uninstantiated 
>> class.
>
> This sounds like a significant functionality gap.  I'm guessing you want
> to reconsider the unshare_template approach.
>
>> +  /* FIXME do we need magic to perfectly forward this so we don't 
>> clobber
>> +RVO/NRVO etc?  */
>
> Yes.  CALL_FROM_THUNK_P will probably get you some of the magic you want.

These points are still being investigated and addressed; including them
for reference.

> More soon.

Please let me know what other issues need work.


Thank you!
Jeff Chapman

[PATCH] PING implement pre-c++20 contracts

[Jeff Chapman via Gcc-patches]

Ping. re:
https://gcc.gnu.org/pipermail/gcc-patches/2020-December/561135.html

> OK, I'll start with -alt then, thanks.
>
> Andrew is exactly correct, contracts-jac-alt is still the current branch
> we're focusing our upstreaming efforts on.
>
> It's trailing upstream master by a fair bit at this point. I'll get a
> merge pushed shortly.
>

The latest is still on the same branch, which hasn't been updated since
that last merge:
https://github.com/lock3/gcc/tree/contracts-jac-alt

Would you prefer me to keep it from trailing upstream too much through
regular merges, or would it be more beneficial for it to be left alone so
you have a more stable review target?

Please let me know if there's initial feedback I can start addressing, or
anything I can do to help the review process along in general.

Thank you,
Jeff Chapman

Re: [PATCH] implement pre-c++20 contracts

[Jeff Chapman via Gcc-patches]

> OK, I'll start with -alt then, thanks.

Andrew is exactly correct, contracts-jac-alt is still the current branch
we're focusing our upstreaming efforts on.

It's trailing upstream master by a fair bit at this point. I'll get a merge
pushed shortly.

Please let me know if there's anything I can do to help review along!


On Thu, Dec 3, 2020 at 12:41 PM Jason Merrill  wrote:

> On 12/3/20 12:07 PM, Andrew Sutton wrote:
> >
> >  > Attached is a new squashed revision of the patch sans ChangeLogs.
> The
> >  > current work is now being done on github:
> >  > https://github.com/lock3/gcc/tree/contracts-jac-alt
> > 
> >
> > I'm starting to review this now, sorry for the delay. Is this still
> the
> > branch you want me to consider for GCC 11?  I notice that the
> > -constexpr
> > and -mangled-config branches are newer.
> >
> >
> > I think so. Jeff can answer more authoritatively. I know we had one set
> > of changes to the design (how contracts) work aimed at improving the
> > debugging experience for violated contracts. I'm not sure if that's in
> > the jac-alt branch though.
> >
> > The -constexpr branch checks for trivially satisfied contracts (e.g.,
> > [[assert: true]]) and issues warnings. It also preemptively checks
> > preconditions against constant function arguments. It's probably worth
> > reviewing that separately.
> >
> > I'm not sure the -manged-config branch is worth considering for merging
> > at this point. It's trying to solve a problem that might not be worth
> > solving.
>
> OK, I'll start with -alt then, thanks.
>
> > Out of curiosity, are you concerned that future versions of contracts
> > might have considerably different syntax or configurability? I'd hope it
> > wouldn't, but who knows where SG21 is going :)
>
> Not particularly; I figure that most of the implementation would be
> unaffected.
>
> Jason
>
>

[PATCH 2/2] modules: c++: Fix cross module member redecl/add long distance friendship warning

[Jeff Chapman via Gcc-patches]

Attached is a patch adding a -Wlong-distance-friends flag to diagnose long
distance (cross module) friendship.


2020-08-14  Jeff Chapman II  

gcc/c-family/
* c.opt (Wlong-distance-friends): New.

gcc/cp/
* cp-tree.h (module_friendship_compatible): New.
* friend.c (add_friend): Warn when befriending a function owned
by a different module.
(make_friend_class): Warn when befriending a class owned by a
different module.
* module.cc (module_friendship_compatible): New function.
Returns true if a decl may be befriended by another without
issuing a long distance friend warning.

gcc/testsuite/
* g++.dg/modules/long-distance-friend-1_[ab].C: New test.
* g++.dg/modules/long-distance-friend-2_a.[Ch]: Ditto.
* g++.dg/modules/long-distance-friend-3_[ab].[Ch]: Ditto.
* g++.dg/modules/redecl-3_b.C: Add case for -Wlong-distance-friends.


Thanks,
Jeff Chapman II
From 3d229111dcbe4bc3438207a500452c4420fba527 Mon Sep 17 00:00:00 2001
From: Jeff Chapman II 
Date: Fri, 20 Dec 2019 11:02:20 -0500
Subject: [PATCH 2/2] c++: Add warning flag for cross module friendship

2020-08-14  Jeff Chapman II  

gcc/c-family/
	* c.opt (Wlong-distance-friends): New.

gcc/cp/
	* cp-tree.h (module_friendship_compatible): New.
	* friend.c (add_friend): Warn when befriending a function owned
	by a different module.
	(make_friend_class): Warn when befriending a class owned by a
	different module.
	* module.cc (module_friendship_compatible): New function.
	Returns true if a decl may be befriended by another without
	issuing a long distance friend warning.

gcc/testsuite/
	* g++.dg/modules/long-distance-friend-1_[ab].C: New test.
	* g++.dg/modules/long-distance-friend-2_a.[Ch]: Ditto.
	* g++.dg/modules/long-distance-friend-3_[ab].[Ch]: Ditto.
	* g++.dg/modules/redecl-3_b.C: Add case for -Wlong-distance-friends.
---
 gcc/c-family/c.opt|  4 ++
 gcc/cp/cp-tree.h  |  1 +
 gcc/cp/friend.c   | 24 
 gcc/cp/module.cc  | 38 +++
 .../g++.dg/modules/long-distance-friend-1_a.C | 17 +
 .../g++.dg/modules/long-distance-friend-1_b.C | 24 
 .../g++.dg/modules/long-distance-friend-2_a.C | 18 +
 .../g++.dg/modules/long-distance-friend-2_a.h |  7 
 .../g++.dg/modules/long-distance-friend-3_a.C |  9 +
 .../g++.dg/modules/long-distance-friend-3_b.C |  9 +
 .../g++.dg/modules/long-distance-friend-3_b.h | 15 
 gcc/testsuite/g++.dg/modules/redecl-3_b.C |  4 +-
 12 files changed, 168 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/long-distance-friend-1_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/long-distance-friend-1_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/long-distance-friend-2_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/long-distance-friend-2_a.h
 create mode 100644 gcc/testsuite/g++.dg/modules/long-distance-friend-3_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/long-distance-friend-3_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/long-distance-friend-3_b.h

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 506d091e5f2..2d39ac5f974 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -739,6 +739,10 @@ Wlogical-not-parentheses
 C ObjC C++ ObjC++ Var(warn_logical_not_paren) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 Warn when logical not is used on the left hand side operand of a comparison.
 
+Wlong-distance-friends
+C++ Var(warn_long_distance_friends) Warning LangEnabledBy(C++,Wall)
+Warn when friends are declared across module boundaries.
+
 Wlong-long
 C ObjC C++ ObjC++ CPP(cpp_warn_long_long) CppReason(CPP_W_LONG_LONG) Var(warn_long_long) Init(-1) Warning LangEnabledBy(C ObjC,Wc90-c99-compat)
 Do not warn about using \"long long\" when -pedantic.
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index d633b4d5c70..9299625f33b 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7071,6 +7071,7 @@ inline bool module_exporting_p ()
 extern module_state *get_module (tree name, module_state *parent = NULL,
  bool partition = false);
 extern bool module_may_redeclare (tree decl);
+extern bool module_friendship_compatible (tree decl1, tree decl2);
 
 extern int module_initializer_kind ();
 extern void module_add_import_initializers ();
diff --git a/gcc/cp/friend.c b/gcc/cp/friend.c
index 9b27d7d340a..1e6e746a3a5 100644
--- a/gcc/cp/friend.c
+++ b/gcc/cp/friend.c
@@ -173,6 +173,17 @@ add_friend (tree type, tree decl, bool complain)
   if (decl == error_mark_node)
 return;
 
+  if (modules_p ()
+  && complain
+  && warn_long_distance_friends
+  && !module_friendship_compatible (TYPE_NAME (type), decl))
+{
+  warning (OPT_Wlong_distance_friends,
+	   "%q#T is not visible to befriended declaration %q#

[PATCH 1/2] modules: c++: Fix cross module member redecl/add long distance friendship warning

[Jeff Chapman via Gcc-patches]

Hello again,

This is part one of a patchset to add an optional warning for long distance
(cross module) friendship when the friendship has no useful/sensical meaning.

Attached is a patch to error when trying to define a member function of a type
owned by a different module. This also fixes an issue where a friend decl lets
a user define a function owned by a different module.


2020-08-14  Jeff Chapman II  

gcc/cp/
* decl.c (duplicate_decls): Return original decl when
attempting to redeclare a function owned by another module
instead of clobbering the original decl.
(grokfndecl): Error on member declarations of types owned by
another module.

gcc/testsuite/
* g++.dg/modules/redecl-1_[ab].C: New test.
* g++.dg/modules/redecl-2_[ab].C: Ditto.
* g++.dg/modules/redecl-3_[ab].C: Ditto.


Please let me know if there are any questions,
Jeff Chapman II
From fc95c562ec87520f66388a35009649c4b020a843 Mon Sep 17 00:00:00 2001
From: Jeff Chapman II 
Date: Thu, 19 Dec 2019 09:43:16 -0500
Subject: [PATCH 1/2] c++: Fix cross module member redecl

2020-08-14  Jeff Chapman II  

gcc/cp/
	* decl.c (duplicate_decls): Return original decl when
	attempting to redeclare a function owned by another module
	instead of clobbering the original decl.
	(grokfndecl): Error on member declarations of types owned by
	another module.

gcc/testsuite/
	* g++.dg/modules/redecl-1_[ab].C: New test.
	* g++.dg/modules/redecl-2_[ab].C: Ditto.
	* g++.dg/modules/redecl-3_[ab].C: Ditto.
---
 gcc/cp/decl.c | 14 ++
 gcc/testsuite/g++.dg/modules/redecl-1_a.C |  9 +
 gcc/testsuite/g++.dg/modules/redecl-1_b.C |  8 
 gcc/testsuite/g++.dg/modules/redecl-2_a.C | 10 ++
 gcc/testsuite/g++.dg/modules/redecl-2_b.C |  7 +++
 gcc/testsuite/g++.dg/modules/redecl-3_a.C |  6 ++
 gcc/testsuite/g++.dg/modules/redecl-3_b.C | 13 +
 7 files changed, 67 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/modules/redecl-1_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/redecl-1_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/redecl-2_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/redecl-2_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/redecl-3_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/redecl-3_b.C

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index decd58791ae..e8637dac7eb 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -2031,6 +2031,12 @@ duplicate_decls (tree newdecl, tree olddecl, bool newdecl_is_friend)
 	}
 	}
 }
+  /* FIXME Is there a better place to handle this? Without this, we end up
+   * potentially merging a decl owned by a separate module with a friend decl
+   * injected in the current module (see modules/redecl-3). Returning olddecl
+   * relies on code higher up handling the issue.  */
+  else if (modules_p () && !module_may_redeclare (olddecl))
+return olddecl;
 
   /* We have committed to returning OLDDECL at this point.  */
 
@@ -9517,6 +9523,14 @@ grokfndecl (tree ctype,
 
   if (TREE_CODE (type) == METHOD_TYPE)
 {
+  if (modules_p()
+	  && !module_may_redeclare (TYPE_NAME (ctype)))
+	{
+	  error_at (location, "declaration conflicts with import");
+	  inform (location_of (ctype), "import declared %q#T here", ctype);
+	  return NULL_TREE;
+	}
+
   tree parm = build_this_parm (decl, type, quals);
   DECL_CHAIN (parm) = parms;
   parms = parm;
diff --git a/gcc/testsuite/g++.dg/modules/redecl-1_a.C b/gcc/testsuite/g++.dg/modules/redecl-1_a.C
new file mode 100644
index 000..2646adf8327
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/redecl-1_a.C
@@ -0,0 +1,9 @@
+// { dg-additional-options -fmodules-ts }
+export module foo;
+// { dg-module-cmi foo }
+
+export struct Foo
+{
+  int foo();
+};
+
diff --git a/gcc/testsuite/g++.dg/modules/redecl-1_b.C b/gcc/testsuite/g++.dg/modules/redecl-1_b.C
new file mode 100644
index 000..b980bfe290c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/redecl-1_b.C
@@ -0,0 +1,8 @@
+// { dg-additional-options -fmodules-ts }
+import foo;
+
+int Foo::foo() // { dg-error "conflicts with import" }
+{
+  return 1;
+}
+
diff --git a/gcc/testsuite/g++.dg/modules/redecl-2_a.C b/gcc/testsuite/g++.dg/modules/redecl-2_a.C
new file mode 100644
index 000..eea99f2024e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/redecl-2_a.C
@@ -0,0 +1,10 @@
+// { dg-additional-options -fmodules-ts }
+export module foo;
+// { dg-module-cmi foo }
+
+export struct Foo
+{
+  int foo();
+  friend class Bar;
+};
+
diff --git a/gcc/testsuite/g++.dg/modules/redecl-2_b.C b/gcc/testsuite/g++.dg/modules/redecl-2_b.C
new file mode 100644
index 000..9c3b545f6d2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/redecl-2_b.C
@@ -0,0 +1,7 @@
+// { dg-additional-options -fmodules-ts }
+import foo;
+
+struct Bar // { dg-error "cannot declare.*in a different module" }
+

[PATCH] modules: c++: Fix push_namespace ICE with modules

[Jeff Chapman via Gcc-patches]

Hello!

Attached is a patch that fixes an ICE on the devel/c++-modules branch caused
by a slot invalidation edge case in push_namespace.

It's been sitting around for a while and I wasn't sure if I should use the
original date or not. Feel free to adjust that to the commit date if that's
what it should be instead.


2020-05-15  Jeff Chapman II  

gcc/cp/
* name-lookup.c (push_namespace): Fix slot invalidation related
ICE when compiling with modules enabled.

gcc/testsuite/

* g++.dg/modules/string-view1.C: New test.
* g++.dg/modules/string-view2.C: Ditto.


Please let me know if there's anything more I can do,
Jeff Chapman II
From d33a239c187cb6cef1c39953c5f014bd266492de Mon Sep 17 00:00:00 2001
From: Jeff Chapman II 
Date: Fri, 15 May 2020 06:37:41 -0400
Subject: [PATCH 1/1] c++: Fix push_namespace ICE with modules

push_namespace was reusing an earlier find_namespace_slot result after
it was invalidated by pushdecl in corner cases.

2020-05-15  Jeff Chapman II  

gcc/cp/
	* name-lookup.c (push_namespace): Fix slot invalidation related
	ICE when compiling with modules enabled.

gcc/testsuite/

	* g++.dg/modules/string-view1.C: New test.
	* g++.dg/modules/string-view2.C: Ditto.
---
 gcc/cp/name-lookup.c|  2 +
 gcc/testsuite/g++.dg/modules/string-view1.C |  6 +++
 gcc/testsuite/g++.dg/modules/string-view2.C | 53 +
 3 files changed, 61 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/modules/string-view1.C
 create mode 100644 gcc/testsuite/g++.dg/modules/string-view2.C

diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index e9495d2a282..462f028617c 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -8920,6 +8920,8 @@ push_namespace (tree name, bool make_inline)
 	{
 	  /* finish up making the namespace.  */
 	  add_decl_to_level (NAMESPACE_LEVEL (current_namespace), ns);
+	  /* pushdecl may invalidate slot, find name again.  */
+	  slot = find_namespace_slot (current_namespace, name, true);
 	  make_namespace_finish (ns, slot);
 
 	  /* Add the anon using-directive here, we don't do it in
diff --git a/gcc/testsuite/g++.dg/modules/string-view1.C b/gcc/testsuite/g++.dg/modules/string-view1.C
new file mode 100644
index 000..dabc16a8b01
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/string-view1.C
@@ -0,0 +1,6 @@
+// { dg-additional-options "-fmodules-ts" }
+module;
+#include 
+#include 
+export module foo;
+
diff --git a/gcc/testsuite/g++.dg/modules/string-view2.C b/gcc/testsuite/g++.dg/modules/string-view2.C
new file mode 100644
index 000..2e389eacd8f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/string-view2.C
@@ -0,0 +1,53 @@
+// { dg-additional-options "-fmodules-ts" }
+// reduced from string-view1 through cvise. Broken under c++2a too.
+namespace std {
+typedef int a;
+int b;
+decltype(nullptr) c;
+namespace xyz {}
+__builtin_va_list d;
+int n;
+int e;
+int f;
+int g;
+int h;
+int i;
+int j;
+int k;
+typedef struct l m;
+typedef struct aa w;
+typedef struct x o;
+typedef x p;
+long q;
+long r;
+typedef l s;
+extern p ab;
+void t();
+void v();
+extern p ac;
+void ad();
+int ae;
+int af;
+extern p ag;
+extern p ah;
+void ai();
+void y();
+int aj;
+int ak;
+int al;
+char am;
+int an;
+a ao;
+int ap;
+int aq;
+void z();
+int ar;
+int as;
+void at();
+void au();
+void av();
+void aw();
+int u;
+namespace zz {
+}
+}
-- 
2.27.0

Re: [PATCH] implement pre-c++20 contracts

[Jeff Chapman via Gcc-patches]

Ping. Any feedback would be appreciated :)

re: https://gcc.gnu.org/pipermail/gcc-patches/2020-July/549868.html
older reply: https://gcc.gnu.org/pipermail/gcc-patches/2020-May/545339.html

On 7/10/20, Jeff Chapman  wrote:
> Hello again :)
>
> Attached is a new squashed revision of the patch sans ChangeLogs. The
> current work is now being done on github:
> https://github.com/lock3/gcc/tree/contracts-jac-alt
>
> Please let me know if there's a better way to share revisions.
>
>>>> +  /* Check that assertions are null statements.  */
>>>> +  if (attribute_contract_assert_p (contract_attrs))
>>>> +if (token->type != CPP_SEMICOLON)
>>>> +  error_at (token->location, "assertions must be followed by
>>>> %<;%>");
>>>
>>> Better I think to handle this further down where [[fallthrough]] has the
>>> same requirement.
>>>
>>
>> I'm wondering if it would be better to move [[fallthrough]] up, since
>> the later check is not always executed and in the case of [[assert]] we
>> actually need to error.
>>
>>   [[fallthrough]] int x;
>>
>> for instance just gets a generic 'attribute ignored' warning. I'm not
>> entirely happy with how we prevent assert/pre/post from appearing in
>> invalid locations in general which I'll try to improve. If you have
>> concrete suggestions please let me know.
>
> Improvements have been made to handling contract attributes appearing in
> places they don't belong. Any feedback about moving the logic dealing
> with [[fallthrough]]?
>
>
>>> Why not leave the function the user declared as the unchecked function
>>> (just changing some linkage properties) and create a new function for
>>> the checked wrapper?
>>>
>>
>> This revision of the patch does not include changes to the
>> checked/unchecked function split. We're exploring an alternative rewrite
>> that leaves the original function declaration alone and should address
>> or sidestep a number of these comments.
>>
>> Specifically, we're exploring generating pre and post functions with
>> calls to them in the correct places (upon entering a guarded function,
>> wrapping the return value):
>>
>>   int f(int n) [[ pre: n > 0 ]] [[ post r: r < 0 ]] { return -n; }
>>
>> turns into
>>
>>   void __pre_f(int n) { [[ assert: n > 0 ]]; }
>>   int __post_f(int r) { [[ assert: r < 0 ]]; return r; }
>>   int f(int n) {
>> __pre_f(n);
>> return __post_f(-n);
>>   }
>>
>> with whatever hints we can give to optimize this as much as possible.
>
> This is the main change since the last submission. We now leave the
> original decl intact and instead generate a pre and post function.
> Naming and mangling of these pre/post functions needs addressed.
>
> Beyond that, more cleanup has been done. There's still outstanding
> cleanup work, mostly around investigating and improving performance.
> Feedback on the main direction of the patch would be appreciated, and
> any specific commentary or directions of investigation would be helpful.
>
>
>>>> +/* Return the source text between two locations.  */
>>>> +
>>>> +static char *
>>>> +get_source (location_t start, location_t end)
>>>
>>> This seems like it belongs in libcpp.  It also needs to
>>>
>>
>> This has been moved to input since it uses input functions, but needs
>> more work. Was there another comment you had that cutoff?
>>
>> ...snip...
>>
>>>> +  /* We never want to accidentally instantiate templates.  */
>>>> +  if (code == TEMPLATE_DECL)
>>>> +return *tp; /* FIXME? */
>>>
>>> This seems unlikely to have the desired effect; we should see template
>>> instantiations as FUNCTION_DECL or VAR_DECL.  I'm also not sure what the
>>> cp_tree_defined_p check is trying to do; surely using defined functions
>>> and variables can also lead to runtime code?
>>>
>>
>> This is an result of the transform we do for axioms when they're enabled
>> that you may know of a better way to do. Essentially we turn this:
>>
>>   [[ assert axiom: f() > 0 ]];
>>
>> into this:
>>
>>   if (!(f() > 0))
>> __builtin_unreachable ();
>>
>> but we potentially run into issues later if f isn't (yet) defined or is
>> defined in a different translation unit, such as constexpr time. We're
>> avoiding those errors by generating a nop if there's any chance can't be
>> evaluated. We'd prefer something like
>>
>>   __builtin_assume (f() > 0);
>>
>> where the condition is simply ignored if enough information isn't
>> present to affect codegen. Is there a better way to handle this?
>>
>> I should mention that there are likely significant further changes
>> around axiom coming down the pike that may tie into or replace the
>> "is defined" check. Specifically for expressions that _cannot_ be
>> defined rather than ones that are simply not yet defined.
>>
>
> Not much has changed yet regarding axiom, but if you have any
> suggestions for handling any of the above then I'm all ears.
>

Re: [PATCH] implement pre-c++20 contracts

[Jeff Chapman via Gcc-patches]

Hello again :)

Attached is a new squashed revision of the patch sans ChangeLogs. The
current work is now being done on github:
https://github.com/lock3/gcc/tree/contracts-jac-alt

Please let me know if there's a better way to share revisions.

>>> +  /* Check that assertions are null statements.  */
>>> +  if (attribute_contract_assert_p (contract_attrs))
>>> +if (token->type != CPP_SEMICOLON)
>>> +  error_at (token->location, "assertions must be followed by
>>> %<;%>");
>>
>> Better I think to handle this further down where [[fallthrough]] has the
>> same requirement.
>>
>
> I'm wondering if it would be better to move [[fallthrough]] up, since
> the later check is not always executed and in the case of [[assert]] we
> actually need to error.
>
>   [[fallthrough]] int x;
>
> for instance just gets a generic 'attribute ignored' warning. I'm not
> entirely happy with how we prevent assert/pre/post from appearing in
> invalid locations in general which I'll try to improve. If you have
> concrete suggestions please let me know.

Improvements have been made to handling contract attributes appearing in
places they don't belong. Any feedback about moving the logic dealing
with [[fallthrough]]?


>> Why not leave the function the user declared as the unchecked function
>> (just changing some linkage properties) and create a new function for
>> the checked wrapper?
>>
>
> This revision of the patch does not include changes to the
> checked/unchecked function split. We're exploring an alternative rewrite
> that leaves the original function declaration alone and should address
> or sidestep a number of these comments.
>
> Specifically, we're exploring generating pre and post functions with
> calls to them in the correct places (upon entering a guarded function,
> wrapping the return value):
>
>   int f(int n) [[ pre: n > 0 ]] [[ post r: r < 0 ]] { return -n; }
>
> turns into
>
>   void __pre_f(int n) { [[ assert: n > 0 ]]; }
>   int __post_f(int r) { [[ assert: r < 0 ]]; return r; }
>   int f(int n) {
> __pre_f(n);
> return __post_f(-n);
>   }
>
> with whatever hints we can give to optimize this as much as possible.

This is the main change since the last submission. We now leave the
original decl intact and instead generate a pre and post function.
Naming and mangling of these pre/post functions needs addressed.

Beyond that, more cleanup has been done. There's still outstanding
cleanup work, mostly around investigating and improving performance.
Feedback on the main direction of the patch would be appreciated, and
any specific commentary or directions of investigation would be helpful.


>>> +/* Return the source text between two locations.  */
>>> +
>>> +static char *
>>> +get_source (location_t start, location_t end)
>>
>> This seems like it belongs in libcpp.  It also needs to
>>
>
> This has been moved to input since it uses input functions, but needs
> more work. Was there another comment you had that cutoff?
>
> ...snip...
>
>>> +  /* We never want to accidentally instantiate templates.  */
>>> +  if (code == TEMPLATE_DECL)
>>> +return *tp; /* FIXME? */
>>
>> This seems unlikely to have the desired effect; we should see template
>> instantiations as FUNCTION_DECL or VAR_DECL.  I'm also not sure what the
>> cp_tree_defined_p check is trying to do; surely using defined functions
>> and variables can also lead to runtime code?
>>
>
> This is an result of the transform we do for axioms when they're enabled
> that you may know of a better way to do. Essentially we turn this:
>
>   [[ assert axiom: f() > 0 ]];
>
> into this:
>
>   if (!(f() > 0))
> __builtin_unreachable ();
>
> but we potentially run into issues later if f isn't (yet) defined or is
> defined in a different translation unit, such as constexpr time. We're
> avoiding those errors by generating a nop if there's any chance can't be
> evaluated. We'd prefer something like
>
>   __builtin_assume (f() > 0);
>
> where the condition is simply ignored if enough information isn't
> present to affect codegen. Is there a better way to handle this?
>
> I should mention that there are likely significant further changes
> around axiom coming down the pike that may tie into or replace the
> "is defined" check. Specifically for expressions that _cannot_ be
> defined rather than ones that are simply not yet defined.
>

Not much has changed yet regarding axiom, but if you have any
suggestions for handling any of the above then I'm all ears.


0001-Implement-pre-c-20-contracts.patch.gz
Description: GNU Zip compressed data

Re: [PATCH] implement pre-c++20 contracts

[Jeff Chapman via Gcc-patches]

Hello,

On 12/10/19, Jason Merrill wrote:
> On 11/13/19, Jeff Chapman wrote:
>> Attached is a patch that implements pre-c++20 contracts. This comes
>> from a long running development branch which included ChangeLog entries
>> as we went, which are included in the patch itself. The repo and
>> initial wiki are located here:
>> https://gitlab.com/lock3/gcc-new/wikis/GCC-with-Contracts
>
> Thanks.  I've mostly been referring to the repo rather than the attached
> patch.  Below are a bunch of comments about the implementation, in no
> particular order.
>

I've attached a new squashed revision of the patch, and you can see the
changes I've made from your input on the contracts-jac-prep branch:
https://gitlab.com/lock3/gcc-new/-/tree/contracts-jac-prep . If there's
an easier format for you to review that I can produce please let me
know.

I'll address a few things inline below. Everything else should either be
handled or explained by Andrew's last email. If anything needs further
addressing or something hasn't been brought up yet please let me know :)


>> +handle_OPT_fcontract_build_level_ (const char *arg)
>> +{
>> +  if (contracts_p1332_default || contracts_p1332_review ||
>> contracts_p1429)
>> +{
>> +  error ("-fcontract-build-level= cannot be mixed with p1332/p1429");
>
> Hmm, P1429 includes the notion of build level, it's just checked after
> explicit semantics.  In general, P1429 seems like a compatible extension
> of the semantics previously in the working paper.
>
> P1332 could also be treated as compatible if we consider the P0542 build
> level to affect the default role as specified in P1429.  P1680 seems to
> suggest that this is what you had in mind.
>

These could possibly be made compatible, but in some cases the flags are
changing the same entries in the table. That would require deciding
whether flag ordering matters or whether a certain flags can't change
values set by other flags.

I'm not sure it's a worthwhile change. While it increases the valid
space of command line invocations, it doesn't actually increase the the
result space. I'd prefer an eventual solution that removed flags
entirely instead.


>> +  /* Check that assertions are null statements.  */
>> +  if (attribute_contract_assert_p (contract_attrs))
>> +if (token->type != CPP_SEMICOLON)
>> +  error_at (token->location, "assertions must be followed by
>> %<;%>");
>
> Better I think to handle this further down where [[fallthrough]] has the
> same requirement.
>

I'm wondering if it would be better to move [[fallthrough]] up, since
the later check is not always executed and in the case of [[assert]] we
actually need to error.

  [[fallthrough]] int x;

for instance just gets a generic 'attribute ignored' warning. I'm not
entirely happy with how we prevent assert/pre/post from appearing in
invalid locations in general which I'll try to improve. If you have
concrete suggestions please let me know.


> Why not leave the function the user declared as the unchecked function
> (just changing some linkage properties) and create a new function for
> the checked wrapper?
>

This revision of the patch does not include changes to the
checked/unchecked function split. We're exploring an alternative rewrite
that leaves the original function declaration alone and should address
or sidestep a number of these comments.

Specifically, we're exploring generating pre and post functions with
calls to them in the correct places (upon entering a guarded function,
wrapping the return value):

  int f(int n) [[ pre: n > 0 ]] [[ post r: r < 0 ]] { return -n; }

turns into

  void __pre_f(int n) { [[ assert: n > 0 ]]; }
  int __post_f(int r) { [[ assert: r < 0 ]]; return r; }
  int f(int n) {
__pre_f(n);
return __post_f(-n);
  }

with whatever hints we can give to optimize this as much as possible.


>> +/* Return the source text between two locations.  */
>> +
>> +static char *
>> +get_source (location_t start, location_t end)
>
> This seems like it belongs in libcpp.  It also needs to
>

This has been moved to input since it uses input functions, but needs
more work. Was there another comment you had that cutoff?


>> +  tree level_str = build_string_literal (strlen (level) + 1, level);
>> +  tree role_str = build_string_literal (strlen (role) + 1, role);
>
> Maybe combine these into a single string argument?
>

These are used separately in std::contract_violation and to my
understanding building them separately will collapse duplicate levels
and roles instead of duplicating them unless they both match -- is that
correct?


>> +  /* We never want to accidentally instantiate templates.  */
>> +  if (code == TEMPLATE_DECL)
>> +return *tp; /

[PATCH] implement pre-c++20 contracts

[Jeff Chapman]

Hello,

Attached is a patch that implements pre-c++20 contracts. This comes
from a long running development branch which included ChangeLog entries
as we went, which are included in the patch itself. The repo and
initial wiki are located here:
https://gitlab.com/lock3/gcc-new/wikis/GCC-with-Contracts

We've previously circulated a paper (P1680) which documents our
implementation experience which largely covers new flags and features.
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1680r0.pdf

That paper documents our changes in depth, barring the recently added
-fcontracts flag which is a global opt-in for contracts functionality.
As an overview of what we've done is included below for convenience.

The following switches have been added:

-fcontracts
  enable contract features in general

Flags from the old Working Draft:

-fcontract-build-level=[off|default|audit]
  specify max contract level to generate runtime checks for

-fcontract-continuation-mode=[on|off]
  toggle whether execution resumes after contract failure

Flags from P1290:

-fcontract-assumption-mode=[on|off]
  enable treating axiom level contracts as compile time assumptions
  (default on)

Flags from P1429:

-fcontract-mode=[on|off]
  enable or disable all contract facilities (default on)

-fcontract-semantic=:
  specify the concrete semantics for a level

Flags from P1332:

-fcontract-role=:
  specify semantics for all levels in a role (default, review, or a
custom role)
  (ex: opt:assume,assume,assume)

Additional flags:

-fcontract-strict-declarations=[on|off]
  toggle warnings on generalized redecl of member functions
without contracts (default off)


One assert contract may be present on any block scope empty statement:
  [[ assert contract-mode_opt : conditional-expression ]]

Function declarations have an optional, ordered, list of pre and post
contracts:
  [[ pre contract-mode_opt : conditional-expression ]]
  [[ post contract-mode_opt identifier_opt : conditional-expression ]]


The grammar for the contract-mode_opt portion which configures the
concrete semantics of the contracts is:

contract-mode
  contract-semantic
  contract-level_opt contract-role_opt

contract-semantic
  ignore
  check_never_continue
  check_maybe_continue
  check_always_continue
  assume

contract-level
  default
  audit
  axiom

contract-role
  %default
  %identifier


Contracts are configured via concrete semantics or by an optional
level and role which map to one of the concrete semantics:

  ignore – The contract does not affect the behavior of the program.
  assume – The condition may be used for optimization.
  never_continue – The program terminates if the contract is
   not satisfied.
  maybe_continue – A user-defined violation handler may terminate the
   program.
  always_continue – The program continues even if the contract is not
satisfied.


-fcontracts enables generalized member function redeclaration. That is,
non-defining declarations of member functions are allowed outside the
initial class definition.

Contracts are not required on the initial declaration of a
(non-virtual) function as long as all TUs that see a set of contracts
eventually see the same set of contracts. All contracts must be present
on the first declaration for virtual functions to ensure we know to
split the function in all overrides.


Explicit template specializations have an independent set of contracts
than any other explicit specializations.


Functions with contracts (which we call guarded functions) are split
just before genericization. A wrapper which checks pre and post
contracts is emitted with the original function name, while the
original function body is emitted under a new unchecked name. This
means that calls to functions in TUs which never see the contract list
still call a checked version of the function (see insulated contracts
in P1680).

Having a checked and unchecked version of the function makes it
potentially easier to inline the checks into the caller, and prevents
the need to repeat the post contracts at all return statements in the
original function.


A custom contract violation handler can be installed by defining
  void handle_contract_violation(const std::contract_violation &)

(you must #include  for this to work) or by using LD_PRELOAD.
An example of defining the handler in the main TU can be found in the
testsuite g++.dg/cpp2a/contracts16.C .

Examples of how to use LD_PRELOAD to install a custom violation handler
are available in the contracts folder inside the testsuite.


Bootstrapped and tested on x86_64-pc-linux-gnu. cmcstl2 compiles cleanly
and has no `make test` failures.


Please let me know if you have any questions or know what the next
steps will be.


Thank you,
Jeff Chapman II


0001-Implement-pre-c-20-contracts.patch.gz
Description: application/gzip

[PATCH] PR c++/84810 - constraints on lambdas

[Jeff Chapman]

Hello,

Attached is a patch that adds parsing of the optional requires-clause in a
lambda-expression and lambda-declarator. Additionally, shorthand constraints
from the template-parameter-list are now actually applied and constrain the
synthesized operator().

Previously we were not parsing the requires clauses at all and not saving the
shorthand constraints in the place expected by grokfndecl.

The trailing requires-clause is now also used to suppress synthesis of the
conversion to function pointer for non-capturing non-generic lambdas as per
expr.prim.lambda.closure/7.

This includes a fix to template_class_depth. Previously it was computing the
wrong depth for lambdas in the initializer of a static member of a class
template, exhibited by the concepts-lambda4 test which currently fails on
trunk. The bug was causing grokfndecl to use the constraints from the template
class for the lambda.

gcc/cp/
2019-10-30  Jeff Chapman II  

PR c++/84810 - constraints on lambdas
* lambda.c (maybe_add_lambda_conv_op): Do not synthesize
conversion if the call operator does not satisfy its constraints.
* parser.c (cp_parser_lambda_declarator_opt): Parse
requires-clause on generic lambdas; combine with shorthand
constraints. Parse trailing requires-clause and attach to the
synthesized call operator.
* pt.c (template_class_depth): Only inspect
LAMBDA_TYPE_EXTRA_SCOPE if it is present. This fixes an
incorrect depth calculation for lambdas inside the initializer
of a static data member of a template class.

gcc/testsuite/
2019-10-30  Jeff Chapman II  

PR c++/84810 - constraints on lambdas
* g++.dg/cpp2a/concepts-lambda2.C: New test.
* g++.dg/cpp2a/concepts-lambda3.C: Ditto.
* g++.dg/cpp2a/concepts-lambda4.C: Ditto.
* g++.dg/cpp2a/concepts-pr84810.C: Ditto.

Bootstrapped and tested on x86_64-pc-linux-gnu.

Please let me know if there's any issues.

Thanks,
Jeff Chapman II
From d196b03e6f6924935a521a8d140d621d03ae18f2 Mon Sep 17 00:00:00 2001
From: Jeff Chapman II 
Date: Mon, 28 Oct 2019 12:57:36 -0400
Subject: [PATCH 1/1] PR c++/84810 - constraints on lambdas

gcc/cp/
2019-10-30  Jeff Chapman II  

	PR c++/84810 - constraints on lambdas
	* lambda.c (maybe_add_lambda_conv_op): Do not synthesize
	conversion if the call operator does not satisfy its constraints.
	* parser.c (cp_parser_lambda_declarator_opt): Parse
	requires-clause on generic lambdas; combine with shorthand
	constraints. Parse trailing requires-clause and attach to the
	synthesized call operator.
	* pt.c (template_class_depth): Only inspect
	LAMBDA_TYPE_EXTRA_SCOPE if it is present. This fixes an
	incorrect depth calculation for lambdas inside the initializer
	of a static data member of a template class.

gcc/testsuite/
2019-10-30  Jeff Chapman II  

	PR c++/84810 - constraints on lambdas
	* g++.dg/cpp2a/concepts-lambda2.C: New test.
	* g++.dg/cpp2a/concepts-lambda3.C: Ditto.
	* g++.dg/cpp2a/concepts-lambda4.C: Ditto.
	* g++.dg/cpp2a/concepts-pr84810.C: Ditto.
---
 gcc/cp/lambda.c   |   6 +
 gcc/cp/parser.c   |  21 ++-
 gcc/cp/pt.c   |   2 +-
 gcc/testsuite/g++.dg/cpp2a/concepts-lambda2.C | 153 ++
 gcc/testsuite/g++.dg/cpp2a/concepts-lambda3.C |  64 
 gcc/testsuite/g++.dg/cpp2a/concepts-lambda4.C |  14 ++
 gcc/testsuite/g++.dg/cpp2a/concepts-pr84810.C |  13 ++
 7 files changed, 270 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda2.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda3.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda4.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr84810.C

diff --git a/gcc/cp/lambda.c b/gcc/cp/lambda.c
index f128ed800f6..d621beca2eb 100644
--- a/gcc/cp/lambda.c
+++ b/gcc/cp/lambda.c
@@ -1046,6 +1046,12 @@ maybe_add_lambda_conv_op (tree type)
   return;
 }
 
+  /* Non-generic non-capturing lambdas only have a conversion function to
+ pointer to function when the trailing requires-clause's constraints are
+ satisfied.  */
+  if (!generic_lambda_p && !constraints_satisfied_p (callop))
+return;
+
   /* Non-template conversion operators are defined directly with build_call_a
  and using DIRECT_ARGVEC for arguments (including 'this').  Templates are
  deferred and the CALL is built in-place.  In the case of a deduced return
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 3857fe47d67..bbdf8d69077 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -10854,11 +10854,13 @@ cp_parser_lambda_introducer (cp_parser* parser, tree lambda_expr)
 
lambda-declarator:
  < template-parameter-list [opt] >
+   requires-clause [opt]
  ( parameter-declaration-clause [opt] )
attribute-specifier [opt]
decl-specifier-seq [opt]
exception-spec