coroutines: Fix cases where proxy variables are used [PR94879]

Message ID 2D37A3D2-F639-40A7-B06E-9F4AFAFF77F8@sandoe.co.uk
State New
Headers show
Series
  • coroutines: Fix cases where proxy variables are used [PR94879]
Related show

Commit Message

Iain Sandoe April 30, 2020, 12:33 p.m.
Hi,

This was found when attempting to build folly’s experimental coros
tests.

tested on x86_64-darwin, linux, powerpc64-linux
ok for master?
thanks
Iain

----

There are several places where the handling of a variable
declaration depends on whether it corresponds to a compiler
temporary, or to some other entity.  We were testing that var
decls were artificial in determining this.  However, proxy vars
are also artificial so that this is not sufficient.  The solution
is to exclude variables with a DECL_VALUE_EXPR as well, since
the value variable will not be a temporary.

gcc/cp/ChangeLog:

2020-04-30  Iain Sandoe  <iain@sandoe.co.uk>

	PR c++/94879
	* coroutines.cc (build_co_await): Account for variables
	with DECL_VALUE_EXPRs.
	(captures_temporary): Likewise.
	(register_awaits): Likewise.

gcc/testsuite/ChangeLog:

2020-04-30  Iain Sandoe  <iain@sandoe.co.uk>

	PR c++/94879
	* g++.dg/coroutines/pr94879-folly-1.C: New test.

---
 gcc/cp/coroutines.cc                          |  9 ++--
 .../g++.dg/coroutines/pr94879-folly-1.C       | 49 +++++++++++++++++++
 2 files changed, 55 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/pr94879-folly-1.C

-- 
2.24.1

Comments

Nathan Sidwell April 30, 2020, 1:40 p.m. | #1
On 4/30/20 8:33 AM, Iain Sandoe wrote:

> There are several places where the handling of a variable

> declaration depends on whether it corresponds to a compiler

> temporary, or to some other entity.  We were testing that var

> decls were artificial in determining this.  However, proxy vars

> are also artificial so that this is not sufficient.  The solution

> is to exclude variables with a DECL_VALUE_EXPR as well, since

> the value variable will not be a temporary.


Ok.  This approach seems rather brittle, but it is what we have.

> gcc/cp/ChangeLog:

> 

> 2020-04-30  Iain Sandoe  <iain@sandoe.co.uk>

> 

> 	PR c++/94879

> 	* coroutines.cc (build_co_await): Account for variables

> 	with DECL_VALUE_EXPRs.

> 	(captures_temporary): Likewise.

> 	(register_awaits): Likewise.

> 

> gcc/testsuite/ChangeLog:

> 

> 2020-04-30  Iain Sandoe  <iain@sandoe.co.uk>

> 

> 	PR c++/94879

> 	* g++.dg/coroutines/pr94879-folly-1.C: New test.

> 

> ---

>   gcc/cp/coroutines.cc                          |  9 ++--

>   .../g++.dg/coroutines/pr94879-folly-1.C       | 49 +++++++++++++++++++

>   2 files changed, 55 insertions(+), 3 deletions(-)

>   create mode 100644 gcc/testsuite/g++.dg/coroutines/pr94879-folly-1.C

> 

> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc

> index 7bb3e98fe6c..e2dbeabf48b 100644

> --- a/gcc/cp/coroutines.cc

> +++ b/gcc/cp/coroutines.cc

> @@ -748,7 +748,8 @@ build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind)

>     if (INDIRECT_REF_P (e_proxy))

>       e_proxy = TREE_OPERAND (e_proxy, 0);

>     if (TREE_CODE (e_proxy) == PARM_DECL

> -      || (TREE_CODE (e_proxy) == VAR_DECL && !DECL_ARTIFICIAL (e_proxy)))

> +      || (VAR_P (e_proxy) && (!DECL_ARTIFICIAL (e_proxy)

> +			      || DECL_HAS_VALUE_EXPR_P (e_proxy))))

>       e_proxy = o;

>     else

>       {

> @@ -2659,7 +2660,8 @@ captures_temporary (tree *stmt, int *do_subtree, void *d)

>   	}

>   

>         /* This isn't a temporary.  */

> -      if ((TREE_CODE (parm) == VAR_DECL && !DECL_ARTIFICIAL (parm))

> +      if ((VAR_P (parm)

> +	   && (!DECL_ARTIFICIAL (parm) || DECL_HAS_VALUE_EXPR_P (parm)))

>   	  || TREE_CODE (parm) == PARM_DECL

>   	  || TREE_CODE (parm) == NON_LVALUE_EXPR)

>   	continue;

> @@ -2742,7 +2744,8 @@ register_awaits (tree *stmt, int *do_subtree ATTRIBUTE_UNUSED, void *d)

>     if (INDIRECT_REF_P (aw))

>       aw = TREE_OPERAND (aw, 0);

>     if (TREE_CODE (aw) == PARM_DECL

> -      || (TREE_CODE (aw) == VAR_DECL && !DECL_ARTIFICIAL (aw)))

> +      || (VAR_P (aw) && (!DECL_ARTIFICIAL (aw)

> +			 || DECL_HAS_VALUE_EXPR_P (aw))))

>       ; /* Don't make an additional copy.  */

>     else

>       {

> diff --git a/gcc/testsuite/g++.dg/coroutines/pr94879-folly-1.C b/gcc/testsuite/g++.dg/coroutines/pr94879-folly-1.C

> new file mode 100644

> index 00000000000..7d66ce004f7

> --- /dev/null

> +++ b/gcc/testsuite/g++.dg/coroutines/pr94879-folly-1.C

> @@ -0,0 +1,49 @@

> +//  { dg-additional-options  "-fpreprocessed -w" }

> +

> +namespace std {

> +template <typename a> a b(a &&);

> +template <typename c> struct d { c e; };

> +template <typename f, typename> struct coroutine_traits : f {};

> +template <typename = void> struct coroutine_handle;

> +template <> struct coroutine_handle<> {};

> +template <typename> struct coroutine_handle : coroutine_handle<> {};

> +struct g {};

> +} // namespace std

> +

> +class h {};

> +class i {

> +  i(i &&);

> +};

> +

> +namespace ac {

> +template <typename> class ad {

> +public:

> +  bool await_ready();

> +  void await_resume();

> +  void await_suspend(std::coroutine_handle<>);

> +  i ae;

> +};

> +} // namespace ac

> +

> +template <typename ab> ac::ad<ab> operator co_await(ab);

> +class j {

> +  class l {};

> +

> +public:

> +  std::g initial_suspend();

> +  l final_suspend();

> +};

> +class m : public j {

> +public:

> +  void get_return_object();

> +  void unhandled_exception();

> +};

> +class n {

> +public:

> +  using promise_type = m;

> +};

> +std::d<h> k;

> +void a() {

> +  auto am = k;

> +  [&]() -> n { co_await std::b(am.e); };

> +}

> 



-- 
Nathan Sidwell

Patch

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 7bb3e98fe6c..e2dbeabf48b 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -748,7 +748,8 @@  build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind)
   if (INDIRECT_REF_P (e_proxy))
     e_proxy = TREE_OPERAND (e_proxy, 0);
   if (TREE_CODE (e_proxy) == PARM_DECL
-      || (TREE_CODE (e_proxy) == VAR_DECL && !DECL_ARTIFICIAL (e_proxy)))
+      || (VAR_P (e_proxy) && (!DECL_ARTIFICIAL (e_proxy)
+			      || DECL_HAS_VALUE_EXPR_P (e_proxy))))
     e_proxy = o;
   else
     {
@@ -2659,7 +2660,8 @@  captures_temporary (tree *stmt, int *do_subtree, void *d)
 	}
 
       /* This isn't a temporary.  */
-      if ((TREE_CODE (parm) == VAR_DECL && !DECL_ARTIFICIAL (parm))
+      if ((VAR_P (parm)
+	   && (!DECL_ARTIFICIAL (parm) || DECL_HAS_VALUE_EXPR_P (parm)))
 	  || TREE_CODE (parm) == PARM_DECL
 	  || TREE_CODE (parm) == NON_LVALUE_EXPR)
 	continue;
@@ -2742,7 +2744,8 @@  register_awaits (tree *stmt, int *do_subtree ATTRIBUTE_UNUSED, void *d)
   if (INDIRECT_REF_P (aw))
     aw = TREE_OPERAND (aw, 0);
   if (TREE_CODE (aw) == PARM_DECL
-      || (TREE_CODE (aw) == VAR_DECL && !DECL_ARTIFICIAL (aw)))
+      || (VAR_P (aw) && (!DECL_ARTIFICIAL (aw)
+			 || DECL_HAS_VALUE_EXPR_P (aw))))
     ; /* Don't make an additional copy.  */
   else
     {
diff --git a/gcc/testsuite/g++.dg/coroutines/pr94879-folly-1.C b/gcc/testsuite/g++.dg/coroutines/pr94879-folly-1.C
new file mode 100644
index 00000000000..7d66ce004f7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr94879-folly-1.C
@@ -0,0 +1,49 @@ 
+//  { dg-additional-options  "-fpreprocessed -w" }
+
+namespace std {
+template <typename a> a b(a &&);
+template <typename c> struct d { c e; };
+template <typename f, typename> struct coroutine_traits : f {};
+template <typename = void> struct coroutine_handle;
+template <> struct coroutine_handle<> {};
+template <typename> struct coroutine_handle : coroutine_handle<> {};
+struct g {};
+} // namespace std
+
+class h {};
+class i {
+  i(i &&);
+};
+
+namespace ac {
+template <typename> class ad {
+public:
+  bool await_ready();
+  void await_resume();
+  void await_suspend(std::coroutine_handle<>);
+  i ae;
+};
+} // namespace ac
+
+template <typename ab> ac::ad<ab> operator co_await(ab);
+class j {
+  class l {};
+
+public:
+  std::g initial_suspend();
+  l final_suspend();
+};
+class m : public j {
+public:
+  void get_return_object();
+  void unhandled_exception();
+};
+class n {
+public:
+  using promise_type = m;
+};
+std::d<h> k;
+void a() {
+  auto am = k;
+  [&]() -> n { co_await std::b(am.e); };
+}