diff mbox series

c++: Fix return type deduction with an abbreviated function template

Message ID 20200210132032.586931-1-ppalka@redhat.com
State New
Headers show
Series c++: Fix return type deduction with an abbreviated function template | expand

Commit Message

Patrick Palka Feb. 10, 2020, 1:20 p.m. UTC
This patch fixes two issues with return type deduction in the presence of an
abbreviated function template.

The first issue (PR 69448) is that if a placeholder auto return type contains
any modifiers such as & or *, then the abbreviated-function-template
compensation in splice_late_return_type does not get performed for the
underlying auto node, leading to incorrect return type deduction.  This happens
because splice_late_return_type checks for a placeholder auto with is_auto,
which does not look through modifiers.  This patch replaces the use of is_auto
with find_type_usage, but it first refactors find_type_usage to return a pointer
to the matched tree so that we next subsequently replace it.

The second issue (PR 80471) is that the AUTO_IS_DECLTYPE flag is not being
preserved in splice_late_return_type when compensating for an abbreviated
function template, leading to us treating a decltype(auto) return type as if it
was an auto return type.  Fixed by propagating AUTO_IS_DECLTYPE.  The test for
PR 80471 is adjusted to expect the correct behavior.

Bootstrapped and regtested on x86_64-pc-linux-gnu, is this OK to commit?

gcc/cp/ChangeLog:

	PR c++/69448
	PR c++/80471
	* type-utils.h (find_type_usage): Refactor to take a tree * and to
	return a tree *, and update documentation accordingly.
	* pt.c (splice_late_return_type): Use find_type_usage to find and
	replace a possibly nested auto node.  Preserve the AUTO_IS_DECLTYPE
	flag when replacing the node.
	(type_uses_auto): Adjust the call to find_type_usage.

gcc/testsuite/ChangeLog:

	PR c++/69448
	PR c++/80471
	* g++.dg/concepts/abbrev3.C: New test.
	* g++.dg/cpp2a/concepts-pr80471.C: Adjust a static_assert to expect the
	correct behavior.
---
 gcc/cp/pt.c                                   | 26 ++++++++++++-------
 gcc/cp/type-utils.h                           | 26 +++++++++----------
 gcc/testsuite/g++.dg/concepts/abbrev3.C       | 11 ++++++++
 gcc/testsuite/g++.dg/cpp2a/concepts-pr80471.C |  2 +-
 4 files changed, 41 insertions(+), 24 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/concepts/abbrev3.C

Comments

Jason Merrill Feb. 10, 2020, 6:26 p.m. UTC | #1
On 2/10/20 2:20 PM, Patrick Palka wrote:
> This patch fixes two issues with return type deduction in the presence of an
> abbreviated function template.
> 
> The first issue (PR 69448) is that if a placeholder auto return type contains
> any modifiers such as & or *, then the abbreviated-function-template
> compensation in splice_late_return_type does not get performed for the
> underlying auto node, leading to incorrect return type deduction.  This happens
> because splice_late_return_type checks for a placeholder auto with is_auto,
> which does not look through modifiers.  This patch replaces the use of is_auto
> with find_type_usage, but it first refactors find_type_usage to return a pointer
> to the matched tree so that we next subsequently replace it.
> 
> The second issue (PR 80471) is that the AUTO_IS_DECLTYPE flag is not being
> preserved in splice_late_return_type when compensating for an abbreviated
> function template, leading to us treating a decltype(auto) return type as if it
> was an auto return type.  Fixed by propagating AUTO_IS_DECLTYPE.  The test for
> PR 80471 is adjusted to expect the correct behavior.

The comment in make_constrained_decltype_auto suggests that we should 
set AUTO_IS_DECLTYPE in make_auto_1 if name == decltype_auto_identifier; 
then callers won't need to worry about it.

> Bootstrapped and regtested on x86_64-pc-linux-gnu, is this OK to commit?
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/69448
> 	PR c++/80471
> 	* type-utils.h (find_type_usage): Refactor to take a tree * and to
> 	return a tree *, and update documentation accordingly.
> 	* pt.c (splice_late_return_type): Use find_type_usage to find and
> 	replace a possibly nested auto node.  Preserve the AUTO_IS_DECLTYPE
> 	flag when replacing the node.
> 	(type_uses_auto): Adjust the call to find_type_usage.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/69448
> 	PR c++/80471
> 	* g++.dg/concepts/abbrev3.C: New test.
> 	* g++.dg/cpp2a/concepts-pr80471.C: Adjust a static_assert to expect the
> 	correct behavior.
> ---
>   gcc/cp/pt.c                                   | 26 ++++++++++++-------
>   gcc/cp/type-utils.h                           | 26 +++++++++----------
>   gcc/testsuite/g++.dg/concepts/abbrev3.C       | 11 ++++++++
>   gcc/testsuite/g++.dg/cpp2a/concepts-pr80471.C |  2 +-
>   4 files changed, 41 insertions(+), 24 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/concepts/abbrev3.C
> 
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index 2fb52caa5d4..c83aaa7c7e1 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -28904,17 +28904,21 @@ do_auto_deduction (tree type, tree init, tree auto_node,
>   tree
>   splice_late_return_type (tree type, tree late_return_type)
>   {
> -  if (is_auto (type))
> -    {
> -      if (late_return_type)
> -	return late_return_type;
> +  if (is_auto (type) && late_return_type)
> +    return late_return_type;

It occurs to me that we could just check late_return_type here, maybe 
change the is_auto test to an assert.

>   
> -      tree idx = get_template_parm_index (type);
> +  if (tree *auto_node = find_type_usage (&type, is_auto))
> +    {
> +      tree idx = get_template_parm_index (*auto_node);
>         if (TEMPLATE_PARM_LEVEL (idx) <= processing_template_decl)
> -	/* In an abbreviated function template we didn't know we were dealing
> -	   with a function template when we saw the auto return type, so update
> -	   it to have the correct level.  */
> -	return make_auto_1 (TYPE_IDENTIFIER (type), true);
> +	{
> +	  /* In an abbreviated function template we didn't know we were dealing
> +	     with a function template when we saw the auto return type, so
> +	     update it to have the correct level.  */
> +	  tree new_node = make_auto_1 (TYPE_IDENTIFIER (*auto_node), true);
> +	  AUTO_IS_DECLTYPE (new_node) = AUTO_IS_DECLTYPE (*auto_node);
> +	  *auto_node = new_node;
> +	}
>       }
>     return type;
>   }
> @@ -28960,8 +28964,10 @@ type_uses_auto (tree type)
>         else
>   	return NULL_TREE;
>       }
> +  else if (tree *tp = find_type_usage (&type, is_auto))
> +    return *tp;
>     else
> -    return find_type_usage (type, is_auto);
> +    return NULL_TREE;
>   }
>   
>   /* Report ill-formed occurrences of auto types in ARGUMENTS.  If
> diff --git a/gcc/cp/type-utils.h b/gcc/cp/type-utils.h
> index 680b2497a36..4ad0d822119 100644
> --- a/gcc/cp/type-utils.h
> +++ b/gcc/cp/type-utils.h
> @@ -20,36 +20,36 @@ along with GCC; see the file COPYING3.  If not see
>   #ifndef GCC_CP_TYPE_UTILS_H
>   #define GCC_CP_TYPE_UTILS_H
>   
> -/* Returns the first tree within T that is directly matched by PRED.  T may be a
> -   type or PARM_DECL and is incrementally decomposed toward its type-specifier
> -   until a match is found.  NULL_TREE is returned if PRED does not match any
> -   part of T.
> +/* Returns a pointer to the first tree within *TP that is directly matched by
> +   PRED.  *TP may be a type or PARM_DECL and is incrementally decomposed toward
> +   its type-specifier until a match is found.  NULL is returned if PRED does not
> +   match any part of *TP.
>   
> -   This is primarily intended for detecting whether T uses `auto' or a concept
> +   This is primarily intended for detecting whether *TP uses `auto' or a concept
>      identifier.  Since either of these can only appear as a type-specifier for
>      the declaration in question, only top-level qualifications are traversed;
>      find_type_usage does not look through the whole type.  */
>   
> -inline tree
> -find_type_usage (tree t, bool (*pred) (const_tree))
> +inline tree *
> +find_type_usage (tree *tp, bool (*pred) (const_tree))
>   {
> -  enum tree_code code;
> +  tree t = *tp;
>     if (pred (t))
> -    return t;
> +    return tp;
>   
> -  code = TREE_CODE (t);
> +  enum tree_code code = TREE_CODE (t);
>   
>     if (code == POINTER_TYPE || code == REFERENCE_TYPE
>         || code == PARM_DECL || code == OFFSET_TYPE
>         || code == FUNCTION_TYPE || code == METHOD_TYPE
>         || code == ARRAY_TYPE)
> -    return find_type_usage (TREE_TYPE (t), pred);
> +    return find_type_usage (&TREE_TYPE (t), pred);
>   
>     if (TYPE_PTRMEMFUNC_P (t))
>       return find_type_usage
> -      (TREE_TYPE (TYPE_PTRMEMFUNC_FN_TYPE (t)), pred);
> +      (&TREE_TYPE (TYPE_PTRMEMFUNC_FN_TYPE (t)), pred);
>   
> -  return NULL_TREE;
> +  return NULL;
>   }
>   
>   #endif // GCC_CP_TYPE_UTILS_H
> diff --git a/gcc/testsuite/g++.dg/concepts/abbrev3.C b/gcc/testsuite/g++.dg/concepts/abbrev3.C
> new file mode 100644
> index 00000000000..ba2a648156e
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/concepts/abbrev3.C
> @@ -0,0 +1,11 @@
> +// PR c++/69448
> +// { dg-do compile { target c++14 } }
> +// { dg-additional-options "-fconcepts" }
> +
> +long x;
> +
> +auto& f(auto) { return x; }
> +auto* g(auto) { return &x; }
> +
> +long& r = f(1);
> +long* p = g(1);
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr80471.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr80471.C
> index d5fa5a536d3..6ea6164b417 100644
> --- a/gcc/testsuite/g++.dg/cpp2a/concepts-pr80471.C
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr80471.C
> @@ -18,6 +18,6 @@ int main()
>   {
>     int i;
>     static_assert(is_same< decltype(f(i)), int& >, "");
> -  static_assert(is_same< decltype(g(i)), int  >, "");
> +  static_assert(is_same< decltype(g(i)), int& >, "");
>     static_assert(is_same< decltype(z(i)), int& >, "");
>   }
>
Patrick Palka Feb. 10, 2020, 7:21 p.m. UTC | #2
On Mon, 10 Feb 2020, Jason Merrill wrote:

> On 2/10/20 2:20 PM, Patrick Palka wrote:
> > This patch fixes two issues with return type deduction in the presence of an
> > abbreviated function template.
> > 
> > The first issue (PR 69448) is that if a placeholder auto return type
> > contains
> > any modifiers such as & or *, then the abbreviated-function-template
> > compensation in splice_late_return_type does not get performed for the
> > underlying auto node, leading to incorrect return type deduction.  This
> > happens
> > because splice_late_return_type checks for a placeholder auto with is_auto,
> > which does not look through modifiers.  This patch replaces the use of
> > is_auto
> > with find_type_usage, but it first refactors find_type_usage to return a
> > pointer
> > to the matched tree so that we next subsequently replace it.
> > 
> > The second issue (PR 80471) is that the AUTO_IS_DECLTYPE flag is not being
> > preserved in splice_late_return_type when compensating for an abbreviated
> > function template, leading to us treating a decltype(auto) return type as if
> > it
> > was an auto return type.  Fixed by propagating AUTO_IS_DECLTYPE.  The test
> > for
> > PR 80471 is adjusted to expect the correct behavior.
> 
> The comment in make_constrained_decltype_auto suggests that we should set
> AUTO_IS_DECLTYPE in make_auto_1 if name == decltype_auto_identifier; then
> callers won't need to worry about it.

Updated the patch to make_auto_1 to set AUTO_IS_DECLTYPE when
appropriate, and adjusted callers accordingly.  Now make_auto_1 is the
only place where we set AUTO_IS_DECLTYPE.

> 
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, is this OK to commit?
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	PR c++/69448
> > 	PR c++/80471
> > 	* type-utils.h (find_type_usage): Refactor to take a tree * and to
> > 	return a tree *, and update documentation accordingly.
> > 	* pt.c (splice_late_return_type): Use find_type_usage to find and
> > 	replace a possibly nested auto node.  Preserve the AUTO_IS_DECLTYPE
> > 	flag when replacing the node.
> > 	(type_uses_auto): Adjust the call to find_type_usage.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	PR c++/69448
> > 	PR c++/80471
> > 	* g++.dg/concepts/abbrev3.C: New test.
> > 	* g++.dg/cpp2a/concepts-pr80471.C: Adjust a static_assert to expect
> > the
> > 	correct behavior.
> > ---
> >   gcc/cp/pt.c                                   | 26 ++++++++++++-------
> >   gcc/cp/type-utils.h                           | 26 +++++++++----------
> >   gcc/testsuite/g++.dg/concepts/abbrev3.C       | 11 ++++++++
> >   gcc/testsuite/g++.dg/cpp2a/concepts-pr80471.C |  2 +-
> >   4 files changed, 41 insertions(+), 24 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/concepts/abbrev3.C
> > 
> > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > index 2fb52caa5d4..c83aaa7c7e1 100644
> > --- a/gcc/cp/pt.c
> > +++ b/gcc/cp/pt.c
> > @@ -28904,17 +28904,21 @@ do_auto_deduction (tree type, tree init, tree
> > auto_node,
> >   tree
> >   splice_late_return_type (tree type, tree late_return_type)
> >   {
> > -  if (is_auto (type))
> > -    {
> > -      if (late_return_type)
> > -	return late_return_type;
> > +  if (is_auto (type) && late_return_type)
> > +    return late_return_type;
> 
> It occurs to me that we could just check late_return_type here, maybe change
> the is_auto test to an assert.

Done -- I had to add a seen_error test to the assert to accomodate error
recovery and also adjust a dg-error directive in g++.dg/cpp0x/auto9.C.
Does this version look OK after testing?

-- >8 --

gcc/cp/ChangeLog:

	PR c++/69448
	PR c++/80471
	* type-utils.h (find_type_usage): Refactor to take a tree * and to
	return a tree *, and update documentation accordingly.
	* pt.c (make_auto_1): Set AUTO_IS_DECLTYPE when building a
	decltype(auto) node.
	(make_constrained_decltype_auto): No need to explicitly set
	AUTO_IS_DECLTYPE anymore.
	(splice_late_return_type): Use find_type_usage to find and
	replace a possibly nested auto node instead of using is_auto.
	Check test for is_auto into an assert when deciding whether
	to late_return_type.
	(type_uses_auto): Adjust the call to find_type_usage.
	* parser.c (cp_parser_decltype): No need to explicitly set
	AUTO_IS_DECLTYPE anymore.

gcc/libcc1/ChangeLog:

	* libcp1plugin.cc (plugin_get_expr_type): No need to explicitly set
	AUTO_IS_DECLTYPE anymore.

gcc/testsuite/ChangeLog:

	PR c++/69448
	PR c++/80471
	* g++.dg/concepts/abbrev3.C: New test.
	* g++.dg/cpp2a/concepts-pr80471.C: Adjust a static_assert to expect the
	correct behavior.
	* g++.dg/cpp0x/auto9.C: Adjust a dg-error directive.
---
 gcc/cp/parser.c                               |  7 ++---
 gcc/cp/pt.c                                   | 21 +++++++++------
 gcc/cp/type-utils.h                           | 26 +++++++++----------
 gcc/testsuite/g++.dg/concepts/abbrev3.C       | 11 ++++++++
 gcc/testsuite/g++.dg/cpp0x/auto9.C            |  2 +-
 gcc/testsuite/g++.dg/cpp2a/concepts-pr80471.C |  2 +-
 libcc1/libcp1plugin.cc                        |  5 +---
 7 files changed, 42 insertions(+), 32 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/concepts/abbrev3.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index d4c9523289f..640affd8368 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -14901,11 +14901,8 @@ cp_parser_decltype (cp_parser *parser)
     }
 
   if (!expr)
-    {
-      /* Build auto.  */
-      expr = make_decltype_auto ();
-      AUTO_IS_DECLTYPE (expr) = true;
-    }
+    /* Build auto.  */
+    expr = make_decltype_auto ();
   else
     expr = finish_decltype_type (expr, id_expression_or_member_access_p,
 				 tf_warning_or_error);
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 01bade85cdf..e1c832c7035 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -27539,6 +27539,8 @@ make_auto_1 (tree name, bool set_canonical)
     TYPE_CANONICAL (au) = canonical_type_parameter (au);
   DECL_ARTIFICIAL (TYPE_NAME (au)) = 1;
   SET_DECL_TEMPLATE_PARM_P (TYPE_NAME (au));
+  if (name == decltype_auto_identifier)
+    AUTO_IS_DECLTYPE (au) = true;
 
   return au;
 }
@@ -27616,8 +27618,6 @@ tree
 make_constrained_decltype_auto (tree con, tree args)
 {
   tree type = make_auto_1 (decltype_auto_identifier, false);
-  /* FIXME: I don't know why this isn't done in make_auto_1.  */
-  AUTO_IS_DECLTYPE (type) = true;
   return make_constrained_placeholder_type (type, con, args);
 }
 
@@ -28930,17 +28930,20 @@ do_auto_deduction (tree type, tree init, tree auto_node,
 tree
 splice_late_return_type (tree type, tree late_return_type)
 {
-  if (is_auto (type))
+  if (late_return_type)
     {
-      if (late_return_type)
-	return late_return_type;
+      gcc_assert (is_auto (type) || seen_error ());
+      return late_return_type;
+    }
 
-      tree idx = get_template_parm_index (type);
+  if (tree *auto_node = find_type_usage (&type, is_auto))
+    {
+      tree idx = get_template_parm_index (*auto_node);
       if (TEMPLATE_PARM_LEVEL (idx) <= processing_template_decl)
 	/* In an abbreviated function template we didn't know we were dealing
 	   with a function template when we saw the auto return type, so update
 	   it to have the correct level.  */
-	return make_auto_1 (TYPE_IDENTIFIER (type), true);
+	*auto_node = make_auto_1 (TYPE_IDENTIFIER (*auto_node), true);
     }
   return type;
 }
@@ -28986,8 +28989,10 @@ type_uses_auto (tree type)
       else
 	return NULL_TREE;
     }
+  else if (tree *tp = find_type_usage (&type, is_auto))
+    return *tp;
   else
-    return find_type_usage (type, is_auto);
+    return NULL_TREE;
 }
 
 /* Report ill-formed occurrences of auto types in ARGUMENTS.  If
diff --git a/gcc/cp/type-utils.h b/gcc/cp/type-utils.h
index 680b2497a36..4ad0d822119 100644
--- a/gcc/cp/type-utils.h
+++ b/gcc/cp/type-utils.h
@@ -20,36 +20,36 @@ along with GCC; see the file COPYING3.  If not see
 #ifndef GCC_CP_TYPE_UTILS_H
 #define GCC_CP_TYPE_UTILS_H
 
-/* Returns the first tree within T that is directly matched by PRED.  T may be a
-   type or PARM_DECL and is incrementally decomposed toward its type-specifier
-   until a match is found.  NULL_TREE is returned if PRED does not match any
-   part of T.
+/* Returns a pointer to the first tree within *TP that is directly matched by
+   PRED.  *TP may be a type or PARM_DECL and is incrementally decomposed toward
+   its type-specifier until a match is found.  NULL is returned if PRED does not
+   match any part of *TP.
 
-   This is primarily intended for detecting whether T uses `auto' or a concept
+   This is primarily intended for detecting whether *TP uses `auto' or a concept
    identifier.  Since either of these can only appear as a type-specifier for
    the declaration in question, only top-level qualifications are traversed;
    find_type_usage does not look through the whole type.  */
 
-inline tree
-find_type_usage (tree t, bool (*pred) (const_tree))
+inline tree *
+find_type_usage (tree *tp, bool (*pred) (const_tree))
 {
-  enum tree_code code;
+  tree t = *tp;
   if (pred (t))
-    return t;
+    return tp;
 
-  code = TREE_CODE (t);
+  enum tree_code code = TREE_CODE (t);
 
   if (code == POINTER_TYPE || code == REFERENCE_TYPE
       || code == PARM_DECL || code == OFFSET_TYPE
       || code == FUNCTION_TYPE || code == METHOD_TYPE
       || code == ARRAY_TYPE)
-    return find_type_usage (TREE_TYPE (t), pred);
+    return find_type_usage (&TREE_TYPE (t), pred);
 
   if (TYPE_PTRMEMFUNC_P (t))
     return find_type_usage
-      (TREE_TYPE (TYPE_PTRMEMFUNC_FN_TYPE (t)), pred);
+      (&TREE_TYPE (TYPE_PTRMEMFUNC_FN_TYPE (t)), pred);
 
-  return NULL_TREE;
+  return NULL;
 }
 
 #endif // GCC_CP_TYPE_UTILS_H
diff --git a/gcc/testsuite/g++.dg/concepts/abbrev3.C b/gcc/testsuite/g++.dg/concepts/abbrev3.C
new file mode 100644
index 00000000000..ba2a648156e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/abbrev3.C
@@ -0,0 +1,11 @@
+// PR c++/69448
+// { dg-do compile { target c++14 } }
+// { dg-additional-options "-fconcepts" }
+
+long x;
+
+auto& f(auto) { return x; }
+auto* g(auto) { return &x; }
+
+long& r = f(1);
+long* p = g(1);
diff --git a/gcc/testsuite/g++.dg/cpp0x/auto9.C b/gcc/testsuite/g++.dg/cpp0x/auto9.C
index 8d77b0b3667..a3f9be521d6 100644
--- a/gcc/testsuite/g++.dg/cpp0x/auto9.C
+++ b/gcc/testsuite/g++.dg/cpp0x/auto9.C
@@ -22,7 +22,7 @@ struct A
 struct A2
 {
   operator auto () -> int;			// { dg-error "invalid use of|trailing return type" }
-  operator auto*() -> int;			// { dg-error "invalid use of|trailing return type" }
+  operator auto*() -> int;			// { dg-error "invalid use of|trailing return type|cannot be overloaded" }
 };
 
 template <typename> struct B
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr80471.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr80471.C
index d5fa5a536d3..6ea6164b417 100644
--- a/gcc/testsuite/g++.dg/cpp2a/concepts-pr80471.C
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr80471.C
@@ -18,6 +18,6 @@ int main()
 {
   int i;
   static_assert(is_same< decltype(f(i)), int& >, "");
-  static_assert(is_same< decltype(g(i)), int  >, "");
+  static_assert(is_same< decltype(g(i)), int& >, "");
   static_assert(is_same< decltype(z(i)), int& >, "");
 }
diff --git a/libcc1/libcp1plugin.cc b/libcc1/libcp1plugin.cc
index b466b34bee3..00449f43b52 100644
--- a/libcc1/libcp1plugin.cc
+++ b/libcc1/libcp1plugin.cc
@@ -3343,10 +3343,7 @@ plugin_get_expr_type (cc1_plugin::connection *self,
   if (op0)
     type = TREE_TYPE (op0);
   else
-    {
-      type = make_decltype_auto ();
-      AUTO_IS_DECLTYPE (type) = true;
-    }
+    type = make_decltype_auto ();
   return convert_out (ctx->preserve (type));
 }
Jason Merrill Feb. 10, 2020, 7:41 p.m. UTC | #3
On 2/10/20 8:21 PM, Patrick Palka wrote:
> On Mon, 10 Feb 2020, Jason Merrill wrote:
> 
>> On 2/10/20 2:20 PM, Patrick Palka wrote:
>>> This patch fixes two issues with return type deduction in the presence of an
>>> abbreviated function template.
>>>
>>> The first issue (PR 69448) is that if a placeholder auto return type
>>> contains
>>> any modifiers such as & or *, then the abbreviated-function-template
>>> compensation in splice_late_return_type does not get performed for the
>>> underlying auto node, leading to incorrect return type deduction.  This
>>> happens
>>> because splice_late_return_type checks for a placeholder auto with is_auto,
>>> which does not look through modifiers.  This patch replaces the use of
>>> is_auto
>>> with find_type_usage, but it first refactors find_type_usage to return a
>>> pointer
>>> to the matched tree so that we next subsequently replace it.
>>>
>>> The second issue (PR 80471) is that the AUTO_IS_DECLTYPE flag is not being
>>> preserved in splice_late_return_type when compensating for an abbreviated
>>> function template, leading to us treating a decltype(auto) return type as if
>>> it
>>> was an auto return type.  Fixed by propagating AUTO_IS_DECLTYPE.  The test
>>> for
>>> PR 80471 is adjusted to expect the correct behavior.
>>
>> The comment in make_constrained_decltype_auto suggests that we should set
>> AUTO_IS_DECLTYPE in make_auto_1 if name == decltype_auto_identifier; then
>> callers won't need to worry about it.
> 
> Updated the patch to make_auto_1 to set AUTO_IS_DECLTYPE when
> appropriate, and adjusted callers accordingly.  Now make_auto_1 is the
> only place where we set AUTO_IS_DECLTYPE.
> 
>>
>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, is this OK to commit?
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 	PR c++/69448
>>> 	PR c++/80471
>>> 	* type-utils.h (find_type_usage): Refactor to take a tree * and to
>>> 	return a tree *, and update documentation accordingly.
>>> 	* pt.c (splice_late_return_type): Use find_type_usage to find and
>>> 	replace a possibly nested auto node.  Preserve the AUTO_IS_DECLTYPE
>>> 	flag when replacing the node.
>>> 	(type_uses_auto): Adjust the call to find_type_usage.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	PR c++/69448
>>> 	PR c++/80471
>>> 	* g++.dg/concepts/abbrev3.C: New test.
>>> 	* g++.dg/cpp2a/concepts-pr80471.C: Adjust a static_assert to expect
>>> the
>>> 	correct behavior.
>>> ---
>>>    gcc/cp/pt.c                                   | 26 ++++++++++++-------
>>>    gcc/cp/type-utils.h                           | 26 +++++++++----------
>>>    gcc/testsuite/g++.dg/concepts/abbrev3.C       | 11 ++++++++
>>>    gcc/testsuite/g++.dg/cpp2a/concepts-pr80471.C |  2 +-
>>>    4 files changed, 41 insertions(+), 24 deletions(-)
>>>    create mode 100644 gcc/testsuite/g++.dg/concepts/abbrev3.C
>>>
>>> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
>>> index 2fb52caa5d4..c83aaa7c7e1 100644
>>> --- a/gcc/cp/pt.c
>>> +++ b/gcc/cp/pt.c
>>> @@ -28904,17 +28904,21 @@ do_auto_deduction (tree type, tree init, tree
>>> auto_node,
>>>    tree
>>>    splice_late_return_type (tree type, tree late_return_type)
>>>    {
>>> -  if (is_auto (type))
>>> -    {
>>> -      if (late_return_type)
>>> -	return late_return_type;
>>> +  if (is_auto (type) && late_return_type)
>>> +    return late_return_type;
>>
>> It occurs to me that we could just check late_return_type here, maybe change
>> the is_auto test to an assert.
> 
> Done -- I had to add a seen_error test to the assert to accomodate error
> recovery and also adjust a dg-error directive in g++.dg/cpp0x/auto9.C.
> Does this version look OK after testing?

OK, thanks.

Jason
> 
> -- >8 --
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/69448
> 	PR c++/80471
> 	* type-utils.h (find_type_usage): Refactor to take a tree * and to
> 	return a tree *, and update documentation accordingly.
> 	* pt.c (make_auto_1): Set AUTO_IS_DECLTYPE when building a
> 	decltype(auto) node.
> 	(make_constrained_decltype_auto): No need to explicitly set
> 	AUTO_IS_DECLTYPE anymore.
> 	(splice_late_return_type): Use find_type_usage to find and
> 	replace a possibly nested auto node instead of using is_auto.
> 	Check test for is_auto into an assert when deciding whether
> 	to late_return_type.
> 	(type_uses_auto): Adjust the call to find_type_usage.
> 	* parser.c (cp_parser_decltype): No need to explicitly set
> 	AUTO_IS_DECLTYPE anymore.
> 
> gcc/libcc1/ChangeLog:
> 
> 	* libcp1plugin.cc (plugin_get_expr_type): No need to explicitly set
> 	AUTO_IS_DECLTYPE anymore.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/69448
> 	PR c++/80471
> 	* g++.dg/concepts/abbrev3.C: New test.
> 	* g++.dg/cpp2a/concepts-pr80471.C: Adjust a static_assert to expect the
> 	correct behavior.
> 	* g++.dg/cpp0x/auto9.C: Adjust a dg-error directive.
> ---
>   gcc/cp/parser.c                               |  7 ++---
>   gcc/cp/pt.c                                   | 21 +++++++++------
>   gcc/cp/type-utils.h                           | 26 +++++++++----------
>   gcc/testsuite/g++.dg/concepts/abbrev3.C       | 11 ++++++++
>   gcc/testsuite/g++.dg/cpp0x/auto9.C            |  2 +-
>   gcc/testsuite/g++.dg/cpp2a/concepts-pr80471.C |  2 +-
>   libcc1/libcp1plugin.cc                        |  5 +---
>   7 files changed, 42 insertions(+), 32 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/concepts/abbrev3.C
> 
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index d4c9523289f..640affd8368 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -14901,11 +14901,8 @@ cp_parser_decltype (cp_parser *parser)
>       }
>   
>     if (!expr)
> -    {
> -      /* Build auto.  */
> -      expr = make_decltype_auto ();
> -      AUTO_IS_DECLTYPE (expr) = true;
> -    }
> +    /* Build auto.  */
> +    expr = make_decltype_auto ();
>     else
>       expr = finish_decltype_type (expr, id_expression_or_member_access_p,
>   				 tf_warning_or_error);
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index 01bade85cdf..e1c832c7035 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -27539,6 +27539,8 @@ make_auto_1 (tree name, bool set_canonical)
>       TYPE_CANONICAL (au) = canonical_type_parameter (au);
>     DECL_ARTIFICIAL (TYPE_NAME (au)) = 1;
>     SET_DECL_TEMPLATE_PARM_P (TYPE_NAME (au));
> +  if (name == decltype_auto_identifier)
> +    AUTO_IS_DECLTYPE (au) = true;
>   
>     return au;
>   }
> @@ -27616,8 +27618,6 @@ tree
>   make_constrained_decltype_auto (tree con, tree args)
>   {
>     tree type = make_auto_1 (decltype_auto_identifier, false);
> -  /* FIXME: I don't know why this isn't done in make_auto_1.  */
> -  AUTO_IS_DECLTYPE (type) = true;
>     return make_constrained_placeholder_type (type, con, args);
>   }
>   
> @@ -28930,17 +28930,20 @@ do_auto_deduction (tree type, tree init, tree auto_node,
>   tree
>   splice_late_return_type (tree type, tree late_return_type)
>   {
> -  if (is_auto (type))
> +  if (late_return_type)
>       {
> -      if (late_return_type)
> -	return late_return_type;
> +      gcc_assert (is_auto (type) || seen_error ());
> +      return late_return_type;
> +    }
>   
> -      tree idx = get_template_parm_index (type);
> +  if (tree *auto_node = find_type_usage (&type, is_auto))
> +    {
> +      tree idx = get_template_parm_index (*auto_node);
>         if (TEMPLATE_PARM_LEVEL (idx) <= processing_template_decl)
>   	/* In an abbreviated function template we didn't know we were dealing
>   	   with a function template when we saw the auto return type, so update
>   	   it to have the correct level.  */
> -	return make_auto_1 (TYPE_IDENTIFIER (type), true);
> +	*auto_node = make_auto_1 (TYPE_IDENTIFIER (*auto_node), true);
>       }
>     return type;
>   }
> @@ -28986,8 +28989,10 @@ type_uses_auto (tree type)
>         else
>   	return NULL_TREE;
>       }
> +  else if (tree *tp = find_type_usage (&type, is_auto))
> +    return *tp;
>     else
> -    return find_type_usage (type, is_auto);
> +    return NULL_TREE;
>   }
>   
>   /* Report ill-formed occurrences of auto types in ARGUMENTS.  If
> diff --git a/gcc/cp/type-utils.h b/gcc/cp/type-utils.h
> index 680b2497a36..4ad0d822119 100644
> --- a/gcc/cp/type-utils.h
> +++ b/gcc/cp/type-utils.h
> @@ -20,36 +20,36 @@ along with GCC; see the file COPYING3.  If not see
>   #ifndef GCC_CP_TYPE_UTILS_H
>   #define GCC_CP_TYPE_UTILS_H
>   
> -/* Returns the first tree within T that is directly matched by PRED.  T may be a
> -   type or PARM_DECL and is incrementally decomposed toward its type-specifier
> -   until a match is found.  NULL_TREE is returned if PRED does not match any
> -   part of T.
> +/* Returns a pointer to the first tree within *TP that is directly matched by
> +   PRED.  *TP may be a type or PARM_DECL and is incrementally decomposed toward
> +   its type-specifier until a match is found.  NULL is returned if PRED does not
> +   match any part of *TP.
>   
> -   This is primarily intended for detecting whether T uses `auto' or a concept
> +   This is primarily intended for detecting whether *TP uses `auto' or a concept
>      identifier.  Since either of these can only appear as a type-specifier for
>      the declaration in question, only top-level qualifications are traversed;
>      find_type_usage does not look through the whole type.  */
>   
> -inline tree
> -find_type_usage (tree t, bool (*pred) (const_tree))
> +inline tree *
> +find_type_usage (tree *tp, bool (*pred) (const_tree))
>   {
> -  enum tree_code code;
> +  tree t = *tp;
>     if (pred (t))
> -    return t;
> +    return tp;
>   
> -  code = TREE_CODE (t);
> +  enum tree_code code = TREE_CODE (t);
>   
>     if (code == POINTER_TYPE || code == REFERENCE_TYPE
>         || code == PARM_DECL || code == OFFSET_TYPE
>         || code == FUNCTION_TYPE || code == METHOD_TYPE
>         || code == ARRAY_TYPE)
> -    return find_type_usage (TREE_TYPE (t), pred);
> +    return find_type_usage (&TREE_TYPE (t), pred);
>   
>     if (TYPE_PTRMEMFUNC_P (t))
>       return find_type_usage
> -      (TREE_TYPE (TYPE_PTRMEMFUNC_FN_TYPE (t)), pred);
> +      (&TREE_TYPE (TYPE_PTRMEMFUNC_FN_TYPE (t)), pred);
>   
> -  return NULL_TREE;
> +  return NULL;
>   }
>   
>   #endif // GCC_CP_TYPE_UTILS_H
> diff --git a/gcc/testsuite/g++.dg/concepts/abbrev3.C b/gcc/testsuite/g++.dg/concepts/abbrev3.C
> new file mode 100644
> index 00000000000..ba2a648156e
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/concepts/abbrev3.C
> @@ -0,0 +1,11 @@
> +// PR c++/69448
> +// { dg-do compile { target c++14 } }
> +// { dg-additional-options "-fconcepts" }
> +
> +long x;
> +
> +auto& f(auto) { return x; }
> +auto* g(auto) { return &x; }
> +
> +long& r = f(1);
> +long* p = g(1);
> diff --git a/gcc/testsuite/g++.dg/cpp0x/auto9.C b/gcc/testsuite/g++.dg/cpp0x/auto9.C
> index 8d77b0b3667..a3f9be521d6 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/auto9.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/auto9.C
> @@ -22,7 +22,7 @@ struct A
>   struct A2
>   {
>     operator auto () -> int;			// { dg-error "invalid use of|trailing return type" }
> -  operator auto*() -> int;			// { dg-error "invalid use of|trailing return type" }
> +  operator auto*() -> int;			// { dg-error "invalid use of|trailing return type|cannot be overloaded" }
>   };
>   
>   template <typename> struct B
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr80471.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr80471.C
> index d5fa5a536d3..6ea6164b417 100644
> --- a/gcc/testsuite/g++.dg/cpp2a/concepts-pr80471.C
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr80471.C
> @@ -18,6 +18,6 @@ int main()
>   {
>     int i;
>     static_assert(is_same< decltype(f(i)), int& >, "");
> -  static_assert(is_same< decltype(g(i)), int  >, "");
> +  static_assert(is_same< decltype(g(i)), int& >, "");
>     static_assert(is_same< decltype(z(i)), int& >, "");
>   }
> diff --git a/libcc1/libcp1plugin.cc b/libcc1/libcp1plugin.cc
> index b466b34bee3..00449f43b52 100644
> --- a/libcc1/libcp1plugin.cc
> +++ b/libcc1/libcp1plugin.cc
> @@ -3343,10 +3343,7 @@ plugin_get_expr_type (cc1_plugin::connection *self,
>     if (op0)
>       type = TREE_TYPE (op0);
>     else
> -    {
> -      type = make_decltype_auto ();
> -      AUTO_IS_DECLTYPE (type) = true;
> -    }
> +    type = make_decltype_auto ();
>     return convert_out (ctx->preserve (type));
>   }
>   
>
diff mbox series

Patch

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 2fb52caa5d4..c83aaa7c7e1 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -28904,17 +28904,21 @@  do_auto_deduction (tree type, tree init, tree auto_node,
 tree
 splice_late_return_type (tree type, tree late_return_type)
 {
-  if (is_auto (type))
-    {
-      if (late_return_type)
-	return late_return_type;
+  if (is_auto (type) && late_return_type)
+    return late_return_type;
 
-      tree idx = get_template_parm_index (type);
+  if (tree *auto_node = find_type_usage (&type, is_auto))
+    {
+      tree idx = get_template_parm_index (*auto_node);
       if (TEMPLATE_PARM_LEVEL (idx) <= processing_template_decl)
-	/* In an abbreviated function template we didn't know we were dealing
-	   with a function template when we saw the auto return type, so update
-	   it to have the correct level.  */
-	return make_auto_1 (TYPE_IDENTIFIER (type), true);
+	{
+	  /* In an abbreviated function template we didn't know we were dealing
+	     with a function template when we saw the auto return type, so
+	     update it to have the correct level.  */
+	  tree new_node = make_auto_1 (TYPE_IDENTIFIER (*auto_node), true);
+	  AUTO_IS_DECLTYPE (new_node) = AUTO_IS_DECLTYPE (*auto_node);
+	  *auto_node = new_node;
+	}
     }
   return type;
 }
@@ -28960,8 +28964,10 @@  type_uses_auto (tree type)
       else
 	return NULL_TREE;
     }
+  else if (tree *tp = find_type_usage (&type, is_auto))
+    return *tp;
   else
-    return find_type_usage (type, is_auto);
+    return NULL_TREE;
 }
 
 /* Report ill-formed occurrences of auto types in ARGUMENTS.  If
diff --git a/gcc/cp/type-utils.h b/gcc/cp/type-utils.h
index 680b2497a36..4ad0d822119 100644
--- a/gcc/cp/type-utils.h
+++ b/gcc/cp/type-utils.h
@@ -20,36 +20,36 @@  along with GCC; see the file COPYING3.  If not see
 #ifndef GCC_CP_TYPE_UTILS_H
 #define GCC_CP_TYPE_UTILS_H
 
-/* Returns the first tree within T that is directly matched by PRED.  T may be a
-   type or PARM_DECL and is incrementally decomposed toward its type-specifier
-   until a match is found.  NULL_TREE is returned if PRED does not match any
-   part of T.
+/* Returns a pointer to the first tree within *TP that is directly matched by
+   PRED.  *TP may be a type or PARM_DECL and is incrementally decomposed toward
+   its type-specifier until a match is found.  NULL is returned if PRED does not
+   match any part of *TP.
 
-   This is primarily intended for detecting whether T uses `auto' or a concept
+   This is primarily intended for detecting whether *TP uses `auto' or a concept
    identifier.  Since either of these can only appear as a type-specifier for
    the declaration in question, only top-level qualifications are traversed;
    find_type_usage does not look through the whole type.  */
 
-inline tree
-find_type_usage (tree t, bool (*pred) (const_tree))
+inline tree *
+find_type_usage (tree *tp, bool (*pred) (const_tree))
 {
-  enum tree_code code;
+  tree t = *tp;
   if (pred (t))
-    return t;
+    return tp;
 
-  code = TREE_CODE (t);
+  enum tree_code code = TREE_CODE (t);
 
   if (code == POINTER_TYPE || code == REFERENCE_TYPE
       || code == PARM_DECL || code == OFFSET_TYPE
       || code == FUNCTION_TYPE || code == METHOD_TYPE
       || code == ARRAY_TYPE)
-    return find_type_usage (TREE_TYPE (t), pred);
+    return find_type_usage (&TREE_TYPE (t), pred);
 
   if (TYPE_PTRMEMFUNC_P (t))
     return find_type_usage
-      (TREE_TYPE (TYPE_PTRMEMFUNC_FN_TYPE (t)), pred);
+      (&TREE_TYPE (TYPE_PTRMEMFUNC_FN_TYPE (t)), pred);
 
-  return NULL_TREE;
+  return NULL;
 }
 
 #endif // GCC_CP_TYPE_UTILS_H
diff --git a/gcc/testsuite/g++.dg/concepts/abbrev3.C b/gcc/testsuite/g++.dg/concepts/abbrev3.C
new file mode 100644
index 00000000000..ba2a648156e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/abbrev3.C
@@ -0,0 +1,11 @@ 
+// PR c++/69448
+// { dg-do compile { target c++14 } }
+// { dg-additional-options "-fconcepts" }
+
+long x;
+
+auto& f(auto) { return x; }
+auto* g(auto) { return &x; }
+
+long& r = f(1);
+long* p = g(1);
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr80471.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr80471.C
index d5fa5a536d3..6ea6164b417 100644
--- a/gcc/testsuite/g++.dg/cpp2a/concepts-pr80471.C
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr80471.C
@@ -18,6 +18,6 @@  int main()
 {
   int i;
   static_assert(is_same< decltype(f(i)), int& >, "");
-  static_assert(is_same< decltype(g(i)), int  >, "");
+  static_assert(is_same< decltype(g(i)), int& >, "");
   static_assert(is_same< decltype(z(i)), int& >, "");
 }