Re: [PATCH RFC] c++: streamline process for adding new builtin trait

[Patrick Palka <ppalka@redhat.com>]

On Thu, 29 Sep 2022, Marek Polacek wrote:

> On Thu, Sep 29, 2022 at 11:05:04AM -0400, Patrick Palka via Gcc-patches wrote:
> > Adding a new builtin trait currently involves some boilerplate (as can
> > be seen in r13-2956-g9ca147154074a0) of defining corresponding RID_ and
> > CPTK_ enumerators and adding them to various switch statements across
> > many files.  The exact switch statements we need to change is determined
> > by whether the proposed trait yields a type or an expression.
> > 
> > This RFC patch attempts to streamline this process via a centralized
> > cp-trait.def file for declaring the important parts about a builtin trait
> > (whether it yields a type or an expression, its code, its spelling and
> > its arity) and using this file to automate away the switch statement
> > addition boilerplate.  It also converts 9 traits to use this approach
> > by way of example (we can convert all the traits once the design is
> > settled).
> > 
> > After this change, the process of adding a new builtin trait is just
> > (modulo tests): declare it in cp-trait.def, define its behavior in
> > finish_trait_type/expr, and handle it in diagnose_trait_expr if it's
> > an expression-yielding trait (this last step is unfortunate but since
> > the switch has no default case, we'll at least get a diagnostic if we
> > forget to do it).
> > 
> > Does this look like a good approach?
> 
> I think it'd be fantastic to have this.  It's been very easy to forget
> to update pp_cxx_trait, or names_builtin_p.  cp-trait.def just needs to
> describe what the arguments mean.

Here's v2 which documents cp-trait.def and factors out its macro
definitions into cp-trait-head.h and cp-trait-tail.h:

-- >8 --

gcc/c-family/ChangeLog:

	* c-common.cc (c_common_reswords): Use cp/cp-trait.def
	to handle C++ traits.
	* c-common.h (enum rid): Likewise.

gcc/cp/ChangeLog:

	* constraint.cc (diagnose_trait_expr): Likewise.
	* cp-objcp-common.cc (names_builtin_p): Likewise.
	* cp-tree.h (enum cp_trait_kind): Likewise.
	* cxx-pretty-print (pp_cxx_trait): Likewise.
	* parser.cc (cp_keyword_starts_decl_specifier_p): Likewise.
	(cp_parser_primary_expression): Likewise.
	(cp_parser_trait): Likewise.
	(cp_parser_simple_type_specifier): Likewise.
	* cp-trait-head.h: New file.
	* cp-trait-tail.h: New file.
	* cp-trait.def: New file.
---
 gcc/c-family/c-common.cc   | 13 +++-----
 gcc/c-family/c-common.h    |  8 ++---
 gcc/cp/constraint.cc       |  7 ++--
 gcc/cp/cp-objcp-common.cc  | 13 +++-----
 gcc/cp/cp-trait-head.h     | 48 +++++++++++++++++++++++++++
 gcc/cp/cp-trait-tail.h     | 30 +++++++++++++++++
 gcc/cp/cp-trait.def        | 45 +++++++++++++++++++++++++
 gcc/cp/cp-tree.h           | 13 +++-----
 gcc/cp/cxx-pretty-print.cc | 31 +++---------------
 gcc/cp/parser.cc           | 67 ++++++++++++--------------------------
 10 files changed, 168 insertions(+), 107 deletions(-)
 create mode 100644 gcc/cp/cp-trait-head.h
 create mode 100644 gcc/cp/cp-trait-tail.h
 create mode 100644 gcc/cp/cp-trait.def

diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
index 6e0af863a49..1b2fd37c583 100644
--- a/gcc/c-family/c-common.cc
+++ b/gcc/c-family/c-common.cc
@@ -537,19 +537,14 @@ const struct c_common_resword c_common_reswords[] =
   { "volatile",		RID_VOLATILE,	0 },
   { "wchar_t",		RID_WCHAR,	D_CXXONLY },
   { "while",		RID_WHILE,	0 },
-  { "__is_assignable", RID_IS_ASSIGNABLE, D_CXXONLY },
-  { "__is_constructible", RID_IS_CONSTRUCTIBLE, D_CXXONLY },
-  { "__is_nothrow_assignable", RID_IS_NOTHROW_ASSIGNABLE, D_CXXONLY },
-  { "__is_nothrow_constructible", RID_IS_NOTHROW_CONSTRUCTIBLE, D_CXXONLY },
-  { "__is_convertible", RID_IS_CONVERTIBLE, D_CXXONLY },
-  { "__is_nothrow_convertible", RID_IS_NOTHROW_CONVERTIBLE, D_CXXONLY },
   { "__reference_constructs_from_temporary", RID_REF_CONSTRUCTS_FROM_TEMPORARY,
 					D_CXXONLY },
   { "__reference_converts_from_temporary", RID_REF_CONVERTS_FROM_TEMPORARY,
 					D_CXXONLY },
-  { "__remove_cv", RID_REMOVE_CV, D_CXXONLY },
-  { "__remove_reference", RID_REMOVE_REFERENCE, D_CXXONLY },
-  { "__remove_cvref", RID_REMOVE_CVREF, D_CXXONLY },
+#define DEFTRAIT(TCC, CODE, NAME, ARITY) \
+  { NAME, RID_##CODE, D_CXXONLY },
+#include "cp/cp-trait.def"
+#undef DEFTRAIT
 
   /* C++ transactional memory.  */
   { "synchronized",	RID_SYNCHRONIZED, D_CXX_OBJC | D_TRANSMEM },
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index d5c98d306ce..b306815c23b 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -182,12 +182,12 @@ enum rid
   RID_IS_TRIVIALLY_ASSIGNABLE, RID_IS_TRIVIALLY_CONSTRUCTIBLE,
   RID_IS_TRIVIALLY_COPYABLE,
   RID_IS_UNION,                RID_UNDERLYING_TYPE,
-  RID_IS_ASSIGNABLE,           RID_IS_CONSTRUCTIBLE,
-  RID_IS_NOTHROW_ASSIGNABLE,   RID_IS_NOTHROW_CONSTRUCTIBLE,
-  RID_IS_CONVERTIBLE,		RID_IS_NOTHROW_CONVERTIBLE,
   RID_REF_CONSTRUCTS_FROM_TEMPORARY,
   RID_REF_CONVERTS_FROM_TEMPORARY,
-  RID_REMOVE_CV, RID_REMOVE_REFERENCE, RID_REMOVE_CVREF,
+#define DEFTRAIT(TCC, CODE, NAME, ARITY) \
+  RID_##CODE,
+#include "cp/cp-trait.def"
+#undef DEFTRAIT
 
   /* C++11 */
   RID_CONSTEXPR, RID_DECLTYPE, RID_NOEXCEPT, RID_NULLPTR, RID_STATIC_ASSERT,
diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index ca73aff3f38..9323bb091e1 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -3714,9 +3714,10 @@ diagnose_trait_expr (tree expr, tree args)
     case CPTK_BASES:
     case CPTK_DIRECT_BASES:
     case CPTK_UNDERLYING_TYPE:
-    case CPTK_REMOVE_CV:
-    case CPTK_REMOVE_REFERENCE:
-    case CPTK_REMOVE_CVREF:
+#define DEFTRAIT_TYPE(CODE, NAME, ARITY) \
+    case CPTK_##CODE:
+#include "cp/cp-trait.def"
+#undef DEFTRAIT_TYPE
       /* We shouldn't see these non-expression traits.  */
       gcc_unreachable ();
     /* We deliberately omit the default case so that when adding a new
diff --git a/gcc/cp/cp-objcp-common.cc b/gcc/cp/cp-objcp-common.cc
index 2d3f206b530..bc397a5447f 100644
--- a/gcc/cp/cp-objcp-common.cc
+++ b/gcc/cp/cp-objcp-common.cc
@@ -458,18 +458,13 @@ names_builtin_p (const char *name)
     case RID_IS_TRIVIALLY_CONSTRUCTIBLE:
     case RID_IS_TRIVIALLY_COPYABLE:
     case RID_IS_UNION:
-    case RID_IS_ASSIGNABLE:
-    case RID_IS_CONSTRUCTIBLE:
-    case RID_IS_NOTHROW_ASSIGNABLE:
-    case RID_IS_NOTHROW_CONSTRUCTIBLE:
     case RID_UNDERLYING_TYPE:
-    case RID_IS_CONVERTIBLE:
-    case RID_IS_NOTHROW_CONVERTIBLE:
     case RID_REF_CONSTRUCTS_FROM_TEMPORARY:
     case RID_REF_CONVERTS_FROM_TEMPORARY:
-    case RID_REMOVE_CV:
-    case RID_REMOVE_REFERENCE:
-    case RID_REMOVE_CVREF:
+#define DEFTRAIT(TCC, CODE, NAME, ARITY) \
+    case RID_##CODE:
+#include "cp/cp-trait.def"
+#undef DEFTRAIT
       return true;
     default:
       break;
diff --git a/gcc/cp/cp-trait-head.h b/gcc/cp/cp-trait-head.h
new file mode 100644
index 00000000000..5cec25af041
--- /dev/null
+++ b/gcc/cp/cp-trait-head.h
@@ -0,0 +1,48 @@
+/* Convenience macro definitions for programmatically tabulating over traits
+   defined in cp-trait.def.  Used only by cp-trait.def.
+
+   Copyright The GNU Toolchain Authors.
+
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3, or (at your option)
+   any later version.
+
+   GCC is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   <http://www.gnu.org/licenses/>.  */
+
+/* Define DEFTRAIT(TCC, CODE, NAME, ARITY) and include cp-trait.def to
+   tabulate over all traits defined in cp-trait.def.  The first argument
+   will be tcc_expression for expression-yielding traits and tcc_type for
+   type-yielding traits.  */
+
+#ifdef DEFTRAIT
+#define DEFTRAIT_EXPR(CODE, NAME, ARITY) DEFTRAIT(tcc_expression, CODE, NAME, ARITY)
+#define DEFTRAIT_TYPE(CODE, NAME, ARITY) DEFTRAIT(tcc_type, CODE, NAME, ARITY)
+#define DEFTRAIT_EXPR_DEFAULTED
+#define DEFTRAIT_TYPE_DEFAULTED
+#endif
+
+/* Define DEFTRAIT_EXPR(CODE, NAME, ARITY) and include cp-trait.def to
+   tabulate over all expression-yielding traits.  */
+
+#ifndef DEFTRAIT_EXPR
+#define DEFTRAIT_EXPR(CODE, NAME, ARITY)
+#define DEFTRAIT_EXPR_DEFAULTED
+#endif
+
+/* Define DEFTRAIT_TYPE(CODE, NAME, ARITY) and include cp-trait.def to
+   tabulate over all type-yielding traits.  */
+
+#ifndef DEFTRAIT_TYPE
+#define DEFTRAIT_TYPE(CODE, NAME, ARITY)
+#define DEFTRAIT_TYPE_DEFAULTED
+#endif
diff --git a/gcc/cp/cp-trait-tail.h b/gcc/cp/cp-trait-tail.h
new file mode 100644
index 00000000000..cc8425119b2
--- /dev/null
+++ b/gcc/cp/cp-trait-tail.h
@@ -0,0 +1,30 @@
+/* Undefines the macros that may have been implicitly defined in
+   cp-trait-head.h.  Used only by cp-trait.def.
+
+   Copyright The GNU Toolchain Authors.
+
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3, or (at your option)
+   any later version.
+
+   GCC is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifdef DEFTRAIT_EXPR_DEFAULTED
+#undef DEFTRAIT_EXPR
+#undef DEFTRAIT_EXPR_DEFAULTED
+#endif
+
+#ifdef DEFTRAIT_TYPE_DEFAULTED
+#undef DEFTRAIT_TYPE
+#undef DEFTRAIT_TYPE_DEFAULTED
+#endif
diff --git a/gcc/cp/cp-trait.def b/gcc/cp/cp-trait.def
new file mode 100644
index 00000000000..61a0bbfdfa5
--- /dev/null
+++ b/gcc/cp/cp-trait.def
@@ -0,0 +1,45 @@
+/* This file contains the definitions for C++-specific builtin traits.
+
+   Copyright The GNU Toolchain Authors.
+
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3, or (at your option)
+   any later version.
+
+   GCC is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   <http://www.gnu.org/licenses/>.  */
+
+#include "cp-trait-head.h"
+
+/* Add a DEFTRAIT_EXPR (CODE, NAME, N) line to this file to define an
+   expression-yielding builtin trait with internal code name CODE, spelled
+   as NAME and takes exactly N type arguments (where N is either 1, 2, or
+   the special value -1 which denotes that it takes at least one argument).
+   Such traits are represented as TRAIT_EXPR tree whose TRAIT_EXPR_KIND is
+   CPTK_NAME.  */
+
+/* Add a DEFTRAIT_TYPE (CODE, NAME, N) line to this file to define a
+   type-yielding trait in the same manner as described above.  Such traits
+   are represented as a TRAIT_TYPE tree whose TRAIT_TYPE_KIND is CPTK_NAME.  */
+
+DEFTRAIT_EXPR (IS_ASSIGNABLE, "__is_assignable", 2)
+DEFTRAIT_EXPR (IS_NOTHROW_ASSIGNABLE, "__is_nothrow_assignable", 2)
+DEFTRAIT_EXPR (IS_CONSTRUCTIBLE, "__is_constructible", -1)
+DEFTRAIT_EXPR (IS_NOTHROW_CONSTRUCTIBLE, "__is_nothrow_constructible", -1)
+DEFTRAIT_EXPR (IS_CONVERTIBLE, "__is_convertible", 2)
+DEFTRAIT_EXPR (IS_NOTHROW_CONVERTIBLE, "__is_nothrow_convertible", 2)
+
+DEFTRAIT_TYPE (REMOVE_CV, "__remove_cv", 1)
+DEFTRAIT_TYPE (REMOVE_REFERENCE, "__remove_reference", 1)
+DEFTRAIT_TYPE (REMOVE_CVREF, "__remove_cvref", 1)
+
+#include "cp-trait-tail.h"
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 3cbcdf726ca..8c5a85ab5fe 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -1405,17 +1405,12 @@ enum cp_trait_kind
   CPTK_IS_TRIVIALLY_COPYABLE,
   CPTK_IS_UNION,
   CPTK_UNDERLYING_TYPE,
-  CPTK_IS_ASSIGNABLE,
-  CPTK_IS_CONSTRUCTIBLE,
-  CPTK_IS_NOTHROW_ASSIGNABLE,
-  CPTK_IS_NOTHROW_CONSTRUCTIBLE,
-  CPTK_IS_CONVERTIBLE,
-  CPTK_IS_NOTHROW_CONVERTIBLE,
   CPTK_REF_CONSTRUCTS_FROM_TEMPORARY,
   CPTK_REF_CONVERTS_FROM_TEMPORARY,
-  CPTK_REMOVE_CV,
-  CPTK_REMOVE_REFERENCE,
-  CPTK_REMOVE_CVREF,
+#define DEFTRAIT(TCC, CODE, NAME, ARITY) \
+  CPTK_##CODE,
+#include "cp/cp-trait.def"
+#undef DEFTRAIT
 };
 
 /* The types that we are processing.  */
diff --git a/gcc/cp/cxx-pretty-print.cc b/gcc/cp/cxx-pretty-print.cc
index b91615439e4..a2ddbcb899a 100644
--- a/gcc/cp/cxx-pretty-print.cc
+++ b/gcc/cp/cxx-pretty-print.cc
@@ -2701,24 +2701,6 @@ pp_cxx_trait (cxx_pretty_printer *pp, tree t)
     case CPTK_IS_LITERAL_TYPE:
       pp_cxx_ws_string (pp, "__is_literal_type");
       break;
-    case CPTK_IS_ASSIGNABLE:
-      pp_cxx_ws_string (pp, "__is_assignable");
-      break;
-    case CPTK_IS_CONSTRUCTIBLE:
-      pp_cxx_ws_string (pp, "__is_constructible");
-      break;
-    case CPTK_IS_NOTHROW_ASSIGNABLE:
-      pp_cxx_ws_string (pp, "__is_nothrow_assignable");
-      break;
-    case CPTK_IS_NOTHROW_CONSTRUCTIBLE:
-      pp_cxx_ws_string (pp, "__is_nothrow_constructible");
-      break;
-    case CPTK_IS_CONVERTIBLE:
-      pp_cxx_ws_string (pp, "__is_convertible");
-      break;
-    case CPTK_IS_NOTHROW_CONVERTIBLE:
-      pp_cxx_ws_string (pp, "__is_nothrow_convertible");
-      break;
     case CPTK_REF_CONSTRUCTS_FROM_TEMPORARY:
       pp_cxx_ws_string (pp, "__reference_constructs_from_temporary");
       break;
@@ -2728,15 +2710,12 @@ pp_cxx_trait (cxx_pretty_printer *pp, tree t)
     case CPTK_UNDERLYING_TYPE:
       pp_cxx_ws_string (pp, "__underlying_type");
       break;
-    case CPTK_REMOVE_CV:
-      pp_cxx_ws_string (pp, "__remove_cv");
-      break;
-    case CPTK_REMOVE_REFERENCE:
-      pp_cxx_ws_string (pp, "__remove_reference");
-      break;
-    case CPTK_REMOVE_CVREF:
-      pp_cxx_ws_string (pp, "__remove_cvref");
+#define DEFTRAIT(TCC, CODE, NAME, ARITY) \
+    case CPTK_##CODE:			 \
+      pp_cxx_ws_string (pp, NAME);	 \
       break;
+#include "cp/cp-trait.def"
+#undef DEFTRAIT
     default:
       gcc_unreachable ();
     }
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index d592d783250..138892716b4 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -1147,9 +1147,10 @@ cp_keyword_starts_decl_specifier_p (enum rid keyword)
       /* C++11 extensions.  */
     case RID_DECLTYPE:
     case RID_UNDERLYING_TYPE:
-    case RID_REMOVE_CV:
-    case RID_REMOVE_REFERENCE:
-    case RID_REMOVE_CVREF:
+#define DEFTRAIT_TYPE(CODE, NAME, ARITY) \
+    case RID_##CODE:
+#include "cp/cp-trait.def"
+#undef DEFTRAIT_TYPE
     case RID_CONSTEXPR:
       /* C++20 extensions.  */
     case RID_CONSTINIT:
@@ -5923,14 +5924,12 @@ cp_parser_primary_expression (cp_parser *parser,
 	case RID_IS_TRIVIALLY_CONSTRUCTIBLE:
 	case RID_IS_TRIVIALLY_COPYABLE:
 	case RID_IS_UNION:
-	case RID_IS_ASSIGNABLE:
-	case RID_IS_CONSTRUCTIBLE:
-	case RID_IS_NOTHROW_ASSIGNABLE:
-	case RID_IS_NOTHROW_CONSTRUCTIBLE:
-	case RID_IS_CONVERTIBLE:
-	case RID_IS_NOTHROW_CONVERTIBLE:
 	case RID_REF_CONSTRUCTS_FROM_TEMPORARY:
 	case RID_REF_CONVERTS_FROM_TEMPORARY:
+#define DEFTRAIT_EXPR(CODE, NAME, ARITY) \
+	case RID_##CODE:
+#include "cp/cp-trait.def"
+#undef DEFTRAIT_EXPR
 	  return cp_parser_trait (parser, token->keyword);
 
 	// C++ concepts
@@ -10998,30 +10997,6 @@ cp_parser_trait (cp_parser* parser, enum rid keyword)
     case RID_DIRECT_BASES:
       kind = CPTK_DIRECT_BASES;
       break;
-    case RID_IS_ASSIGNABLE:
-      kind = CPTK_IS_ASSIGNABLE;
-      binary = true;
-      break;
-    case RID_IS_CONSTRUCTIBLE:
-      kind = CPTK_IS_CONSTRUCTIBLE;
-      variadic = true;
-      break;
-    case RID_IS_NOTHROW_ASSIGNABLE:
-      kind = CPTK_IS_NOTHROW_ASSIGNABLE;
-      binary = true;
-      break;
-    case RID_IS_NOTHROW_CONSTRUCTIBLE:
-      kind = CPTK_IS_NOTHROW_CONSTRUCTIBLE;
-      variadic = true;
-      break;
-    case RID_IS_CONVERTIBLE:
-      kind = CPTK_IS_CONVERTIBLE;
-      binary = true;
-      break;
-    case RID_IS_NOTHROW_CONVERTIBLE:
-      kind = CPTK_IS_NOTHROW_CONVERTIBLE;
-      binary = true;
-      break;
     case RID_REF_CONSTRUCTS_FROM_TEMPORARY:
       kind = CPTK_REF_CONSTRUCTS_FROM_TEMPORARY;
       binary = true;
@@ -11030,18 +11005,15 @@ cp_parser_trait (cp_parser* parser, enum rid keyword)
       kind = CPTK_REF_CONVERTS_FROM_TEMPORARY;
       binary = true;
       break;
-    case RID_REMOVE_CV:
-      kind = CPTK_REMOVE_CV;
-      type = true;
-      break;
-    case RID_REMOVE_REFERENCE:
-      kind = CPTK_REMOVE_REFERENCE;
-      type = true;
-      break;
-    case RID_REMOVE_CVREF:
-      kind = CPTK_REMOVE_CVREF;
-      type = true;
+#define DEFTRAIT(TCC, CODE, NAME, ARITY) \
+    case RID_##CODE:			 \
+      type = (TCC == tcc_type);		 \
+      kind = CPTK_##CODE;		 \
+      binary = (ARITY == 2);		 \
+      variadic = (ARITY == -1);		 \
       break;
+#include "cp/cp-trait.def"
+#undef DEFTRAIT
     default:
       gcc_unreachable ();
     }
@@ -19882,9 +19854,10 @@ cp_parser_simple_type_specifier (cp_parser* parser,
       return type;
 
     case RID_UNDERLYING_TYPE:
-    case RID_REMOVE_CV:
-    case RID_REMOVE_REFERENCE:
-    case RID_REMOVE_CVREF:
+#define DEFTRAIT_TYPE(CODE, NAME, ARITY) \
+    case RID_##CODE:
+#include "cp/cp-trait.def"
+#undef DEFTRAIT_TYPE
       type = cp_parser_trait (parser, token->keyword);
       if (decl_specs)
 	cp_parser_set_decl_spec_type (decl_specs, type,
-- 
2.38.0.rc2