diff mbox series

c++: improve class NTTP object pretty printing [PR111471]

Message ID 20230919164055.728094-1-ppalka@redhat.com
State Unresolved
Headers
Series c++: improve class NTTP object pretty printing [PR111471] |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Patrick Palka Sept. 19, 2023, 4:40 p.m. UTC 
  Tested on x86_64-pc-linux-gnu, does this look OK for trunk/13?

-- >8 --

1. Move class NTTP object pretty printing to a more general spot in
   the pretty printer.
2. Print the type of an class NTTP object alongside its CONSTRUCTOR
   value, like dump_expr would have done.
3. Don't pretty print const VIEW_CONVERT_EXPR wrapping class NTTPs.

	PR c++/111471

gcc/cp/ChangeLog:

	* cxx-pretty-print.cc (cxx_pretty_printer::expression)
	<case VAR_DECL>: Print the value of a class NTTP object.
	<case VIEW_CONVERT_EXPR>: Strip cosnt VIEW_CONVERT_EXPR
	wrappers for class NTTPs.
	(pp_cxx_template_argument_list): Don't handle the class
	NTTP objects here.

gcc/testsuite/ChangeLog:

	* g++.dg/concepts/diagnostic19.C: New test.
---
 gcc/cp/cxx-pretty-print.cc                   | 19 +++++++++++++++++--
 gcc/testsuite/g++.dg/concepts/diagnostic19.C | 20 ++++++++++++++++++++
 2 files changed, 37 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic19.C

Comments

Jason Merrill Sept. 19, 2023, 5:01 p.m. UTC | #1 
On 9/19/23 12:40, Patrick Palka wrote:
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk/13?

OK for trunk.  What's your argument for backporting?

> -- >8 --
> 
> 1. Move class NTTP object pretty printing to a more general spot in
>     the pretty printer.
> 2. Print the type of an class NTTP object alongside its CONSTRUCTOR
>     value, like dump_expr would have done.
> 3. Don't pretty print const VIEW_CONVERT_EXPR wrapping class NTTPs.
> 
> 	PR c++/111471
> 
> gcc/cp/ChangeLog:
> 
> 	* cxx-pretty-print.cc (cxx_pretty_printer::expression)
> 	<case VAR_DECL>: Print the value of a class NTTP object.
> 	<case VIEW_CONVERT_EXPR>: Strip cosnt VIEW_CONVERT_EXPR
> 	wrappers for class NTTPs.
> 	(pp_cxx_template_argument_list): Don't handle the class
> 	NTTP objects here.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/concepts/diagnostic19.C: New test.
> ---
>   gcc/cp/cxx-pretty-print.cc                   | 19 +++++++++++++++++--
>   gcc/testsuite/g++.dg/concepts/diagnostic19.C | 20 ++++++++++++++++++++
>   2 files changed, 37 insertions(+), 2 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic19.C
> 
> diff --git a/gcc/cp/cxx-pretty-print.cc b/gcc/cp/cxx-pretty-print.cc
> index 909a9dc917f..7cd43151592 100644
> --- a/gcc/cp/cxx-pretty-print.cc
> +++ b/gcc/cp/cxx-pretty-print.cc
> @@ -1121,6 +1121,15 @@ cxx_pretty_printer::expression (tree t)
>         t = OVL_FIRST (t);
>         /* FALLTHRU */
>       case VAR_DECL:
> +      if (DECL_NTTP_OBJECT_P (t))
> +	{
> +	  /* Print the type followed by the CONSTRUCTOR value of an
> +	     NTTP object.  */
> +	  simple_type_specifier (cv_unqualified (TREE_TYPE (t)));
> +	  expression (DECL_INITIAL (t));
> +	  break;
> +	}
> +      /* FALLTHRU */
>       case PARM_DECL:
>       case FIELD_DECL:
>       case CONST_DECL:
> @@ -1261,6 +1270,14 @@ cxx_pretty_printer::expression (tree t)
>         pp_cxx_right_paren (this);
>         break;
>   
> +    case VIEW_CONVERT_EXPR:
> +      if (TREE_CODE (TREE_OPERAND (t, 0)) == TEMPLATE_PARM_INDEX)
> +	{
> +	  /* Strip const VIEW_CONVERT_EXPR wrappers for class NTTPs.  */
> +	  expression (TREE_OPERAND (t, 0));
> +	  break;
> +	}
> +      /* FALLTHRU */
>       default:
>         c_pretty_printer::expression (t);
>         break;
> @@ -1966,8 +1983,6 @@ pp_cxx_template_argument_list (cxx_pretty_printer *pp, tree t)
>   	  if (TYPE_P (arg) || (TREE_CODE (arg) == TEMPLATE_DECL
>   			       && TYPE_P (DECL_TEMPLATE_RESULT (arg))))
>   	    pp->type_id (arg);
> -	  else if (VAR_P (arg) && DECL_NTTP_OBJECT_P (arg))
> -	    pp->expression (DECL_INITIAL (arg));
>   	  else
>   	    pp->expression (arg);
>   	}
> diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic19.C b/gcc/testsuite/g++.dg/concepts/diagnostic19.C
> new file mode 100644
> index 00000000000..20cdb63380b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/concepts/diagnostic19.C
> @@ -0,0 +1,20 @@
> +// Verify our pretty printing of class NTTP objects.
> +// PR c++/111471
> +// { dg-do compile { target c++20 } }
> +
> +struct A { bool value; };
> +
> +template<A V>
> +  requires (V.value) // { dg-message {'\(V\).value \[with V = A\{false\}\]'} }
> +void f();
> +
> +template<A V> struct B { static constexpr auto value = V.value; };
> +
> +template<class T>
> +  requires T::value // { dg-message {'T::value \[with T = B<A\{false\}>\]'} }
> +void g();
> +
> +int main() {
> +  f<A{false}>(); // { dg-error "no match" }
> +  g<B<A{false}>>(); // { dg-error "no match" }
> +}
Patrick Palka Sept. 19, 2023, 5:33 p.m. UTC | #2 
On Tue, 19 Sep 2023, Jason Merrill wrote:

> On 9/19/23 12:40, Patrick Palka wrote:
> > Tested on x86_64-pc-linux-gnu, does this look OK for trunk/13?
> 
> OK for trunk.  What's your argument for backporting?

Thanks.  I don't feel strongly about it, but I was thinking that since
we typically backport C++20-only correctness fixes to the most recent
release branch, C++20-only diagnostic improvements might be suitable
too?

> 
> > -- >8 --
> > 
> > 1. Move class NTTP object pretty printing to a more general spot in
> >     the pretty printer.
> > 2. Print the type of an class NTTP object alongside its CONSTRUCTOR
> >     value, like dump_expr would have done.
> > 3. Don't pretty print const VIEW_CONVERT_EXPR wrapping class NTTPs.
> > 
> > 	PR c++/111471
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* cxx-pretty-print.cc (cxx_pretty_printer::expression)
> > 	<case VAR_DECL>: Print the value of a class NTTP object.
> > 	<case VIEW_CONVERT_EXPR>: Strip cosnt VIEW_CONVERT_EXPR
> > 	wrappers for class NTTPs.
> > 	(pp_cxx_template_argument_list): Don't handle the class
> > 	NTTP objects here.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/concepts/diagnostic19.C: New test.
> > ---
> >   gcc/cp/cxx-pretty-print.cc                   | 19 +++++++++++++++++--
> >   gcc/testsuite/g++.dg/concepts/diagnostic19.C | 20 ++++++++++++++++++++
> >   2 files changed, 37 insertions(+), 2 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic19.C
> > 
> > diff --git a/gcc/cp/cxx-pretty-print.cc b/gcc/cp/cxx-pretty-print.cc
> > index 909a9dc917f..7cd43151592 100644
> > --- a/gcc/cp/cxx-pretty-print.cc
> > +++ b/gcc/cp/cxx-pretty-print.cc
> > @@ -1121,6 +1121,15 @@ cxx_pretty_printer::expression (tree t)
> >         t = OVL_FIRST (t);
> >         /* FALLTHRU */
> >       case VAR_DECL:
> > +      if (DECL_NTTP_OBJECT_P (t))
> > +	{
> > +	  /* Print the type followed by the CONSTRUCTOR value of an
> > +	     NTTP object.  */
> > +	  simple_type_specifier (cv_unqualified (TREE_TYPE (t)));
> > +	  expression (DECL_INITIAL (t));
> > +	  break;
> > +	}
> > +      /* FALLTHRU */
> >       case PARM_DECL:
> >       case FIELD_DECL:
> >       case CONST_DECL:
> > @@ -1261,6 +1270,14 @@ cxx_pretty_printer::expression (tree t)
> >         pp_cxx_right_paren (this);
> >         break;
> >   +    case VIEW_CONVERT_EXPR:
> > +      if (TREE_CODE (TREE_OPERAND (t, 0)) == TEMPLATE_PARM_INDEX)
> > +	{
> > +	  /* Strip const VIEW_CONVERT_EXPR wrappers for class NTTPs.  */
> > +	  expression (TREE_OPERAND (t, 0));
> > +	  break;
> > +	}
> > +      /* FALLTHRU */
> >       default:
> >         c_pretty_printer::expression (t);
> >         break;
> > @@ -1966,8 +1983,6 @@ pp_cxx_template_argument_list (cxx_pretty_printer *pp,
> > tree t)
> >   	  if (TYPE_P (arg) || (TREE_CODE (arg) == TEMPLATE_DECL
> >   			       && TYPE_P (DECL_TEMPLATE_RESULT (arg))))
> >   	    pp->type_id (arg);
> > -	  else if (VAR_P (arg) && DECL_NTTP_OBJECT_P (arg))
> > -	    pp->expression (DECL_INITIAL (arg));
> >   	  else
> >   	    pp->expression (arg);
> >   	}
> > diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic19.C
> > b/gcc/testsuite/g++.dg/concepts/diagnostic19.C
> > new file mode 100644
> > index 00000000000..20cdb63380b
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/concepts/diagnostic19.C
> > @@ -0,0 +1,20 @@
> > +// Verify our pretty printing of class NTTP objects.
> > +// PR c++/111471
> > +// { dg-do compile { target c++20 } }
> > +
> > +struct A { bool value; };
> > +
> > +template<A V>
> > +  requires (V.value) // { dg-message {'\(V\).value \[with V =
> > A\{false\}\]'} }
> > +void f();
> > +
> > +template<A V> struct B { static constexpr auto value = V.value; };
> > +
> > +template<class T>
> > +  requires T::value // { dg-message {'T::value \[with T = B<A\{false\}>\]'}
> > }
> > +void g();
> > +
> > +int main() {
> > +  f<A{false}>(); // { dg-error "no match" }
> > +  g<B<A{false}>>(); // { dg-error "no match" }
> > +}
> 
>
Patrick Palka Sept. 20, 2023, 2:13 p.m. UTC | #3 
On Tue, 19 Sep 2023, Patrick Palka wrote:

> On Tue, 19 Sep 2023, Jason Merrill wrote:
> 
> > On 9/19/23 12:40, Patrick Palka wrote:
> > > Tested on x86_64-pc-linux-gnu, does this look OK for trunk/13?
> > 
> > OK for trunk.  What's your argument for backporting?
> 
> Thanks.  I don't feel strongly about it, but I was thinking that since
> we typically backport C++20-only correctness fixes to the most recent
> release branch, C++20-only diagnostic improvements might be suitable
> too?
> 
> > 
> > > -- >8 --
> > > 
> > > 1. Move class NTTP object pretty printing to a more general spot in
> > >     the pretty printer.

FWIW this first change isn't just a refactoring, it means we now pretty
print an NTTP object that appears elsewhere besides in a template
argument list, e.g. in a parameter mapping:

Before:

diagnostic19.C:8:15: note: the expression ‘((const A)V).value [with V = _ZTAXtl1AEE]’ evaluated to ‘false’

After:

diagnostic19.C:8:15: note: the expression ‘(V).value [with V = A{false}]’ evaluated to ‘false’

> > > 2. Print the type of an class NTTP object alongside its CONSTRUCTOR
> > >     value, like dump_expr would have done.
> > > 3. Don't pretty print const VIEW_CONVERT_EXPR wrapping class NTTPs.
> > > 
> > > 	PR c++/111471
> > > 
> > > gcc/cp/ChangeLog:
> > > 
> > > 	* cxx-pretty-print.cc (cxx_pretty_printer::expression)
> > > 	<case VAR_DECL>: Print the value of a class NTTP object.
> > > 	<case VIEW_CONVERT_EXPR>: Strip cosnt VIEW_CONVERT_EXPR
> > > 	wrappers for class NTTPs.
> > > 	(pp_cxx_template_argument_list): Don't handle the class
> > > 	NTTP objects here.
> > > 
> > > gcc/testsuite/ChangeLog:
> > > 
> > > 	* g++.dg/concepts/diagnostic19.C: New test.
> > > ---
> > >   gcc/cp/cxx-pretty-print.cc                   | 19 +++++++++++++++++--
> > >   gcc/testsuite/g++.dg/concepts/diagnostic19.C | 20 ++++++++++++++++++++
> > >   2 files changed, 37 insertions(+), 2 deletions(-)
> > >   create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic19.C
> > > 
> > > diff --git a/gcc/cp/cxx-pretty-print.cc b/gcc/cp/cxx-pretty-print.cc
> > > index 909a9dc917f..7cd43151592 100644
> > > --- a/gcc/cp/cxx-pretty-print.cc
> > > +++ b/gcc/cp/cxx-pretty-print.cc
> > > @@ -1121,6 +1121,15 @@ cxx_pretty_printer::expression (tree t)
> > >         t = OVL_FIRST (t);
> > >         /* FALLTHRU */
> > >       case VAR_DECL:
> > > +      if (DECL_NTTP_OBJECT_P (t))
> > > +	{
> > > +	  /* Print the type followed by the CONSTRUCTOR value of an
> > > +	     NTTP object.  */
> > > +	  simple_type_specifier (cv_unqualified (TREE_TYPE (t)));
> > > +	  expression (DECL_INITIAL (t));
> > > +	  break;
> > > +	}
> > > +      /* FALLTHRU */
> > >       case PARM_DECL:
> > >       case FIELD_DECL:
> > >       case CONST_DECL:
> > > @@ -1261,6 +1270,14 @@ cxx_pretty_printer::expression (tree t)
> > >         pp_cxx_right_paren (this);
> > >         break;
> > >   +    case VIEW_CONVERT_EXPR:
> > > +      if (TREE_CODE (TREE_OPERAND (t, 0)) == TEMPLATE_PARM_INDEX)
> > > +	{
> > > +	  /* Strip const VIEW_CONVERT_EXPR wrappers for class NTTPs.  */
> > > +	  expression (TREE_OPERAND (t, 0));
> > > +	  break;
> > > +	}
> > > +      /* FALLTHRU */
> > >       default:
> > >         c_pretty_printer::expression (t);
> > >         break;
> > > @@ -1966,8 +1983,6 @@ pp_cxx_template_argument_list (cxx_pretty_printer *pp,
> > > tree t)
> > >   	  if (TYPE_P (arg) || (TREE_CODE (arg) == TEMPLATE_DECL
> > >   			       && TYPE_P (DECL_TEMPLATE_RESULT (arg))))
> > >   	    pp->type_id (arg);
> > > -	  else if (VAR_P (arg) && DECL_NTTP_OBJECT_P (arg))
> > > -	    pp->expression (DECL_INITIAL (arg));
> > >   	  else
> > >   	    pp->expression (arg);
> > >   	}
> > > diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic19.C
> > > b/gcc/testsuite/g++.dg/concepts/diagnostic19.C
> > > new file mode 100644
> > > index 00000000000..20cdb63380b
> > > --- /dev/null
> > > +++ b/gcc/testsuite/g++.dg/concepts/diagnostic19.C
> > > @@ -0,0 +1,20 @@
> > > +// Verify our pretty printing of class NTTP objects.
> > > +// PR c++/111471
> > > +// { dg-do compile { target c++20 } }
> > > +
> > > +struct A { bool value; };
> > > +
> > > +template<A V>
> > > +  requires (V.value) // { dg-message {'\(V\).value \[with V =
> > > A\{false\}\]'} }
> > > +void f();
> > > +
> > > +template<A V> struct B { static constexpr auto value = V.value; };
> > > +
> > > +template<class T>
> > > +  requires T::value // { dg-message {'T::value \[with T = B<A\{false\}>\]'}
> > > }
> > > +void g();
> > > +
> > > +int main() {
> > > +  f<A{false}>(); // { dg-error "no match" }
> > > +  g<B<A{false}>>(); // { dg-error "no match" }
> > > +}
> > 
> > 
>
Jason Merrill Sept. 21, 2023, 3:53 p.m. UTC | #4 
On 9/20/23 10:13, Patrick Palka wrote:
> On Tue, 19 Sep 2023, Patrick Palka wrote:
> 
>> On Tue, 19 Sep 2023, Jason Merrill wrote:
>>
>>> On 9/19/23 12:40, Patrick Palka wrote:
>>>> Tested on x86_64-pc-linux-gnu, does this look OK for trunk/13?
>>>
>>> OK for trunk.  What's your argument for backporting?
>>
>> Thanks.  I don't feel strongly about it, but I was thinking that since
>> we typically backport C++20-only correctness fixes to the most recent
>> release branch, C++20-only diagnostic improvements might be suitable
>> too?
>>
>>>
>>>> -- >8 --
>>>>
>>>> 1. Move class NTTP object pretty printing to a more general spot in
>>>>      the pretty printer.
> 
> FWIW this first change isn't just a refactoring, it means we now pretty
> print an NTTP object that appears elsewhere besides in a template
> argument list, e.g. in a parameter mapping:
> 
> Before:
> 
> diagnostic19.C:8:15: note: the expression ‘((const A)V).value [with V = _ZTAXtl1AEE]’ evaluated to ‘false’
> 
> After:
> 
> diagnostic19.C:8:15: note: the expression ‘(V).value [with V = A{false}]’ evaluated to ‘false’

Ah, that is a pretty big improvement.  The patch is OK.

Jason
Jason Merrill Sept. 22, 2023, 9:18 a.m. UTC | #5 
On 9/21/23 11:53, Jason Merrill wrote:
> On 9/20/23 10:13, Patrick Palka wrote:
>> On Tue, 19 Sep 2023, Patrick Palka wrote:
>>
>>> On Tue, 19 Sep 2023, Jason Merrill wrote:
>>>
>>>> On 9/19/23 12:40, Patrick Palka wrote:
>>>>> Tested on x86_64-pc-linux-gnu, does this look OK for trunk/13?
>>>>
>>>> OK for trunk.  What's your argument for backporting?
>>>
>>> Thanks.  I don't feel strongly about it, but I was thinking that since
>>> we typically backport C++20-only correctness fixes to the most recent
>>> release branch, C++20-only diagnostic improvements might be suitable
>>> too?
>>>
>>>>
>>>>> -- >8 --
>>>>>
>>>>> 1. Move class NTTP object pretty printing to a more general spot in
>>>>>      the pretty printer.
>>
>> FWIW this first change isn't just a refactoring, it means we now pretty
>> print an NTTP object that appears elsewhere besides in a template
>> argument list, e.g. in a parameter mapping:
>>
>> Before:
>>
>> diagnostic19.C:8:15: note: the expression ‘((const A)V).value [with V 
>> = _ZTAXtl1AEE]’ evaluated to ‘false’
>>
>> After:
>>
>> diagnostic19.C:8:15: note: the expression ‘(V).value [with V = 
>> A{false}]’ evaluated to ‘false’
> 
> Ah, that is a pretty big improvement.  The patch is OK.

...for 13 as well.

Jason
diff mbox series

Patch

diff --git a/gcc/cp/cxx-pretty-print.cc b/gcc/cp/cxx-pretty-print.cc
index 909a9dc917f..7cd43151592 100644
--- a/gcc/cp/cxx-pretty-print.cc
+++ b/gcc/cp/cxx-pretty-print.cc
@@ -1121,6 +1121,15 @@  cxx_pretty_printer::expression (tree t)
       t = OVL_FIRST (t);
       /* FALLTHRU */
     case VAR_DECL:
+      if (DECL_NTTP_OBJECT_P (t))
+	{
+	  /* Print the type followed by the CONSTRUCTOR value of an
+	     NTTP object.  */
+	  simple_type_specifier (cv_unqualified (TREE_TYPE (t)));
+	  expression (DECL_INITIAL (t));
+	  break;
+	}
+      /* FALLTHRU */
     case PARM_DECL:
     case FIELD_DECL:
     case CONST_DECL:
@@ -1261,6 +1270,14 @@  cxx_pretty_printer::expression (tree t)
       pp_cxx_right_paren (this);
       break;
 
+    case VIEW_CONVERT_EXPR:
+      if (TREE_CODE (TREE_OPERAND (t, 0)) == TEMPLATE_PARM_INDEX)
+	{
+	  /* Strip const VIEW_CONVERT_EXPR wrappers for class NTTPs.  */
+	  expression (TREE_OPERAND (t, 0));
+	  break;
+	}
+      /* FALLTHRU */
     default:
       c_pretty_printer::expression (t);
       break;
@@ -1966,8 +1983,6 @@  pp_cxx_template_argument_list (cxx_pretty_printer *pp, tree t)
 	  if (TYPE_P (arg) || (TREE_CODE (arg) == TEMPLATE_DECL
 			       && TYPE_P (DECL_TEMPLATE_RESULT (arg))))
 	    pp->type_id (arg);
-	  else if (VAR_P (arg) && DECL_NTTP_OBJECT_P (arg))
-	    pp->expression (DECL_INITIAL (arg));
 	  else
 	    pp->expression (arg);
 	}
diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic19.C b/gcc/testsuite/g++.dg/concepts/diagnostic19.C
new file mode 100644
index 00000000000..20cdb63380b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/diagnostic19.C
@@ -0,0 +1,20 @@ 
+// Verify our pretty printing of class NTTP objects.
+// PR c++/111471
+// { dg-do compile { target c++20 } }
+
+struct A { bool value; };
+
+template<A V>
+  requires (V.value) // { dg-message {'\(V\).value \[with V = A\{false\}\]'} }
+void f();
+
+template<A V> struct B { static constexpr auto value = V.value; };
+
+template<class T>
+  requires T::value // { dg-message {'T::value \[with T = B<A\{false\}>\]'} }
+void g();
+
+int main() {
+  f<A{false}>(); // { dg-error "no match" }
+  g<B<A{false}>>(); // { dg-error "no match" } 
+}