Re: [PATCH] c++/reflection: various diagnostic tweaks

2026-02-11 Thread Jakub Jelinek 
On Wed, Feb 11, 2026 at 11:54:58AM -0500, Marek Polacek wrote:
> > Though, why not just use %qE and make sure to pass a REFLECT_EXPR and
> > ensure it is printed in human readable form (partly kind like
> > display_string_of, but e.g. with ^^ at the start).
> 
> Would it be better if tree_category returned e.g.
> 
>   return G_(" (type)");
> 
> and is then used at the end of the error message like this:
> 
>   error_at (loc, "expected a reflection of ... "
>   "instead of %qE" "%s", t, tree_category (t));
> 
> or is it still bad wrt translation?  %s because I don't want to
> quote the category.  This would produce:
> 
> error: expected a reflection of a function template instead of 'x' (variable)

Maybe.

Yet another option would be a pair of diagnostic messages:
  auto_diagnostic_group d;
  error_at (loc, "expected a reflection of ... ");
  inform (loc, "while %qE is a variable", t);
where the inform could be done in a helper function which various
error_at callers would call to explain stuff.

Jakub

Re: [PATCH] c++/reflection: various diagnostic tweaks

2026-02-11 Thread Marek Polacek 
On Wed, Feb 11, 2026 at 10:04:57AM +0100, Jakub Jelinek wrote:
> On Wed, Feb 11, 2026 at 10:02:03AM +0100, Jakub Jelinek wrote:
> > On Wed, Feb 11, 2026 at 05:56:27PM +0900, Jason Merrill wrote:
> > > On 2/11/26 10:50 AM, Marek Polacek wrote:
> > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > > 
> > > > -- >8 --
> > > > This patch improves various reflection diagnostics as discussed in
> > > > .
> > > > 
> > > > In particular:
> > > > - reword "not usable" diagnostics to say what kind of reflections we
> > > >expected,
> > > > - use the new tree_category to better describe what kind of reflection
> > > >we actually got,
> > > 
> > > Hmm, building an error message from strings like this is generally frowned
> > > on because it is difficult to translate.  Maybe add a %R to cp_printer 
> > > and a
> > > reflection_to_string function to print any kind of reflection?
> > 
> > %R is taken already,
> >%r: if pp_show_color(pp), switch to color identified by const char *.
> >%R: if pp_show_color(pp), reset color.
> 
> Though, why not just use %qE and make sure to pass a REFLECT_EXPR and
> ensure it is printed in human readable form (partly kind like
> display_string_of, but e.g. with ^^ at the start).

Would it be better if tree_category returned e.g.

  return G_(" (type)");

and is then used at the end of the error message like this:

  error_at (loc, "expected a reflection of ... "
"instead of %qE" "%s", t, tree_category (t));

or is it still bad wrt translation?  %s because I don't want to
quote the category.  This would produce:

error: expected a reflection of a function template instead of 'x' (variable)

Marek

Re: [PATCH] c++/reflection: various diagnostic tweaks

2026-02-11 Thread David Malcolm 
On Wed, 2026-02-11 at 10:04 +0100, Jakub Jelinek wrote:
> On Wed, Feb 11, 2026 at 10:02:03AM +0100, Jakub Jelinek wrote:
> > On Wed, Feb 11, 2026 at 05:56:27PM +0900, Jason Merrill wrote:
> > > On 2/11/26 10:50 AM, Marek Polacek wrote:
> > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > > 
> > > > -- >8 --
> > > > This patch improves various reflection diagnostics as discussed
> > > > in
> > > > <
> > > > https://gcc.gnu.org/pipermail/gcc-patches/2025-December/704168.h
> > > > tml>.
> > > > 
> > > > In particular:
> > > > - reword "not usable" diagnostics to say what kind of
> > > > reflections we
> > > >    expected,
> > > > - use the new tree_category to better describe what kind of
> > > > reflection
> > > >    we actually got,
> > > 
> > > Hmm, building an error message from strings like this is
> > > generally frowned
> > > on because it is difficult to translate.  Maybe add a %R to
> > > cp_printer and a
> > > reflection_to_string function to print any kind of reflection?
> > 
> > %R is taken already,
> >    %r: if pp_show_color(pp), switch to color identified by const
> > char *.
> >    %R: if pp_show_color(pp), reset color.

FWIW I wanted to make it easier to add new pretty print codes so I
added 'e' (as in "%qe") in GCC 15 which takes a
  pp_element *
and so you create a subclass of pp_element which implements printing
itself as a vfunc (and then you don't have to find an unused letter)...

> 
> Though, why not just use %qE and make sure to pass a REFLECT_EXPR and
> ensure it is printed in human readable form (partly kind like
> display_string_of, but e.g. with ^^ at the start).

...but Jakub's idea here sounds better.

Dave

Re: [PATCH] c++/reflection: various diagnostic tweaks

2026-02-11 Thread Jakub Jelinek 
On Wed, Feb 11, 2026 at 10:02:03AM +0100, Jakub Jelinek wrote:
> On Wed, Feb 11, 2026 at 05:56:27PM +0900, Jason Merrill wrote:
> > On 2/11/26 10:50 AM, Marek Polacek wrote:
> > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > 
> > > -- >8 --
> > > This patch improves various reflection diagnostics as discussed in
> > > .
> > > 
> > > In particular:
> > > - reword "not usable" diagnostics to say what kind of reflections we
> > >expected,
> > > - use the new tree_category to better describe what kind of reflection
> > >we actually got,
> > 
> > Hmm, building an error message from strings like this is generally frowned
> > on because it is difficult to translate.  Maybe add a %R to cp_printer and a
> > reflection_to_string function to print any kind of reflection?
> 
> %R is taken already,
>%r: if pp_show_color(pp), switch to color identified by const char *.
>%R: if pp_show_color(pp), reset color.

Though, why not just use %qE and make sure to pass a REFLECT_EXPR and
ensure it is printed in human readable form (partly kind like
display_string_of, but e.g. with ^^ at the start).

Jakub

Re: [PATCH] c++/reflection: various diagnostic tweaks

2026-02-11 Thread Jakub Jelinek 
On Wed, Feb 11, 2026 at 05:56:27PM +0900, Jason Merrill wrote:
> On 2/11/26 10:50 AM, Marek Polacek wrote:
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > 
> > -- >8 --
> > This patch improves various reflection diagnostics as discussed in
> > .
> > 
> > In particular:
> > - reword "not usable" diagnostics to say what kind of reflections we
> >expected,
> > - use the new tree_category to better describe what kind of reflection
> >we actually got,
> 
> Hmm, building an error message from strings like this is generally frowned
> on because it is difficult to translate.  Maybe add a %R to cp_printer and a
> reflection_to_string function to print any kind of reflection?

%R is taken already,
   %r: if pp_show_color(pp), switch to color identified by const char *.
   %R: if pp_show_color(pp), reset color.

Jakub

Re: [PATCH] c++/reflection: various diagnostic tweaks

2026-02-11 Thread Jason Merrill 

On 2/11/26 10:50 AM, Marek Polacek wrote:

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
This patch improves various reflection diagnostics as discussed in
.

In particular:
- reword "not usable" diagnostics to say what kind of reflections we
   expected,
- use the new tree_category to better describe what kind of reflection
   we actually got,


Hmm, building an error message from strings like this is generally 
frowned on because it is difficult to translate.  Maybe add a %R to 
cp_printer and a reflection_to_string function to print any kind of 
reflection?



- when we see a missing 'template' keyword, emit a -Wmissing-template-keyword
   warning instead of giving a hard error (this should only happen when the
   code would be valid with the 'template' added.

gcc/cp/ChangeLog:

* cp-tree.h (tree_category): Declare.
* error.cc (tree_category): New.
* parser.cc (cp_parser_splice_specifier): Use OVL_FIRST when checking
for TEMPLATE_DECL.
(cp_parser_splice_type_specifier): Reword an error message.
(cp_parser_splice_expression): Check check_splice_expr earlier.  Reword
error messages.  Turn an error into an assert.  Use
missing_template_diag instead of giving an error about missing template.
(cp_parser_splice_scope_specifier): Reword an error message.
(missing_template_diag): Forward declare.  Drop "enum" in a parameter.
* reflect.cc (check_splice_expr): Print tree_category.

gcc/testsuite/ChangeLog:

* g++.dg/reflect/crash9.C: Adjust dg-error.
* g++.dg/reflect/diag1.C: Likewise.
* g++.dg/reflect/error10.C: Likewise.
* g++.dg/reflect/error12.C: Likewise.
* g++.dg/reflect/error5.C: Likewise.
* g++.dg/reflect/expr3.C: Likewise.
* g++.dg/reflect/member1.C: Likewise.
* g++.dg/reflect/ns2.C: Likewise.  Test more cases.
* g++.dg/reflect/p2996-12.C: Adjust dg-error.
* g++.dg/reflect/splice5.C: Likewise.
* g++.dg/reflect/diag1a.C: New test.
* g++.dg/reflect/diag1b.C: New test.
---
  gcc/cp/cp-tree.h|  1 +
  gcc/cp/error.cc | 31 +
  gcc/cp/parser.cc| 92 ++---
  gcc/cp/reflect.cc   |  2 +-
  gcc/testsuite/g++.dg/reflect/crash9.C   |  4 +-
  gcc/testsuite/g++.dg/reflect/diag1.C|  6 +-
  gcc/testsuite/g++.dg/reflect/diag1a.C   | 24 +++
  gcc/testsuite/g++.dg/reflect/diag1b.C   | 23 +++
  gcc/testsuite/g++.dg/reflect/error10.C  | 12 ++--
  gcc/testsuite/g++.dg/reflect/error12.C  | 20 +++---
  gcc/testsuite/g++.dg/reflect/error5.C   |  2 +-
  gcc/testsuite/g++.dg/reflect/expr3.C| 18 ++---
  gcc/testsuite/g++.dg/reflect/member1.C  | 12 ++--
  gcc/testsuite/g++.dg/reflect/ns2.C  | 18 -
  gcc/testsuite/g++.dg/reflect/p2996-12.C |  2 +-
  gcc/testsuite/g++.dg/reflect/splice5.C  |  8 +--
  16 files changed, 171 insertions(+), 104 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/reflect/diag1a.C
  create mode 100644 gcc/testsuite/g++.dg/reflect/diag1b.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index fcad67a662c..b1ac66cb4e9 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7688,6 +7688,7 @@ extern bool pedwarn_cxx98   
(location_t,
  extern location_t location_of   (tree);
  extern void qualified_name_lookup_error   (tree, tree, tree,
 location_t);
+extern const char *tree_category   (tree) ATTRIBUTE_PURE;
  
  struct decl_location_traits

: simple_cache_map_traits { };
diff --git a/gcc/cp/error.cc b/gcc/cp/error.cc
index c184846066e..b9ae87faed4 100644
--- a/gcc/cp/error.cc
+++ b/gcc/cp/error.cc
@@ -3976,6 +3976,37 @@ function_category (tree fn)
  return G_("In function %qD");
  }
  
+/* Return the category for T (type, expression, ...).  */

+
+const char *
+tree_category (tree t)
+{
+  t = maybe_get_first_fn (t);
+  if (TREE_CODE (t) == TYPE_DECL)
+t = TREE_TYPE (t);
+
+  if (TYPE_P (t))
+return G_("type ");
+  else if (EXPR_P (t))
+return G_("expression ");
+  else if (VAR_P (t))
+return G_("variable ");
+  else if (TREE_CODE (t) == PARM_DECL)
+return G_("parameter ");
+  else if (TREE_CODE (t) == FUNCTION_DECL)
+return G_("function ");
+  else if (DECL_FUNCTION_TEMPLATE_P (t))
+return G_("function template ");
+  else if (DECL_CLASS_TEMPLATE_P (t))
+return G_("class template ");
+  else if (TREE_CODE (t) == NAMESPACE_DECL)
+return G_("namespace ");
+  else if (TREE_CODE (t) == CONST_DECL && !DECL_TEMPLATE_PARM_P (t))
+return G_("enumerator ");
+  else
+return "";
+}
+
  /* Disable warnings about missing quoting in GCC diagnostics for
 the pp_verbatim calls.  Their format strings deliberately don't
 follow GCC diagnostic

[PATCH] c++/reflection: various diagnostic tweaks

2026-02-10 Thread Marek Polacek 
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
This patch improves various reflection diagnostics as discussed in
.

In particular:
- reword "not usable" diagnostics to say what kind of reflections we
  expected,
- use the new tree_category to better describe what kind of reflection
  we actually got,
- when we see a missing 'template' keyword, emit a -Wmissing-template-keyword
  warning instead of giving a hard error (this should only happen when the
  code would be valid with the 'template' added.

gcc/cp/ChangeLog:

* cp-tree.h (tree_category): Declare.
* error.cc (tree_category): New.
* parser.cc (cp_parser_splice_specifier): Use OVL_FIRST when checking
for TEMPLATE_DECL.
(cp_parser_splice_type_specifier): Reword an error message.
(cp_parser_splice_expression): Check check_splice_expr earlier.  Reword
error messages.  Turn an error into an assert.  Use
missing_template_diag instead of giving an error about missing template.
(cp_parser_splice_scope_specifier): Reword an error message.
(missing_template_diag): Forward declare.  Drop "enum" in a parameter.
* reflect.cc (check_splice_expr): Print tree_category.

gcc/testsuite/ChangeLog:

* g++.dg/reflect/crash9.C: Adjust dg-error.
* g++.dg/reflect/diag1.C: Likewise.
* g++.dg/reflect/error10.C: Likewise.
* g++.dg/reflect/error12.C: Likewise.
* g++.dg/reflect/error5.C: Likewise.
* g++.dg/reflect/expr3.C: Likewise.
* g++.dg/reflect/member1.C: Likewise.
* g++.dg/reflect/ns2.C: Likewise.  Test more cases.
* g++.dg/reflect/p2996-12.C: Adjust dg-error.
* g++.dg/reflect/splice5.C: Likewise.
* g++.dg/reflect/diag1a.C: New test.
* g++.dg/reflect/diag1b.C: New test.
---
 gcc/cp/cp-tree.h|  1 +
 gcc/cp/error.cc | 31 +
 gcc/cp/parser.cc| 92 ++---
 gcc/cp/reflect.cc   |  2 +-
 gcc/testsuite/g++.dg/reflect/crash9.C   |  4 +-
 gcc/testsuite/g++.dg/reflect/diag1.C|  6 +-
 gcc/testsuite/g++.dg/reflect/diag1a.C   | 24 +++
 gcc/testsuite/g++.dg/reflect/diag1b.C   | 23 +++
 gcc/testsuite/g++.dg/reflect/error10.C  | 12 ++--
 gcc/testsuite/g++.dg/reflect/error12.C  | 20 +++---
 gcc/testsuite/g++.dg/reflect/error5.C   |  2 +-
 gcc/testsuite/g++.dg/reflect/expr3.C| 18 ++---
 gcc/testsuite/g++.dg/reflect/member1.C  | 12 ++--
 gcc/testsuite/g++.dg/reflect/ns2.C  | 18 -
 gcc/testsuite/g++.dg/reflect/p2996-12.C |  2 +-
 gcc/testsuite/g++.dg/reflect/splice5.C  |  8 +--
 16 files changed, 171 insertions(+), 104 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/reflect/diag1a.C
 create mode 100644 gcc/testsuite/g++.dg/reflect/diag1b.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index fcad67a662c..b1ac66cb4e9 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7688,6 +7688,7 @@ extern bool pedwarn_cxx98   
(location_t,
 extern location_t location_of   (tree);
 extern void qualified_name_lookup_error(tree, tree, tree,
 location_t);
+extern const char *tree_category   (tree) ATTRIBUTE_PURE;
 
 struct decl_location_traits
   : simple_cache_map_traits { };
diff --git a/gcc/cp/error.cc b/gcc/cp/error.cc
index c184846066e..b9ae87faed4 100644
--- a/gcc/cp/error.cc
+++ b/gcc/cp/error.cc
@@ -3976,6 +3976,37 @@ function_category (tree fn)
 return G_("In function %qD");
 }
 
+/* Return the category for T (type, expression, ...).  */
+
+const char *
+tree_category (tree t)
+{
+  t = maybe_get_first_fn (t);
+  if (TREE_CODE (t) == TYPE_DECL)
+t = TREE_TYPE (t);
+
+  if (TYPE_P (t))
+return G_("type ");
+  else if (EXPR_P (t))
+return G_("expression ");
+  else if (VAR_P (t))
+return G_("variable ");
+  else if (TREE_CODE (t) == PARM_DECL)
+return G_("parameter ");
+  else if (TREE_CODE (t) == FUNCTION_DECL)
+return G_("function ");
+  else if (DECL_FUNCTION_TEMPLATE_P (t))
+return G_("function template ");
+  else if (DECL_CLASS_TEMPLATE_P (t))
+return G_("class template ");
+  else if (TREE_CODE (t) == NAMESPACE_DECL)
+return G_("namespace ");
+  else if (TREE_CODE (t) == CONST_DECL && !DECL_TEMPLATE_PARM_P (t))
+return G_("enumerator ");
+  else
+return "";
+}
+
 /* Disable warnings about missing quoting in GCC diagnostics for
the pp_verbatim calls.  Their format strings deliberately don't
follow GCC diagnostic conventions.  */
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 8c46b260fff..0dc1d361474 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -279,6 +279,8 @@ static tree omp_start_variant_function
 static int omp_finish_variant_function
   (cp_parser *, tree, tree, tree, bool, bool);
 sta