| Message ID | 20240228192843.188979-22-kmatsui@gcc.gnu.org |
|---|---|
| State | New |
| Headers | show |
| Series | Optimize more type traits | expand |
On 2/28/24 11:26, Ken Matsui wrote: > This patch implements built-in trait for std::rank. __rank seems too short, maybe __array_rank? Actually, it occurs to me that perhaps we should have been adding __builtin to all of these rather than just __ and the library trait name. I guess it's too late to do that for the GCC 14 traits, but we could do it for this group? > gcc/cp/ChangeLog: > > * cp-trait.def: Define __rank. > * constraint.cc (diagnose_trait_expr): Handle CPTK_RANK. > * semantics.cc (trait_expr_value): Likewise. > (finish_trait_expr): Likewise. > > gcc/testsuite/ChangeLog: > > * g++.dg/ext/has-builtin-1.C: Test existence of __rank. > * g++.dg/ext/rank.C: New test. > > Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org> > --- > gcc/cp/constraint.cc | 3 +++ > gcc/cp/cp-trait.def | 1 + > gcc/cp/semantics.cc | 23 ++++++++++++++++++++--- > gcc/testsuite/g++.dg/ext/has-builtin-1.C | 3 +++ > gcc/testsuite/g++.dg/ext/rank.C | 24 ++++++++++++++++++++++++ > 5 files changed, 51 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/ext/rank.C > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > index 000df847342..23ea66d9c12 100644 > --- a/gcc/cp/constraint.cc > +++ b/gcc/cp/constraint.cc > @@ -3870,6 +3870,9 @@ diagnose_trait_expr (tree expr, tree args) > case CPTK_IS_VOLATILE: > inform (loc, " %qT is not a volatile type", t1); > break; > + case CPTK_RANK: > + inform (loc, " %qT cannot yield a rank", t1); > + break; > case CPTK_REF_CONSTRUCTS_FROM_TEMPORARY: > inform (loc, " %qT is not a reference that binds to a temporary " > "object of type %qT (direct-initialization)", t1, t2); > diff --git a/gcc/cp/cp-trait.def b/gcc/cp/cp-trait.def > index 2d1cb7c227c..85056c8140b 100644 > --- a/gcc/cp/cp-trait.def > +++ b/gcc/cp/cp-trait.def > @@ -99,6 +99,7 @@ DEFTRAIT_EXPR (IS_TRIVIALLY_COPYABLE, "__is_trivially_copyable", 1) > DEFTRAIT_EXPR (IS_UNBOUNDED_ARRAY, "__is_unbounded_array", 1) > DEFTRAIT_EXPR (IS_UNION, "__is_union", 1) > DEFTRAIT_EXPR (IS_VOLATILE, "__is_volatile", 1) > +DEFTRAIT_EXPR (RANK, "__rank", 1) > DEFTRAIT_EXPR (REF_CONSTRUCTS_FROM_TEMPORARY, "__reference_constructs_from_temporary", 2) > DEFTRAIT_EXPR (REF_CONVERTS_FROM_TEMPORARY, "__reference_converts_from_temporary", 2) > DEFTRAIT_TYPE (REMOVE_ALL_EXTENTS, "__remove_all_extents", 1) > diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc > index 45dc509855a..7242db75248 100644 > --- a/gcc/cp/semantics.cc > +++ b/gcc/cp/semantics.cc > @@ -12550,6 +12550,9 @@ trait_expr_value (cp_trait_kind kind, tree type1, tree type2) > case CPTK_IS_DEDUCIBLE: > return type_targs_deducible_from (type1, type2); > > + /* __rank is handled in finish_trait_expr. */ > + case CPTK_RANK: This should have a gcc_unreachable. > + > #define DEFTRAIT_TYPE(CODE, NAME, ARITY) \ > case CPTK_##CODE: > #include "cp-trait.def" > @@ -12622,7 +12625,10 @@ finish_trait_expr (location_t loc, cp_trait_kind kind, tree type1, tree type2) > if (processing_template_decl) > { > tree trait_expr = make_node (TRAIT_EXPR); > - TREE_TYPE (trait_expr) = boolean_type_node; > + if (kind == CPTK_RANK) > + TREE_TYPE (trait_expr) = size_type_node; > + else > + TREE_TYPE (trait_expr) = boolean_type_node; > TRAIT_EXPR_TYPE1 (trait_expr) = type1; > TRAIT_EXPR_TYPE2 (trait_expr) = type2; > TRAIT_EXPR_KIND (trait_expr) = kind; > @@ -12714,6 +12720,7 @@ finish_trait_expr (location_t loc, cp_trait_kind kind, tree type1, tree type2) > case CPTK_IS_UNBOUNDED_ARRAY: > case CPTK_IS_UNION: > case CPTK_IS_VOLATILE: > + case CPTK_RANK: > break; > > case CPTK_IS_LAYOUT_COMPATIBLE: > @@ -12745,8 +12752,18 @@ finish_trait_expr (location_t loc, cp_trait_kind kind, tree type1, tree type2) > gcc_unreachable (); > } > > - tree val = (trait_expr_value (kind, type1, type2) > - ? boolean_true_node : boolean_false_node); > + tree val; > + if (kind == CPTK_RANK) > + { > + size_t rank = 0; > + for (; TREE_CODE (type1) == ARRAY_TYPE; type1 = TREE_TYPE (type1)) > + ++rank; > + val = build_int_cst (size_type_node, rank); > + } > + else > + val = (trait_expr_value (kind, type1, type2) > + ? boolean_true_node : boolean_false_node); > + > return maybe_wrap_with_location (val, loc); > } > > diff --git a/gcc/testsuite/g++.dg/ext/has-builtin-1.C b/gcc/testsuite/g++.dg/ext/has-builtin-1.C > index 3aca273aad6..7f7b27f7aa7 100644 > --- a/gcc/testsuite/g++.dg/ext/has-builtin-1.C > +++ b/gcc/testsuite/g++.dg/ext/has-builtin-1.C > @@ -179,6 +179,9 @@ > #if !__has_builtin (__is_volatile) > # error "__has_builtin (__is_volatile) failed" > #endif > +#if !__has_builtin (__rank) > +# error "__has_builtin (__rank) failed" > +#endif > #if !__has_builtin (__reference_constructs_from_temporary) > # error "__has_builtin (__reference_constructs_from_temporary) failed" > #endif > diff --git a/gcc/testsuite/g++.dg/ext/rank.C b/gcc/testsuite/g++.dg/ext/rank.C > new file mode 100644 > index 00000000000..28894184387 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/ext/rank.C > @@ -0,0 +1,24 @@ > +// { dg-do compile { target c++11 } } > + > +#include <cstddef> > + > +#define SA(X) static_assert((X),#X) > + > +class ClassType { }; > + > +SA(__rank(int) == 0); > +SA(__rank(int[2]) == 1); > +SA(__rank(int[][4]) == 2); > +SA(__rank(int[2][2][4][4][6][6]) == 6); > +SA(__rank(ClassType) == 0); > +SA(__rank(ClassType[2]) == 1); > +SA(__rank(ClassType[][4]) == 2); > +SA(__rank(ClassType[2][2][4][4][6][6]) == 6); > + > +template<class T> void f(T) = delete; > +void f(size_t); > + > +template<class T> > +void g() { f(__rank(T)); } > + > +template void g<int>();
On Tue, Apr 30, 2024 at 2:08 PM Jason Merrill <jason@redhat.com> wrote: > > On 2/28/24 11:26, Ken Matsui wrote: > > This patch implements built-in trait for std::rank. > > __rank seems too short, maybe __array_rank? > > Actually, it occurs to me that perhaps we should have been adding > __builtin to all of these rather than just __ and the library trait > name. I guess it's too late to do that for the GCC 14 traits, but we > could do it for this group? Let me add __builtin. I think it would be better to add __builtin for all existing traits for consistency, but is there any reason we want to avoid that? > > > gcc/cp/ChangeLog: > > > > * cp-trait.def: Define __rank. > > * constraint.cc (diagnose_trait_expr): Handle CPTK_RANK. > > * semantics.cc (trait_expr_value): Likewise. > > (finish_trait_expr): Likewise. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/ext/has-builtin-1.C: Test existence of __rank. > > * g++.dg/ext/rank.C: New test. > > > > Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org> > > --- > > gcc/cp/constraint.cc | 3 +++ > > gcc/cp/cp-trait.def | 1 + > > gcc/cp/semantics.cc | 23 ++++++++++++++++++++--- > > gcc/testsuite/g++.dg/ext/has-builtin-1.C | 3 +++ > > gcc/testsuite/g++.dg/ext/rank.C | 24 ++++++++++++++++++++++++ > > 5 files changed, 51 insertions(+), 3 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/ext/rank.C > > > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > > index 000df847342..23ea66d9c12 100644 > > --- a/gcc/cp/constraint.cc > > +++ b/gcc/cp/constraint.cc > > @@ -3870,6 +3870,9 @@ diagnose_trait_expr (tree expr, tree args) > > case CPTK_IS_VOLATILE: > > inform (loc, " %qT is not a volatile type", t1); > > break; > > + case CPTK_RANK: > > + inform (loc, " %qT cannot yield a rank", t1); > > + break; > > case CPTK_REF_CONSTRUCTS_FROM_TEMPORARY: > > inform (loc, " %qT is not a reference that binds to a temporary " > > "object of type %qT (direct-initialization)", t1, t2); > > diff --git a/gcc/cp/cp-trait.def b/gcc/cp/cp-trait.def > > index 2d1cb7c227c..85056c8140b 100644 > > --- a/gcc/cp/cp-trait.def > > +++ b/gcc/cp/cp-trait.def > > @@ -99,6 +99,7 @@ DEFTRAIT_EXPR (IS_TRIVIALLY_COPYABLE, "__is_trivially_copyable", 1) > > DEFTRAIT_EXPR (IS_UNBOUNDED_ARRAY, "__is_unbounded_array", 1) > > DEFTRAIT_EXPR (IS_UNION, "__is_union", 1) > > DEFTRAIT_EXPR (IS_VOLATILE, "__is_volatile", 1) > > +DEFTRAIT_EXPR (RANK, "__rank", 1) > > DEFTRAIT_EXPR (REF_CONSTRUCTS_FROM_TEMPORARY, "__reference_constructs_from_temporary", 2) > > DEFTRAIT_EXPR (REF_CONVERTS_FROM_TEMPORARY, "__reference_converts_from_temporary", 2) > > DEFTRAIT_TYPE (REMOVE_ALL_EXTENTS, "__remove_all_extents", 1) > > diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc > > index 45dc509855a..7242db75248 100644 > > --- a/gcc/cp/semantics.cc > > +++ b/gcc/cp/semantics.cc > > @@ -12550,6 +12550,9 @@ trait_expr_value (cp_trait_kind kind, tree type1, tree type2) > > case CPTK_IS_DEDUCIBLE: > > return type_targs_deducible_from (type1, type2); > > > > + /* __rank is handled in finish_trait_expr. */ > > + case CPTK_RANK: > > This should have a gcc_unreachable. > > > + > > #define DEFTRAIT_TYPE(CODE, NAME, ARITY) \ > > case CPTK_##CODE: > > #include "cp-trait.def" > > @@ -12622,7 +12625,10 @@ finish_trait_expr (location_t loc, cp_trait_kind kind, tree type1, tree type2) > > if (processing_template_decl) > > { > > tree trait_expr = make_node (TRAIT_EXPR); > > - TREE_TYPE (trait_expr) = boolean_type_node; > > + if (kind == CPTK_RANK) > > + TREE_TYPE (trait_expr) = size_type_node; > > + else > > + TREE_TYPE (trait_expr) = boolean_type_node; > > TRAIT_EXPR_TYPE1 (trait_expr) = type1; > > TRAIT_EXPR_TYPE2 (trait_expr) = type2; > > TRAIT_EXPR_KIND (trait_expr) = kind; > > @@ -12714,6 +12720,7 @@ finish_trait_expr (location_t loc, cp_trait_kind kind, tree type1, tree type2) > > case CPTK_IS_UNBOUNDED_ARRAY: > > case CPTK_IS_UNION: > > case CPTK_IS_VOLATILE: > > + case CPTK_RANK: > > break; > > > > case CPTK_IS_LAYOUT_COMPATIBLE: > > @@ -12745,8 +12752,18 @@ finish_trait_expr (location_t loc, cp_trait_kind kind, tree type1, tree type2) > > gcc_unreachable (); > > } > > > > - tree val = (trait_expr_value (kind, type1, type2) > > - ? boolean_true_node : boolean_false_node); > > + tree val; > > + if (kind == CPTK_RANK) > > + { > > + size_t rank = 0; > > + for (; TREE_CODE (type1) == ARRAY_TYPE; type1 = TREE_TYPE (type1)) > > + ++rank; > > + val = build_int_cst (size_type_node, rank); > > + } > > + else > > + val = (trait_expr_value (kind, type1, type2) > > + ? boolean_true_node : boolean_false_node); > > + > > return maybe_wrap_with_location (val, loc); > > } > > > > diff --git a/gcc/testsuite/g++.dg/ext/has-builtin-1.C b/gcc/testsuite/g++.dg/ext/has-builtin-1.C > > index 3aca273aad6..7f7b27f7aa7 100644 > > --- a/gcc/testsuite/g++.dg/ext/has-builtin-1.C > > +++ b/gcc/testsuite/g++.dg/ext/has-builtin-1.C > > @@ -179,6 +179,9 @@ > > #if !__has_builtin (__is_volatile) > > # error "__has_builtin (__is_volatile) failed" > > #endif > > +#if !__has_builtin (__rank) > > +# error "__has_builtin (__rank) failed" > > +#endif > > #if !__has_builtin (__reference_constructs_from_temporary) > > # error "__has_builtin (__reference_constructs_from_temporary) failed" > > #endif > > diff --git a/gcc/testsuite/g++.dg/ext/rank.C b/gcc/testsuite/g++.dg/ext/rank.C > > new file mode 100644 > > index 00000000000..28894184387 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/ext/rank.C > > @@ -0,0 +1,24 @@ > > +// { dg-do compile { target c++11 } } > > + > > +#include <cstddef> > > + > > +#define SA(X) static_assert((X),#X) > > + > > +class ClassType { }; > > + > > +SA(__rank(int) == 0); > > +SA(__rank(int[2]) == 1); > > +SA(__rank(int[][4]) == 2); > > +SA(__rank(int[2][2][4][4][6][6]) == 6); > > +SA(__rank(ClassType) == 0); > > +SA(__rank(ClassType[2]) == 1); > > +SA(__rank(ClassType[][4]) == 2); > > +SA(__rank(ClassType[2][2][4][4][6][6]) == 6); > > + > > +template<class T> void f(T) = delete; > > +void f(size_t); > > + > > +template<class T> > > +void g() { f(__rank(T)); } > > + > > +template void g<int>(); >
On Tue, 30 Apr 2024, Jason Merrill wrote: > On 2/28/24 11:26, Ken Matsui wrote: > > This patch implements built-in trait for std::rank. > > __rank seems too short, maybe __array_rank? > > Actually, it occurs to me that perhaps we should have been adding __builtin to > all of these rather than just __ and the library trait name. I guess it's too > late to do that for the GCC 14 traits, but we could do it for this group? Clang already implements many of these built-in, without using "__builtin" in their name. Shouldn't we be consistent with Clang where we can? > > > gcc/cp/ChangeLog: > > > > * cp-trait.def: Define __rank. > > * constraint.cc (diagnose_trait_expr): Handle CPTK_RANK. > > * semantics.cc (trait_expr_value): Likewise. > > (finish_trait_expr): Likewise. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/ext/has-builtin-1.C: Test existence of __rank. > > * g++.dg/ext/rank.C: New test. > > > > Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org> > > --- > > gcc/cp/constraint.cc | 3 +++ > > gcc/cp/cp-trait.def | 1 + > > gcc/cp/semantics.cc | 23 ++++++++++++++++++++--- > > gcc/testsuite/g++.dg/ext/has-builtin-1.C | 3 +++ > > gcc/testsuite/g++.dg/ext/rank.C | 24 ++++++++++++++++++++++++ > > 5 files changed, 51 insertions(+), 3 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/ext/rank.C > > > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > > index 000df847342..23ea66d9c12 100644 > > --- a/gcc/cp/constraint.cc > > +++ b/gcc/cp/constraint.cc > > @@ -3870,6 +3870,9 @@ diagnose_trait_expr (tree expr, tree args) > > case CPTK_IS_VOLATILE: > > inform (loc, " %qT is not a volatile type", t1); > > break; > > + case CPTK_RANK: > > + inform (loc, " %qT cannot yield a rank", t1); > > + break; > > case CPTK_REF_CONSTRUCTS_FROM_TEMPORARY: > > inform (loc, " %qT is not a reference that binds to a temporary " > > "object of type %qT (direct-initialization)", t1, t2); > > diff --git a/gcc/cp/cp-trait.def b/gcc/cp/cp-trait.def > > index 2d1cb7c227c..85056c8140b 100644 > > --- a/gcc/cp/cp-trait.def > > +++ b/gcc/cp/cp-trait.def > > @@ -99,6 +99,7 @@ DEFTRAIT_EXPR (IS_TRIVIALLY_COPYABLE, > > "__is_trivially_copyable", 1) > > DEFTRAIT_EXPR (IS_UNBOUNDED_ARRAY, "__is_unbounded_array", 1) > > DEFTRAIT_EXPR (IS_UNION, "__is_union", 1) > > DEFTRAIT_EXPR (IS_VOLATILE, "__is_volatile", 1) > > +DEFTRAIT_EXPR (RANK, "__rank", 1) > > DEFTRAIT_EXPR (REF_CONSTRUCTS_FROM_TEMPORARY, > > "__reference_constructs_from_temporary", 2) > > DEFTRAIT_EXPR (REF_CONVERTS_FROM_TEMPORARY, > > "__reference_converts_from_temporary", 2) > > DEFTRAIT_TYPE (REMOVE_ALL_EXTENTS, "__remove_all_extents", 1) > > diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc > > index 45dc509855a..7242db75248 100644 > > --- a/gcc/cp/semantics.cc > > +++ b/gcc/cp/semantics.cc > > @@ -12550,6 +12550,9 @@ trait_expr_value (cp_trait_kind kind, tree type1, > > tree type2) > > case CPTK_IS_DEDUCIBLE: > > return type_targs_deducible_from (type1, type2); > > + /* __rank is handled in finish_trait_expr. */ > > + case CPTK_RANK: > > This should have a gcc_unreachable. > > > + > > #define DEFTRAIT_TYPE(CODE, NAME, ARITY) \ > > case CPTK_##CODE: > > #include "cp-trait.def" > > @@ -12622,7 +12625,10 @@ finish_trait_expr (location_t loc, cp_trait_kind > > kind, tree type1, tree type2) > > if (processing_template_decl) > > { > > tree trait_expr = make_node (TRAIT_EXPR); > > - TREE_TYPE (trait_expr) = boolean_type_node; > > + if (kind == CPTK_RANK) > > + TREE_TYPE (trait_expr) = size_type_node; > > + else > > + TREE_TYPE (trait_expr) = boolean_type_node; > > TRAIT_EXPR_TYPE1 (trait_expr) = type1; > > TRAIT_EXPR_TYPE2 (trait_expr) = type2; > > TRAIT_EXPR_KIND (trait_expr) = kind; > > @@ -12714,6 +12720,7 @@ finish_trait_expr (location_t loc, cp_trait_kind > > kind, tree type1, tree type2) > > case CPTK_IS_UNBOUNDED_ARRAY: > > case CPTK_IS_UNION: > > case CPTK_IS_VOLATILE: > > + case CPTK_RANK: > > break; > > case CPTK_IS_LAYOUT_COMPATIBLE: > > @@ -12745,8 +12752,18 @@ finish_trait_expr (location_t loc, cp_trait_kind > > kind, tree type1, tree type2) > > gcc_unreachable (); > > } > > - tree val = (trait_expr_value (kind, type1, type2) > > - ? boolean_true_node : boolean_false_node); > > + tree val; > > + if (kind == CPTK_RANK) > > + { > > + size_t rank = 0; > > + for (; TREE_CODE (type1) == ARRAY_TYPE; type1 = TREE_TYPE (type1)) > > + ++rank; > > + val = build_int_cst (size_type_node, rank); > > + } > > + else > > + val = (trait_expr_value (kind, type1, type2) > > + ? boolean_true_node : boolean_false_node); > > + > > return maybe_wrap_with_location (val, loc); > > } > > diff --git a/gcc/testsuite/g++.dg/ext/has-builtin-1.C > > b/gcc/testsuite/g++.dg/ext/has-builtin-1.C > > index 3aca273aad6..7f7b27f7aa7 100644 > > --- a/gcc/testsuite/g++.dg/ext/has-builtin-1.C > > +++ b/gcc/testsuite/g++.dg/ext/has-builtin-1.C > > @@ -179,6 +179,9 @@ > > #if !__has_builtin (__is_volatile) > > # error "__has_builtin (__is_volatile) failed" > > #endif > > +#if !__has_builtin (__rank) > > +# error "__has_builtin (__rank) failed" > > +#endif > > #if !__has_builtin (__reference_constructs_from_temporary) > > # error "__has_builtin (__reference_constructs_from_temporary) failed" > > #endif > > diff --git a/gcc/testsuite/g++.dg/ext/rank.C > > b/gcc/testsuite/g++.dg/ext/rank.C > > new file mode 100644 > > index 00000000000..28894184387 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/ext/rank.C > > @@ -0,0 +1,24 @@ > > +// { dg-do compile { target c++11 } } > > + > > +#include <cstddef> > > + > > +#define SA(X) static_assert((X),#X) > > + > > +class ClassType { }; > > + > > +SA(__rank(int) == 0); > > +SA(__rank(int[2]) == 1); > > +SA(__rank(int[][4]) == 2); > > +SA(__rank(int[2][2][4][4][6][6]) == 6); > > +SA(__rank(ClassType) == 0); > > +SA(__rank(ClassType[2]) == 1); > > +SA(__rank(ClassType[][4]) == 2); > > +SA(__rank(ClassType[2][2][4][4][6][6]) == 6); > > + > > +template<class T> void f(T) = delete; > > +void f(size_t); > > + > > +template<class T> > > +void g() { f(__rank(T)); } > > + > > +template void g<int>(); > >
On Thu, May 2, 2024 at 8:16 AM Patrick Palka <ppalka@redhat.com> wrote: > > On Tue, 30 Apr 2024, Jason Merrill wrote: > > > On 2/28/24 11:26, Ken Matsui wrote: > > > This patch implements built-in trait for std::rank. > > > > __rank seems too short, maybe __array_rank? > > > > Actually, it occurs to me that perhaps we should have been adding __builtin to > > all of these rather than just __ and the library trait name. I guess it's too > > late to do that for the GCC 14 traits, but we could do it for this group? > > Clang already implements many of these built-in, without using > "__builtin" in their name. Shouldn't we be consistent with Clang where > we can? Oh, then, would using __array_rank and keeping existing built-in traits as-is sound like a better choice? https://github.com/llvm/llvm-project/blob/4aca302f5a82ee65847c88500b39a2530dfeceb4/libcxx/include/__type_traits/rank.h#L23 > > > > > > gcc/cp/ChangeLog: > > > > > > * cp-trait.def: Define __rank. > > > * constraint.cc (diagnose_trait_expr): Handle CPTK_RANK. > > > * semantics.cc (trait_expr_value): Likewise. > > > (finish_trait_expr): Likewise. > > > > > > gcc/testsuite/ChangeLog: > > > > > > * g++.dg/ext/has-builtin-1.C: Test existence of __rank. > > > * g++.dg/ext/rank.C: New test. > > > > > > Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org> > > > --- > > > gcc/cp/constraint.cc | 3 +++ > > > gcc/cp/cp-trait.def | 1 + > > > gcc/cp/semantics.cc | 23 ++++++++++++++++++++--- > > > gcc/testsuite/g++.dg/ext/has-builtin-1.C | 3 +++ > > > gcc/testsuite/g++.dg/ext/rank.C | 24 ++++++++++++++++++++++++ > > > 5 files changed, 51 insertions(+), 3 deletions(-) > > > create mode 100644 gcc/testsuite/g++.dg/ext/rank.C > > > > > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > > > index 000df847342..23ea66d9c12 100644 > > > --- a/gcc/cp/constraint.cc > > > +++ b/gcc/cp/constraint.cc > > > @@ -3870,6 +3870,9 @@ diagnose_trait_expr (tree expr, tree args) > > > case CPTK_IS_VOLATILE: > > > inform (loc, " %qT is not a volatile type", t1); > > > break; > > > + case CPTK_RANK: > > > + inform (loc, " %qT cannot yield a rank", t1); > > > + break; > > > case CPTK_REF_CONSTRUCTS_FROM_TEMPORARY: > > > inform (loc, " %qT is not a reference that binds to a temporary " > > > "object of type %qT (direct-initialization)", t1, t2); > > > diff --git a/gcc/cp/cp-trait.def b/gcc/cp/cp-trait.def > > > index 2d1cb7c227c..85056c8140b 100644 > > > --- a/gcc/cp/cp-trait.def > > > +++ b/gcc/cp/cp-trait.def > > > @@ -99,6 +99,7 @@ DEFTRAIT_EXPR (IS_TRIVIALLY_COPYABLE, > > > "__is_trivially_copyable", 1) > > > DEFTRAIT_EXPR (IS_UNBOUNDED_ARRAY, "__is_unbounded_array", 1) > > > DEFTRAIT_EXPR (IS_UNION, "__is_union", 1) > > > DEFTRAIT_EXPR (IS_VOLATILE, "__is_volatile", 1) > > > +DEFTRAIT_EXPR (RANK, "__rank", 1) > > > DEFTRAIT_EXPR (REF_CONSTRUCTS_FROM_TEMPORARY, > > > "__reference_constructs_from_temporary", 2) > > > DEFTRAIT_EXPR (REF_CONVERTS_FROM_TEMPORARY, > > > "__reference_converts_from_temporary", 2) > > > DEFTRAIT_TYPE (REMOVE_ALL_EXTENTS, "__remove_all_extents", 1) > > > diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc > > > index 45dc509855a..7242db75248 100644 > > > --- a/gcc/cp/semantics.cc > > > +++ b/gcc/cp/semantics.cc > > > @@ -12550,6 +12550,9 @@ trait_expr_value (cp_trait_kind kind, tree type1, > > > tree type2) > > > case CPTK_IS_DEDUCIBLE: > > > return type_targs_deducible_from (type1, type2); > > > + /* __rank is handled in finish_trait_expr. */ > > > + case CPTK_RANK: > > > > This should have a gcc_unreachable. > > > > > + > > > #define DEFTRAIT_TYPE(CODE, NAME, ARITY) \ > > > case CPTK_##CODE: > > > #include "cp-trait.def" > > > @@ -12622,7 +12625,10 @@ finish_trait_expr (location_t loc, cp_trait_kind > > > kind, tree type1, tree type2) > > > if (processing_template_decl) > > > { > > > tree trait_expr = make_node (TRAIT_EXPR); > > > - TREE_TYPE (trait_expr) = boolean_type_node; > > > + if (kind == CPTK_RANK) > > > + TREE_TYPE (trait_expr) = size_type_node; > > > + else > > > + TREE_TYPE (trait_expr) = boolean_type_node; > > > TRAIT_EXPR_TYPE1 (trait_expr) = type1; > > > TRAIT_EXPR_TYPE2 (trait_expr) = type2; > > > TRAIT_EXPR_KIND (trait_expr) = kind; > > > @@ -12714,6 +12720,7 @@ finish_trait_expr (location_t loc, cp_trait_kind > > > kind, tree type1, tree type2) > > > case CPTK_IS_UNBOUNDED_ARRAY: > > > case CPTK_IS_UNION: > > > case CPTK_IS_VOLATILE: > > > + case CPTK_RANK: > > > break; > > > case CPTK_IS_LAYOUT_COMPATIBLE: > > > @@ -12745,8 +12752,18 @@ finish_trait_expr (location_t loc, cp_trait_kind > > > kind, tree type1, tree type2) > > > gcc_unreachable (); > > > } > > > - tree val = (trait_expr_value (kind, type1, type2) > > > - ? boolean_true_node : boolean_false_node); > > > + tree val; > > > + if (kind == CPTK_RANK) > > > + { > > > + size_t rank = 0; > > > + for (; TREE_CODE (type1) == ARRAY_TYPE; type1 = TREE_TYPE (type1)) > > > + ++rank; > > > + val = build_int_cst (size_type_node, rank); > > > + } > > > + else > > > + val = (trait_expr_value (kind, type1, type2) > > > + ? boolean_true_node : boolean_false_node); > > > + > > > return maybe_wrap_with_location (val, loc); > > > } > > > diff --git a/gcc/testsuite/g++.dg/ext/has-builtin-1.C > > > b/gcc/testsuite/g++.dg/ext/has-builtin-1.C > > > index 3aca273aad6..7f7b27f7aa7 100644 > > > --- a/gcc/testsuite/g++.dg/ext/has-builtin-1.C > > > +++ b/gcc/testsuite/g++.dg/ext/has-builtin-1.C > > > @@ -179,6 +179,9 @@ > > > #if !__has_builtin (__is_volatile) > > > # error "__has_builtin (__is_volatile) failed" > > > #endif > > > +#if !__has_builtin (__rank) > > > +# error "__has_builtin (__rank) failed" > > > +#endif > > > #if !__has_builtin (__reference_constructs_from_temporary) > > > # error "__has_builtin (__reference_constructs_from_temporary) failed" > > > #endif > > > diff --git a/gcc/testsuite/g++.dg/ext/rank.C > > > b/gcc/testsuite/g++.dg/ext/rank.C > > > new file mode 100644 > > > index 00000000000..28894184387 > > > --- /dev/null > > > +++ b/gcc/testsuite/g++.dg/ext/rank.C > > > @@ -0,0 +1,24 @@ > > > +// { dg-do compile { target c++11 } } > > > + > > > +#include <cstddef> > > > + > > > +#define SA(X) static_assert((X),#X) > > > + > > > +class ClassType { }; > > > + > > > +SA(__rank(int) == 0); > > > +SA(__rank(int[2]) == 1); > > > +SA(__rank(int[][4]) == 2); > > > +SA(__rank(int[2][2][4][4][6][6]) == 6); > > > +SA(__rank(ClassType) == 0); > > > +SA(__rank(ClassType[2]) == 1); > > > +SA(__rank(ClassType[][4]) == 2); > > > +SA(__rank(ClassType[2][2][4][4][6][6]) == 6); > > > + > > > +template<class T> void f(T) = delete; > > > +void f(size_t); > > > + > > > +template<class T> > > > +void g() { f(__rank(T)); } > > > + > > > +template void g<int>(); > > > > >
On Thu, May 2, 2024 at 8:34 AM Ken Matsui <kmatsui@cs.washington.edu> wrote: > > On Thu, May 2, 2024 at 8:16 AM Patrick Palka <ppalka@redhat.com> wrote: > > > > On Tue, 30 Apr 2024, Jason Merrill wrote: > > > > > On 2/28/24 11:26, Ken Matsui wrote: > > > > This patch implements built-in trait for std::rank. > > > > > > __rank seems too short, maybe __array_rank? > > > > > > Actually, it occurs to me that perhaps we should have been adding __builtin to > > > all of these rather than just __ and the library trait name. I guess it's too > > > late to do that for the GCC 14 traits, but we could do it for this group? > > > > Clang already implements many of these built-in, without using > > "__builtin" in their name. Shouldn't we be consistent with Clang where > > we can? Since I had already replaced the prefix locally with __builtin, I submitted patches addressing all other Jason's reviews with that. Please let me know if we want to use __ and __array_rank. > > Oh, then, would using __array_rank and keeping existing built-in > traits as-is sound like a better choice? > > https://github.com/llvm/llvm-project/blob/4aca302f5a82ee65847c88500b39a2530dfeceb4/libcxx/include/__type_traits/rank.h#L23 > > > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > * cp-trait.def: Define __rank. > > > > * constraint.cc (diagnose_trait_expr): Handle CPTK_RANK. > > > > * semantics.cc (trait_expr_value): Likewise. > > > > (finish_trait_expr): Likewise. > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > * g++.dg/ext/has-builtin-1.C: Test existence of __rank. > > > > * g++.dg/ext/rank.C: New test. > > > > > > > > Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org> > > > > --- > > > > gcc/cp/constraint.cc | 3 +++ > > > > gcc/cp/cp-trait.def | 1 + > > > > gcc/cp/semantics.cc | 23 ++++++++++++++++++++--- > > > > gcc/testsuite/g++.dg/ext/has-builtin-1.C | 3 +++ > > > > gcc/testsuite/g++.dg/ext/rank.C | 24 ++++++++++++++++++++++++ > > > > 5 files changed, 51 insertions(+), 3 deletions(-) > > > > create mode 100644 gcc/testsuite/g++.dg/ext/rank.C > > > > > > > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > > > > index 000df847342..23ea66d9c12 100644 > > > > --- a/gcc/cp/constraint.cc > > > > +++ b/gcc/cp/constraint.cc > > > > @@ -3870,6 +3870,9 @@ diagnose_trait_expr (tree expr, tree args) > > > > case CPTK_IS_VOLATILE: > > > > inform (loc, " %qT is not a volatile type", t1); > > > > break; > > > > + case CPTK_RANK: > > > > + inform (loc, " %qT cannot yield a rank", t1); > > > > + break; > > > > case CPTK_REF_CONSTRUCTS_FROM_TEMPORARY: > > > > inform (loc, " %qT is not a reference that binds to a temporary " > > > > "object of type %qT (direct-initialization)", t1, t2); > > > > diff --git a/gcc/cp/cp-trait.def b/gcc/cp/cp-trait.def > > > > index 2d1cb7c227c..85056c8140b 100644 > > > > --- a/gcc/cp/cp-trait.def > > > > +++ b/gcc/cp/cp-trait.def > > > > @@ -99,6 +99,7 @@ DEFTRAIT_EXPR (IS_TRIVIALLY_COPYABLE, > > > > "__is_trivially_copyable", 1) > > > > DEFTRAIT_EXPR (IS_UNBOUNDED_ARRAY, "__is_unbounded_array", 1) > > > > DEFTRAIT_EXPR (IS_UNION, "__is_union", 1) > > > > DEFTRAIT_EXPR (IS_VOLATILE, "__is_volatile", 1) > > > > +DEFTRAIT_EXPR (RANK, "__rank", 1) > > > > DEFTRAIT_EXPR (REF_CONSTRUCTS_FROM_TEMPORARY, > > > > "__reference_constructs_from_temporary", 2) > > > > DEFTRAIT_EXPR (REF_CONVERTS_FROM_TEMPORARY, > > > > "__reference_converts_from_temporary", 2) > > > > DEFTRAIT_TYPE (REMOVE_ALL_EXTENTS, "__remove_all_extents", 1) > > > > diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc > > > > index 45dc509855a..7242db75248 100644 > > > > --- a/gcc/cp/semantics.cc > > > > +++ b/gcc/cp/semantics.cc > > > > @@ -12550,6 +12550,9 @@ trait_expr_value (cp_trait_kind kind, tree type1, > > > > tree type2) > > > > case CPTK_IS_DEDUCIBLE: > > > > return type_targs_deducible_from (type1, type2); > > > > + /* __rank is handled in finish_trait_expr. */ > > > > + case CPTK_RANK: > > > > > > This should have a gcc_unreachable. > > > > > > > + > > > > #define DEFTRAIT_TYPE(CODE, NAME, ARITY) \ > > > > case CPTK_##CODE: > > > > #include "cp-trait.def" > > > > @@ -12622,7 +12625,10 @@ finish_trait_expr (location_t loc, cp_trait_kind > > > > kind, tree type1, tree type2) > > > > if (processing_template_decl) > > > > { > > > > tree trait_expr = make_node (TRAIT_EXPR); > > > > - TREE_TYPE (trait_expr) = boolean_type_node; > > > > + if (kind == CPTK_RANK) > > > > + TREE_TYPE (trait_expr) = size_type_node; > > > > + else > > > > + TREE_TYPE (trait_expr) = boolean_type_node; > > > > TRAIT_EXPR_TYPE1 (trait_expr) = type1; > > > > TRAIT_EXPR_TYPE2 (trait_expr) = type2; > > > > TRAIT_EXPR_KIND (trait_expr) = kind; > > > > @@ -12714,6 +12720,7 @@ finish_trait_expr (location_t loc, cp_trait_kind > > > > kind, tree type1, tree type2) > > > > case CPTK_IS_UNBOUNDED_ARRAY: > > > > case CPTK_IS_UNION: > > > > case CPTK_IS_VOLATILE: > > > > + case CPTK_RANK: > > > > break; > > > > case CPTK_IS_LAYOUT_COMPATIBLE: > > > > @@ -12745,8 +12752,18 @@ finish_trait_expr (location_t loc, cp_trait_kind > > > > kind, tree type1, tree type2) > > > > gcc_unreachable (); > > > > } > > > > - tree val = (trait_expr_value (kind, type1, type2) > > > > - ? boolean_true_node : boolean_false_node); > > > > + tree val; > > > > + if (kind == CPTK_RANK) > > > > + { > > > > + size_t rank = 0; > > > > + for (; TREE_CODE (type1) == ARRAY_TYPE; type1 = TREE_TYPE (type1)) > > > > + ++rank; > > > > + val = build_int_cst (size_type_node, rank); > > > > + } > > > > + else > > > > + val = (trait_expr_value (kind, type1, type2) > > > > + ? boolean_true_node : boolean_false_node); > > > > + > > > > return maybe_wrap_with_location (val, loc); > > > > } > > > > diff --git a/gcc/testsuite/g++.dg/ext/has-builtin-1.C > > > > b/gcc/testsuite/g++.dg/ext/has-builtin-1.C > > > > index 3aca273aad6..7f7b27f7aa7 100644 > > > > --- a/gcc/testsuite/g++.dg/ext/has-builtin-1.C > > > > +++ b/gcc/testsuite/g++.dg/ext/has-builtin-1.C > > > > @@ -179,6 +179,9 @@ > > > > #if !__has_builtin (__is_volatile) > > > > # error "__has_builtin (__is_volatile) failed" > > > > #endif > > > > +#if !__has_builtin (__rank) > > > > +# error "__has_builtin (__rank) failed" > > > > +#endif > > > > #if !__has_builtin (__reference_constructs_from_temporary) > > > > # error "__has_builtin (__reference_constructs_from_temporary) failed" > > > > #endif > > > > diff --git a/gcc/testsuite/g++.dg/ext/rank.C > > > > b/gcc/testsuite/g++.dg/ext/rank.C > > > > new file mode 100644 > > > > index 00000000000..28894184387 > > > > --- /dev/null > > > > +++ b/gcc/testsuite/g++.dg/ext/rank.C > > > > @@ -0,0 +1,24 @@ > > > > +// { dg-do compile { target c++11 } } > > > > + > > > > +#include <cstddef> > > > > + > > > > +#define SA(X) static_assert((X),#X) > > > > + > > > > +class ClassType { }; > > > > + > > > > +SA(__rank(int) == 0); > > > > +SA(__rank(int[2]) == 1); > > > > +SA(__rank(int[][4]) == 2); > > > > +SA(__rank(int[2][2][4][4][6][6]) == 6); > > > > +SA(__rank(ClassType) == 0); > > > > +SA(__rank(ClassType[2]) == 1); > > > > +SA(__rank(ClassType[][4]) == 2); > > > > +SA(__rank(ClassType[2][2][4][4][6][6]) == 6); > > > > + > > > > +template<class T> void f(T) = delete; > > > > +void f(size_t); > > > > + > > > > +template<class T> > > > > +void g() { f(__rank(T)); } > > > > + > > > > +template void g<int>(); > > > > > > > >
On 5/2/24 12:20, Ken Matsui wrote: > On Thu, May 2, 2024 at 8:34 AM Ken Matsui <kmatsui@cs.washington.edu> wrote: >> >> On Thu, May 2, 2024 at 8:16 AM Patrick Palka <ppalka@redhat.com> wrote: >>> >>> On Tue, 30 Apr 2024, Jason Merrill wrote: >>> >>>> On 2/28/24 11:26, Ken Matsui wrote: >>>>> This patch implements built-in trait for std::rank. >>>> >>>> __rank seems too short, maybe __array_rank? >>>> >>>> Actually, it occurs to me that perhaps we should have been adding __builtin to >>>> all of these rather than just __ and the library trait name. I guess it's too >>>> late to do that for the GCC 14 traits, but we could do it for this group? >>> >>> Clang already implements many of these built-in, without using >>> "__builtin" in their name. Shouldn't we be consistent with Clang where >>> we can? > > Since I had already replaced the prefix locally with __builtin, I > submitted patches addressing all other Jason's reviews with that. > Please let me know if we want to use __ and __array_rank. If Clang already has a corresponding built-in (as with __array_rank, apparently), let's use the same name; otherwise let's add __builtin to the new ones. >> Oh, then, would using __array_rank and keeping existing built-in >> traits as-is sound like a better choice? >> >> https://github.com/llvm/llvm-project/blob/4aca302f5a82ee65847c88500b39a2530dfeceb4/libcxx/include/__type_traits/rank.h#L23 >> >>> >>>> >>>>> gcc/cp/ChangeLog: >>>>> >>>>> * cp-trait.def: Define __rank. >>>>> * constraint.cc (diagnose_trait_expr): Handle CPTK_RANK. >>>>> * semantics.cc (trait_expr_value): Likewise. >>>>> (finish_trait_expr): Likewise. >>>>> >>>>> gcc/testsuite/ChangeLog: >>>>> >>>>> * g++.dg/ext/has-builtin-1.C: Test existence of __rank. >>>>> * g++.dg/ext/rank.C: New test. >>>>> >>>>> Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org> >>>>> --- >>>>> gcc/cp/constraint.cc | 3 +++ >>>>> gcc/cp/cp-trait.def | 1 + >>>>> gcc/cp/semantics.cc | 23 ++++++++++++++++++++--- >>>>> gcc/testsuite/g++.dg/ext/has-builtin-1.C | 3 +++ >>>>> gcc/testsuite/g++.dg/ext/rank.C | 24 ++++++++++++++++++++++++ >>>>> 5 files changed, 51 insertions(+), 3 deletions(-) >>>>> create mode 100644 gcc/testsuite/g++.dg/ext/rank.C >>>>> >>>>> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc >>>>> index 000df847342..23ea66d9c12 100644 >>>>> --- a/gcc/cp/constraint.cc >>>>> +++ b/gcc/cp/constraint.cc >>>>> @@ -3870,6 +3870,9 @@ diagnose_trait_expr (tree expr, tree args) >>>>> case CPTK_IS_VOLATILE: >>>>> inform (loc, " %qT is not a volatile type", t1); >>>>> break; >>>>> + case CPTK_RANK: >>>>> + inform (loc, " %qT cannot yield a rank", t1); >>>>> + break; >>>>> case CPTK_REF_CONSTRUCTS_FROM_TEMPORARY: >>>>> inform (loc, " %qT is not a reference that binds to a temporary " >>>>> "object of type %qT (direct-initialization)", t1, t2); >>>>> diff --git a/gcc/cp/cp-trait.def b/gcc/cp/cp-trait.def >>>>> index 2d1cb7c227c..85056c8140b 100644 >>>>> --- a/gcc/cp/cp-trait.def >>>>> +++ b/gcc/cp/cp-trait.def >>>>> @@ -99,6 +99,7 @@ DEFTRAIT_EXPR (IS_TRIVIALLY_COPYABLE, >>>>> "__is_trivially_copyable", 1) >>>>> DEFTRAIT_EXPR (IS_UNBOUNDED_ARRAY, "__is_unbounded_array", 1) >>>>> DEFTRAIT_EXPR (IS_UNION, "__is_union", 1) >>>>> DEFTRAIT_EXPR (IS_VOLATILE, "__is_volatile", 1) >>>>> +DEFTRAIT_EXPR (RANK, "__rank", 1) >>>>> DEFTRAIT_EXPR (REF_CONSTRUCTS_FROM_TEMPORARY, >>>>> "__reference_constructs_from_temporary", 2) >>>>> DEFTRAIT_EXPR (REF_CONVERTS_FROM_TEMPORARY, >>>>> "__reference_converts_from_temporary", 2) >>>>> DEFTRAIT_TYPE (REMOVE_ALL_EXTENTS, "__remove_all_extents", 1) >>>>> diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc >>>>> index 45dc509855a..7242db75248 100644 >>>>> --- a/gcc/cp/semantics.cc >>>>> +++ b/gcc/cp/semantics.cc >>>>> @@ -12550,6 +12550,9 @@ trait_expr_value (cp_trait_kind kind, tree type1, >>>>> tree type2) >>>>> case CPTK_IS_DEDUCIBLE: >>>>> return type_targs_deducible_from (type1, type2); >>>>> + /* __rank is handled in finish_trait_expr. */ >>>>> + case CPTK_RANK: >>>> >>>> This should have a gcc_unreachable. >>>> >>>>> + >>>>> #define DEFTRAIT_TYPE(CODE, NAME, ARITY) \ >>>>> case CPTK_##CODE: >>>>> #include "cp-trait.def" >>>>> @@ -12622,7 +12625,10 @@ finish_trait_expr (location_t loc, cp_trait_kind >>>>> kind, tree type1, tree type2) >>>>> if (processing_template_decl) >>>>> { >>>>> tree trait_expr = make_node (TRAIT_EXPR); >>>>> - TREE_TYPE (trait_expr) = boolean_type_node; >>>>> + if (kind == CPTK_RANK) >>>>> + TREE_TYPE (trait_expr) = size_type_node; >>>>> + else >>>>> + TREE_TYPE (trait_expr) = boolean_type_node; >>>>> TRAIT_EXPR_TYPE1 (trait_expr) = type1; >>>>> TRAIT_EXPR_TYPE2 (trait_expr) = type2; >>>>> TRAIT_EXPR_KIND (trait_expr) = kind; >>>>> @@ -12714,6 +12720,7 @@ finish_trait_expr (location_t loc, cp_trait_kind >>>>> kind, tree type1, tree type2) >>>>> case CPTK_IS_UNBOUNDED_ARRAY: >>>>> case CPTK_IS_UNION: >>>>> case CPTK_IS_VOLATILE: >>>>> + case CPTK_RANK: >>>>> break; >>>>> case CPTK_IS_LAYOUT_COMPATIBLE: >>>>> @@ -12745,8 +12752,18 @@ finish_trait_expr (location_t loc, cp_trait_kind >>>>> kind, tree type1, tree type2) >>>>> gcc_unreachable (); >>>>> } >>>>> - tree val = (trait_expr_value (kind, type1, type2) >>>>> - ? boolean_true_node : boolean_false_node); >>>>> + tree val; >>>>> + if (kind == CPTK_RANK) >>>>> + { >>>>> + size_t rank = 0; >>>>> + for (; TREE_CODE (type1) == ARRAY_TYPE; type1 = TREE_TYPE (type1)) >>>>> + ++rank; >>>>> + val = build_int_cst (size_type_node, rank); >>>>> + } >>>>> + else >>>>> + val = (trait_expr_value (kind, type1, type2) >>>>> + ? boolean_true_node : boolean_false_node); >>>>> + >>>>> return maybe_wrap_with_location (val, loc); >>>>> } >>>>> diff --git a/gcc/testsuite/g++.dg/ext/has-builtin-1.C >>>>> b/gcc/testsuite/g++.dg/ext/has-builtin-1.C >>>>> index 3aca273aad6..7f7b27f7aa7 100644 >>>>> --- a/gcc/testsuite/g++.dg/ext/has-builtin-1.C >>>>> +++ b/gcc/testsuite/g++.dg/ext/has-builtin-1.C >>>>> @@ -179,6 +179,9 @@ >>>>> #if !__has_builtin (__is_volatile) >>>>> # error "__has_builtin (__is_volatile) failed" >>>>> #endif >>>>> +#if !__has_builtin (__rank) >>>>> +# error "__has_builtin (__rank) failed" >>>>> +#endif >>>>> #if !__has_builtin (__reference_constructs_from_temporary) >>>>> # error "__has_builtin (__reference_constructs_from_temporary) failed" >>>>> #endif >>>>> diff --git a/gcc/testsuite/g++.dg/ext/rank.C >>>>> b/gcc/testsuite/g++.dg/ext/rank.C >>>>> new file mode 100644 >>>>> index 00000000000..28894184387 >>>>> --- /dev/null >>>>> +++ b/gcc/testsuite/g++.dg/ext/rank.C >>>>> @@ -0,0 +1,24 @@ >>>>> +// { dg-do compile { target c++11 } } >>>>> + >>>>> +#include <cstddef> >>>>> + >>>>> +#define SA(X) static_assert((X),#X) >>>>> + >>>>> +class ClassType { }; >>>>> + >>>>> +SA(__rank(int) == 0); >>>>> +SA(__rank(int[2]) == 1); >>>>> +SA(__rank(int[][4]) == 2); >>>>> +SA(__rank(int[2][2][4][4][6][6]) == 6); >>>>> +SA(__rank(ClassType) == 0); >>>>> +SA(__rank(ClassType[2]) == 1); >>>>> +SA(__rank(ClassType[][4]) == 2); >>>>> +SA(__rank(ClassType[2][2][4][4][6][6]) == 6); >>>>> + >>>>> +template<class T> void f(T) = delete; >>>>> +void f(size_t); >>>>> + >>>>> +template<class T> >>>>> +void g() { f(__rank(T)); } >>>>> + >>>>> +template void g<int>(); >>>> >>>> >>> >
On Thu, May 2, 2024 at 9:45 AM Jason Merrill <jason@redhat.com> wrote: > > On 5/2/24 12:20, Ken Matsui wrote: > > On Thu, May 2, 2024 at 8:34 AM Ken Matsui <kmatsui@cs.washington.edu> wrote: > >> > >> On Thu, May 2, 2024 at 8:16 AM Patrick Palka <ppalka@redhat.com> wrote: > >>> > >>> On Tue, 30 Apr 2024, Jason Merrill wrote: > >>> > >>>> On 2/28/24 11:26, Ken Matsui wrote: > >>>>> This patch implements built-in trait for std::rank. > >>>> > >>>> __rank seems too short, maybe __array_rank? > >>>> > >>>> Actually, it occurs to me that perhaps we should have been adding __builtin to > >>>> all of these rather than just __ and the library trait name. I guess it's too > >>>> late to do that for the GCC 14 traits, but we could do it for this group? > >>> > >>> Clang already implements many of these built-in, without using > >>> "__builtin" in their name. Shouldn't we be consistent with Clang where > >>> we can? > > > > Since I had already replaced the prefix locally with __builtin, I > > submitted patches addressing all other Jason's reviews with that. > > Please let me know if we want to use __ and __array_rank. > > If Clang already has a corresponding built-in (as with __array_rank, > apparently), let's use the same name; otherwise let's add __builtin to > the new ones. I see. It looks like Clang does not have built-ins for is_unbounded_array, is_invocable, and is_nothrow_invocable. I will revert other built-ins. > > >> Oh, then, would using __array_rank and keeping existing built-in > >> traits as-is sound like a better choice? > >> > >> https://github.com/llvm/llvm-project/blob/4aca302f5a82ee65847c88500b39a2530dfeceb4/libcxx/include/__type_traits/rank.h#L23 > >> > >>> > >>>> > >>>>> gcc/cp/ChangeLog: > >>>>> > >>>>> * cp-trait.def: Define __rank. > >>>>> * constraint.cc (diagnose_trait_expr): Handle CPTK_RANK. > >>>>> * semantics.cc (trait_expr_value): Likewise. > >>>>> (finish_trait_expr): Likewise. > >>>>> > >>>>> gcc/testsuite/ChangeLog: > >>>>> > >>>>> * g++.dg/ext/has-builtin-1.C: Test existence of __rank. > >>>>> * g++.dg/ext/rank.C: New test. > >>>>> > >>>>> Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org> > >>>>> --- > >>>>> gcc/cp/constraint.cc | 3 +++ > >>>>> gcc/cp/cp-trait.def | 1 + > >>>>> gcc/cp/semantics.cc | 23 ++++++++++++++++++++--- > >>>>> gcc/testsuite/g++.dg/ext/has-builtin-1.C | 3 +++ > >>>>> gcc/testsuite/g++.dg/ext/rank.C | 24 ++++++++++++++++++++++++ > >>>>> 5 files changed, 51 insertions(+), 3 deletions(-) > >>>>> create mode 100644 gcc/testsuite/g++.dg/ext/rank.C > >>>>> > >>>>> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > >>>>> index 000df847342..23ea66d9c12 100644 > >>>>> --- a/gcc/cp/constraint.cc > >>>>> +++ b/gcc/cp/constraint.cc > >>>>> @@ -3870,6 +3870,9 @@ diagnose_trait_expr (tree expr, tree args) > >>>>> case CPTK_IS_VOLATILE: > >>>>> inform (loc, " %qT is not a volatile type", t1); > >>>>> break; > >>>>> + case CPTK_RANK: > >>>>> + inform (loc, " %qT cannot yield a rank", t1); > >>>>> + break; > >>>>> case CPTK_REF_CONSTRUCTS_FROM_TEMPORARY: > >>>>> inform (loc, " %qT is not a reference that binds to a temporary " > >>>>> "object of type %qT (direct-initialization)", t1, t2); > >>>>> diff --git a/gcc/cp/cp-trait.def b/gcc/cp/cp-trait.def > >>>>> index 2d1cb7c227c..85056c8140b 100644 > >>>>> --- a/gcc/cp/cp-trait.def > >>>>> +++ b/gcc/cp/cp-trait.def > >>>>> @@ -99,6 +99,7 @@ DEFTRAIT_EXPR (IS_TRIVIALLY_COPYABLE, > >>>>> "__is_trivially_copyable", 1) > >>>>> DEFTRAIT_EXPR (IS_UNBOUNDED_ARRAY, "__is_unbounded_array", 1) > >>>>> DEFTRAIT_EXPR (IS_UNION, "__is_union", 1) > >>>>> DEFTRAIT_EXPR (IS_VOLATILE, "__is_volatile", 1) > >>>>> +DEFTRAIT_EXPR (RANK, "__rank", 1) > >>>>> DEFTRAIT_EXPR (REF_CONSTRUCTS_FROM_TEMPORARY, > >>>>> "__reference_constructs_from_temporary", 2) > >>>>> DEFTRAIT_EXPR (REF_CONVERTS_FROM_TEMPORARY, > >>>>> "__reference_converts_from_temporary", 2) > >>>>> DEFTRAIT_TYPE (REMOVE_ALL_EXTENTS, "__remove_all_extents", 1) > >>>>> diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc > >>>>> index 45dc509855a..7242db75248 100644 > >>>>> --- a/gcc/cp/semantics.cc > >>>>> +++ b/gcc/cp/semantics.cc > >>>>> @@ -12550,6 +12550,9 @@ trait_expr_value (cp_trait_kind kind, tree type1, > >>>>> tree type2) > >>>>> case CPTK_IS_DEDUCIBLE: > >>>>> return type_targs_deducible_from (type1, type2); > >>>>> + /* __rank is handled in finish_trait_expr. */ > >>>>> + case CPTK_RANK: > >>>> > >>>> This should have a gcc_unreachable. > >>>> > >>>>> + > >>>>> #define DEFTRAIT_TYPE(CODE, NAME, ARITY) \ > >>>>> case CPTK_##CODE: > >>>>> #include "cp-trait.def" > >>>>> @@ -12622,7 +12625,10 @@ finish_trait_expr (location_t loc, cp_trait_kind > >>>>> kind, tree type1, tree type2) > >>>>> if (processing_template_decl) > >>>>> { > >>>>> tree trait_expr = make_node (TRAIT_EXPR); > >>>>> - TREE_TYPE (trait_expr) = boolean_type_node; > >>>>> + if (kind == CPTK_RANK) > >>>>> + TREE_TYPE (trait_expr) = size_type_node; > >>>>> + else > >>>>> + TREE_TYPE (trait_expr) = boolean_type_node; > >>>>> TRAIT_EXPR_TYPE1 (trait_expr) = type1; > >>>>> TRAIT_EXPR_TYPE2 (trait_expr) = type2; > >>>>> TRAIT_EXPR_KIND (trait_expr) = kind; > >>>>> @@ -12714,6 +12720,7 @@ finish_trait_expr (location_t loc, cp_trait_kind > >>>>> kind, tree type1, tree type2) > >>>>> case CPTK_IS_UNBOUNDED_ARRAY: > >>>>> case CPTK_IS_UNION: > >>>>> case CPTK_IS_VOLATILE: > >>>>> + case CPTK_RANK: > >>>>> break; > >>>>> case CPTK_IS_LAYOUT_COMPATIBLE: > >>>>> @@ -12745,8 +12752,18 @@ finish_trait_expr (location_t loc, cp_trait_kind > >>>>> kind, tree type1, tree type2) > >>>>> gcc_unreachable (); > >>>>> } > >>>>> - tree val = (trait_expr_value (kind, type1, type2) > >>>>> - ? boolean_true_node : boolean_false_node); > >>>>> + tree val; > >>>>> + if (kind == CPTK_RANK) > >>>>> + { > >>>>> + size_t rank = 0; > >>>>> + for (; TREE_CODE (type1) == ARRAY_TYPE; type1 = TREE_TYPE (type1)) > >>>>> + ++rank; > >>>>> + val = build_int_cst (size_type_node, rank); > >>>>> + } > >>>>> + else > >>>>> + val = (trait_expr_value (kind, type1, type2) > >>>>> + ? boolean_true_node : boolean_false_node); > >>>>> + > >>>>> return maybe_wrap_with_location (val, loc); > >>>>> } > >>>>> diff --git a/gcc/testsuite/g++.dg/ext/has-builtin-1.C > >>>>> b/gcc/testsuite/g++.dg/ext/has-builtin-1.C > >>>>> index 3aca273aad6..7f7b27f7aa7 100644 > >>>>> --- a/gcc/testsuite/g++.dg/ext/has-builtin-1.C > >>>>> +++ b/gcc/testsuite/g++.dg/ext/has-builtin-1.C > >>>>> @@ -179,6 +179,9 @@ > >>>>> #if !__has_builtin (__is_volatile) > >>>>> # error "__has_builtin (__is_volatile) failed" > >>>>> #endif > >>>>> +#if !__has_builtin (__rank) > >>>>> +# error "__has_builtin (__rank) failed" > >>>>> +#endif > >>>>> #if !__has_builtin (__reference_constructs_from_temporary) > >>>>> # error "__has_builtin (__reference_constructs_from_temporary) failed" > >>>>> #endif > >>>>> diff --git a/gcc/testsuite/g++.dg/ext/rank.C > >>>>> b/gcc/testsuite/g++.dg/ext/rank.C > >>>>> new file mode 100644 > >>>>> index 00000000000..28894184387 > >>>>> --- /dev/null > >>>>> +++ b/gcc/testsuite/g++.dg/ext/rank.C > >>>>> @@ -0,0 +1,24 @@ > >>>>> +// { dg-do compile { target c++11 } } > >>>>> + > >>>>> +#include <cstddef> > >>>>> + > >>>>> +#define SA(X) static_assert((X),#X) > >>>>> + > >>>>> +class ClassType { }; > >>>>> + > >>>>> +SA(__rank(int) == 0); > >>>>> +SA(__rank(int[2]) == 1); > >>>>> +SA(__rank(int[][4]) == 2); > >>>>> +SA(__rank(int[2][2][4][4][6][6]) == 6); > >>>>> +SA(__rank(ClassType) == 0); > >>>>> +SA(__rank(ClassType[2]) == 1); > >>>>> +SA(__rank(ClassType[][4]) == 2); > >>>>> +SA(__rank(ClassType[2][2][4][4][6][6]) == 6); > >>>>> + > >>>>> +template<class T> void f(T) = delete; > >>>>> +void f(size_t); > >>>>> + > >>>>> +template<class T> > >>>>> +void g() { f(__rank(T)); } > >>>>> + > >>>>> +template void g<int>(); > >>>> > >>>> > >>> > > >
On 5/2/24 12:45, Jason Merrill wrote: > On 5/2/24 12:20, Ken Matsui wrote: >> On Thu, May 2, 2024 at 8:34 AM Ken Matsui <kmatsui@cs.washington.edu> >> wrote: >>> >>> On Thu, May 2, 2024 at 8:16 AM Patrick Palka <ppalka@redhat.com> wrote: >>>> >>>> On Tue, 30 Apr 2024, Jason Merrill wrote: >>>> >>>>> On 2/28/24 11:26, Ken Matsui wrote: >>>>>> This patch implements built-in trait for std::rank. >>>>> >>>>> __rank seems too short, maybe __array_rank? >>>>> >>>>> Actually, it occurs to me that perhaps we should have been adding >>>>> __builtin to >>>>> all of these rather than just __ and the library trait name. I >>>>> guess it's too >>>>> late to do that for the GCC 14 traits, but we could do it for this >>>>> group? >>>> >>>> Clang already implements many of these built-in, without using >>>> "__builtin" in their name. Shouldn't we be consistent with Clang where >>>> we can? >> >> Since I had already replaced the prefix locally with __builtin, I >> submitted patches addressing all other Jason's reviews with that. >> Please let me know if we want to use __ and __array_rank. > > If Clang already has a corresponding built-in (as with __array_rank, > apparently), let's use the same name; otherwise let's add __builtin to > the new ones. Eh, this could probably use more discussion. The practice of omitting __builtin from type-trait builtins goes back to Paolo's r123366 (cb68ec50) for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=26099 There was some discussion of how to name the built-ins back in https://gcc.gnu.org/pipermail/gcc-patches/2007-March/thread.html#212171 but __builtin wasn't discussed. Apparently this naming convention follows the MSVC precedent: http://msdn2.microsoft.com/en-us/library/ms177194.aspx I notice some discussion of this pattern around Clang adding various built-ins in https://github.com/llvm/llvm-project/issues/61852 indicating that this is a policy based on precedent. But I don't see any actual reason for this pattern other than that it's what Paolo happened to do in 2007. I'm not sure what the right way forward is. Perhaps we're stuck with the questionable choices of the past. Jason
On Thu, May 2, 2024 at 10:12 AM Jason Merrill <jason@redhat.com> wrote: > > On 5/2/24 12:45, Jason Merrill wrote: > > On 5/2/24 12:20, Ken Matsui wrote: > >> On Thu, May 2, 2024 at 8:34 AM Ken Matsui <kmatsui@cs.washington.edu> > >> wrote: > >>> > >>> On Thu, May 2, 2024 at 8:16 AM Patrick Palka <ppalka@redhat.com> wrote: > >>>> > >>>> On Tue, 30 Apr 2024, Jason Merrill wrote: > >>>> > >>>>> On 2/28/24 11:26, Ken Matsui wrote: > >>>>>> This patch implements built-in trait for std::rank. > >>>>> > >>>>> __rank seems too short, maybe __array_rank? > >>>>> > >>>>> Actually, it occurs to me that perhaps we should have been adding > >>>>> __builtin to > >>>>> all of these rather than just __ and the library trait name. I > >>>>> guess it's too > >>>>> late to do that for the GCC 14 traits, but we could do it for this > >>>>> group? > >>>> > >>>> Clang already implements many of these built-in, without using > >>>> "__builtin" in their name. Shouldn't we be consistent with Clang where > >>>> we can? > >> > >> Since I had already replaced the prefix locally with __builtin, I > >> submitted patches addressing all other Jason's reviews with that. > >> Please let me know if we want to use __ and __array_rank. > > > > If Clang already has a corresponding built-in (as with __array_rank, > > apparently), let's use the same name; otherwise let's add __builtin to > > the new ones. > > Eh, this could probably use more discussion. > > The practice of omitting __builtin from type-trait builtins goes back to > Paolo's r123366 (cb68ec50) for > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=26099 > > There was some discussion of how to name the built-ins back in > https://gcc.gnu.org/pipermail/gcc-patches/2007-March/thread.html#212171 > but __builtin wasn't discussed. > > Apparently this naming convention follows the MSVC precedent: > http://msdn2.microsoft.com/en-us/library/ms177194.aspx > > I notice some discussion of this pattern around Clang adding various > built-ins in https://github.com/llvm/llvm-project/issues/61852 > indicating that this is a policy based on precedent. > > But I don't see any actual reason for this pattern other than that it's > what Paolo happened to do in 2007. > > I'm not sure what the right way forward is. Perhaps we're stuck with > the questionable choices of the past. > Hmm, I personally prefer the __builtin prefix. However, it seems that we need to reach a consensus across MSVC, Clang, and GCC. Would this be realistically possible? Until then, I think it would be better to use __ for all built-in traits. What do you think? > Jason >
On Thu, 2 May 2024 at 20:25, Ken Matsui <kmatsui@cs.washington.edu> wrote: > > There was some discussion of how to name the built-ins back in > > https://gcc.gnu.org/pipermail/gcc-patches/2007-March/thread.html#212171 > > but __builtin wasn't discussed. > > > > Apparently this naming convention follows the MSVC precedent: > > http://msdn2.microsoft.com/en-us/library/ms177194.aspx > > > > I notice some discussion of this pattern around Clang adding various > > built-ins in https://github.com/llvm/llvm-project/issues/61852 > > indicating that this is a policy based on precedent. > > > > But I don't see any actual reason for this pattern other than that it's > > what Paolo happened to do in 2007. > > > > I'm not sure what the right way forward is. Perhaps we're stuck with > > the questionable choices of the past. > > > > Hmm, I personally prefer the __builtin prefix. However, it seems that > we need to reach a consensus across MSVC, Clang, and GCC. Would this > be realistically possible? > > Until then, I think it would be better to use __ for all built-in > traits. What do you think? My 0.02: __builtin as a prefix doesn't serve much of a purpose. Consider __is_constructible. It doesn't add value to make that __builtin_is_constructible. It's a built-in. Of course it's a built-in. It's a compiler-implemented trait, and this is just the intrinsic that implements it. Most of the existing builtins for traits don't use a __builtin prefix. Not in GCC, not in other compilers. They are indeed just double-underscored versions of the traits. I think that's fine, and consistent. There's precedent for this across Embarcadero, Clang, MSVC, and GCC. See https://clang.llvm.org/docs/LanguageExtensions.html Yes, I know it's inconsistent with other built-ins that aren't C++ library traits. But the water's been flowing under the bridge on that question for a while now. I would also prefer at least considering mimicking a trait builtin's name if some other compiler did it first. That's not a hill to die on, we don't need to be 100% compatible including the naming, but if we can, we should just use a name that was chosen by someone else already. It's just nice to have the same name if the traits do exactly the same thing. If they don't, then it's good and in fact very good to give our trait a different name.
On Thu, May 02, 2024 at 08:37:53PM +0300, Ville Voutilainen wrote: > On Thu, 2 May 2024 at 20:25, Ken Matsui <kmatsui@cs.washington.edu> wrote: > > > There was some discussion of how to name the built-ins back in > > > https://gcc.gnu.org/pipermail/gcc-patches/2007-March/thread.html#212171 > > > but __builtin wasn't discussed. > > > > > > Apparently this naming convention follows the MSVC precedent: > > > http://msdn2.microsoft.com/en-us/library/ms177194.aspx > > > > > > I notice some discussion of this pattern around Clang adding various > > > built-ins in https://github.com/llvm/llvm-project/issues/61852 > > > indicating that this is a policy based on precedent. > > > > > > But I don't see any actual reason for this pattern other than that it's > > > what Paolo happened to do in 2007. > > > > > > I'm not sure what the right way forward is. Perhaps we're stuck with > > > the questionable choices of the past. > > > > > > > Hmm, I personally prefer the __builtin prefix. However, it seems that > > we need to reach a consensus across MSVC, Clang, and GCC. Would this > > be realistically possible? > > > > Until then, I think it would be better to use __ for all built-in > > traits. What do you think? > > My 0.02: __builtin as a prefix doesn't serve much of a purpose. > Consider __is_constructible. It doesn't add value > to make that __builtin_is_constructible. It's a built-in. Of course > it's a built-in. It's a compiler-implemented trait, and > this is just the intrinsic that implements it. FWIW, I also like __is_constructible better than __builtin_is_constructible. > Most of the existing builtins for traits don't use a __builtin prefix. > Not in GCC, not in other compilers. They are indeed > just double-underscored versions of the traits. I think that's fine, > and consistent. There's precedent for this > across Embarcadero, Clang, MSVC, and GCC. See > https://clang.llvm.org/docs/LanguageExtensions.html > > Yes, I know it's inconsistent with other built-ins that aren't C++ > library traits. But the water's been flowing under > the bridge on that question for a while now. > > I would also prefer at least considering mimicking a trait builtin's > name if some other compiler did it first. That's not a hill > to die on, we don't need to be 100% compatible including the naming, > but if we can, we should just use a name that was > chosen by someone else already. It's just nice to have the same name > if the traits do exactly the same thing. If they don't, > then it's good and in fact very good to give our trait a different name. > Marek
On Thu, 2 May 2024 at 18:38, Ville Voutilainen <ville.voutilainen@gmail.com> wrote: > > On Thu, 2 May 2024 at 20:25, Ken Matsui <kmatsui@cs.washington.edu> wrote: > > > There was some discussion of how to name the built-ins back in > > > https://gcc.gnu.org/pipermail/gcc-patches/2007-March/thread.html#212171 > > > but __builtin wasn't discussed. > > > > > > Apparently this naming convention follows the MSVC precedent: > > > http://msdn2.microsoft.com/en-us/library/ms177194.aspx > > > > > > I notice some discussion of this pattern around Clang adding various > > > built-ins in https://github.com/llvm/llvm-project/issues/61852 > > > indicating that this is a policy based on precedent. > > > > > > But I don't see any actual reason for this pattern other than that it's > > > what Paolo happened to do in 2007. > > > > > > I'm not sure what the right way forward is. Perhaps we're stuck with > > > the questionable choices of the past. > > > > > > > Hmm, I personally prefer the __builtin prefix. However, it seems that > > we need to reach a consensus across MSVC, Clang, and GCC. Would this > > be realistically possible? > > > > Until then, I think it would be better to use __ for all built-in > > traits. What do you think? > > My 0.02: __builtin as a prefix doesn't serve much of a purpose. The main advantage is that it avoids clashes with std::lib components that previously used the __is_foo name and which need to be renamed to support a built-in of that name.
On Thu, May 2, 2024 at 12:18 PM Jonathan Wakely <jwakely.gcc@gmail.com> wrote: > > On Thu, 2 May 2024 at 18:38, Ville Voutilainen > <ville.voutilainen@gmail.com> wrote: > > > > On Thu, 2 May 2024 at 20:25, Ken Matsui <kmatsui@cs.washington.edu> wrote: > > > > There was some discussion of how to name the built-ins back in > > > > https://gcc.gnu.org/pipermail/gcc-patches/2007-March/thread.html#212171 > > > > but __builtin wasn't discussed. > > > > > > > > Apparently this naming convention follows the MSVC precedent: > > > > http://msdn2.microsoft.com/en-us/library/ms177194.aspx > > > > > > > > I notice some discussion of this pattern around Clang adding various > > > > built-ins in https://github.com/llvm/llvm-project/issues/61852 > > > > indicating that this is a policy based on precedent. > > > > > > > > But I don't see any actual reason for this pattern other than that it's > > > > what Paolo happened to do in 2007. > > > > > > > > I'm not sure what the right way forward is. Perhaps we're stuck with > > > > the questionable choices of the past. > > > > > > > > > > Hmm, I personally prefer the __builtin prefix. However, it seems that > > > we need to reach a consensus across MSVC, Clang, and GCC. Would this > > > be realistically possible? > > > > > > Until then, I think it would be better to use __ for all built-in > > > traits. What do you think? > > > > My 0.02: __builtin as a prefix doesn't serve much of a purpose. > > The main advantage is that it avoids clashes with std::lib components > that previously used the __is_foo name and which need to be renamed to > support a built-in of that name. I updated the parser to ignore the use of the same identifiers in existing codes, such as struct __is_pointer in bits/cpp_type_traits.h. It is working correctly so far, but for example, if there's a constructor, there would be clashes. https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=df3559d951ba6572e254a1bd1ef9a34b6e543325
On Thu, May 2, 2024 at 10:54 AM Marek Polacek <polacek@redhat.com> wrote: > > On Thu, May 02, 2024 at 08:37:53PM +0300, Ville Voutilainen wrote: > > On Thu, 2 May 2024 at 20:25, Ken Matsui <kmatsui@cs.washington.edu> wrote: > > > > There was some discussion of how to name the built-ins back in > > > > https://gcc.gnu.org/pipermail/gcc-patches/2007-March/thread.html#212171 > > > > but __builtin wasn't discussed. > > > > > > > > Apparently this naming convention follows the MSVC precedent: > > > > http://msdn2.microsoft.com/en-us/library/ms177194.aspx > > > > > > > > I notice some discussion of this pattern around Clang adding various > > > > built-ins in https://github.com/llvm/llvm-project/issues/61852 > > > > indicating that this is a policy based on precedent. > > > > > > > > But I don't see any actual reason for this pattern other than that it's > > > > what Paolo happened to do in 2007. > > > > > > > > I'm not sure what the right way forward is. Perhaps we're stuck with > > > > the questionable choices of the past. > > > > > > > > > > Hmm, I personally prefer the __builtin prefix. However, it seems that > > > we need to reach a consensus across MSVC, Clang, and GCC. Would this > > > be realistically possible? > > > > > > Until then, I think it would be better to use __ for all built-in > > > traits. What do you think? > > > > My 0.02: __builtin as a prefix doesn't serve much of a purpose. > > Consider __is_constructible. It doesn't add value > > to make that __builtin_is_constructible. It's a built-in. Of course > > it's a built-in. It's a compiler-implemented trait, and > > this is just the intrinsic that implements it. > > FWIW, I also like __is_constructible better than __builtin_is_constructible. So, updating all existing built-in trait names does not seem realistic. I think there are two options: 1. As Jason said, we can use the same name as Clang does and otherwise use __builtin. 2. Or we can simply use __ for all built-in traits to keep consistency with other built-in traits. Then, I feel option 2 would sound better since it's consistent across all built-in type traits and it might confuse Clang when they implement the same built-in. Also, it would be easier for me to implement built-in traits as I don't need to dig into the Clang code every time I add a new one. > > > Most of the existing builtins for traits don't use a __builtin prefix. > > Not in GCC, not in other compilers. They are indeed > > just double-underscored versions of the traits. I think that's fine, > > and consistent. There's precedent for this > > across Embarcadero, Clang, MSVC, and GCC. See > > https://clang.llvm.org/docs/LanguageExtensions.html > > > > Yes, I know it's inconsistent with other built-ins that aren't C++ > > library traits. But the water's been flowing under > > the bridge on that question for a while now. > > > > I would also prefer at least considering mimicking a trait builtin's > > name if some other compiler did it first. That's not a hill > > to die on, we don't need to be 100% compatible including the naming, > > but if we can, we should just use a name that was > > chosen by someone else already. It's just nice to have the same name > > if the traits do exactly the same thing. If they don't, > > then it's good and in fact very good to give our trait a different name. > > > > Marek >
> On 2 May 2024, at 20:30, Ken Matsui <kmatsui@cs.washington.edu> wrote: > > On Thu, May 2, 2024 at 10:54 AM Marek Polacek <polacek@redhat.com> wrote: >> >> On Thu, May 02, 2024 at 08:37:53PM +0300, Ville Voutilainen wrote: >>> On Thu, 2 May 2024 at 20:25, Ken Matsui <kmatsui@cs.washington.edu> wrote: >>>>> There was some discussion of how to name the built-ins back in >>>>> https://gcc.gnu.org/pipermail/gcc-patches/2007-March/thread.html#212171 >>>>> but __builtin wasn't discussed. >>>>> >>>>> Apparently this naming convention follows the MSVC precedent: >>>>> http://msdn2.microsoft.com/en-us/library/ms177194.aspx >>>>> >>>>> I notice some discussion of this pattern around Clang adding various >>>>> built-ins in https://github.com/llvm/llvm-project/issues/61852 >>>>> indicating that this is a policy based on precedent. >>>>> >>>>> But I don't see any actual reason for this pattern other than that it's >>>>> what Paolo happened to do in 2007. >>>>> >>>>> I'm not sure what the right way forward is. Perhaps we're stuck with >>>>> the questionable choices of the past. >>>>> >>>> >>>> Hmm, I personally prefer the __builtin prefix. However, it seems that >>>> we need to reach a consensus across MSVC, Clang, and GCC. Would this >>>> be realistically possible? >>>> >>>> Until then, I think it would be better to use __ for all built-in >>>> traits. What do you think? >>> >>> My 0.02: __builtin as a prefix doesn't serve much of a purpose. >>> Consider __is_constructible. It doesn't add value >>> to make that __builtin_is_constructible. It's a built-in. Of course >>> it's a built-in. It's a compiler-implemented trait, and >>> this is just the intrinsic that implements it. >> >> FWIW, I also like __is_constructible better than __builtin_is_constructible. > > So, updating all existing built-in trait names does not seem > realistic. I think there are two options: > > 1. As Jason said, we can use the same name as Clang does and otherwise > use __builtin. > 2. Or we can simply use __ for all built-in traits to keep consistency > with other built-in traits. > > Then, I feel option 2 would sound better since it's consistent across > all built-in type traits and it might confuse Clang when they > implement the same built-in. Also, it would be easier for me to > implement built-in traits as I don't need to dig into the Clang code > every time I add a new one. I agree, being consistent with the status-quo is valuable, some decisions might have not be the best ones - but I think it would be terribly confusing to mix __ and __builtin (it immediately makes the reader wonder whar the difference is). 0.02GBP only, as always Iain > >> >>> Most of the existing builtins for traits don't use a __builtin prefix. >>> Not in GCC, not in other compilers. They are indeed >>> just double-underscored versions of the traits. I think that's fine, >>> and consistent. There's precedent for this >>> across Embarcadero, Clang, MSVC, and GCC. See >>> https://clang.llvm.org/docs/LanguageExtensions.html >>> >>> Yes, I know it's inconsistent with other built-ins that aren't C++ >>> library traits. But the water's been flowing under >>> the bridge on that question for a while now. >>> >>> I would also prefer at least considering mimicking a trait builtin's >>> name if some other compiler did it first. That's not a hill >>> to die on, we don't need to be 100% compatible including the naming, >>> but if we can, we should just use a name that was >>> chosen by someone else already. It's just nice to have the same name >>> if the traits do exactly the same thing. If they don't, >>> then it's good and in fact very good to give our trait a different name. >>> >> >> Marek
On 5/2/24 15:36, Iain Sandoe wrote: > > >> On 2 May 2024, at 20:30, Ken Matsui <kmatsui@cs.washington.edu> wrote: >> >> On Thu, May 2, 2024 at 10:54 AM Marek Polacek <polacek@redhat.com> wrote: >>> >>> On Thu, May 02, 2024 at 08:37:53PM +0300, Ville Voutilainen wrote: >>>> On Thu, 2 May 2024 at 20:25, Ken Matsui <kmatsui@cs.washington.edu> wrote: >>>>>> There was some discussion of how to name the built-ins back in >>>>>> https://gcc.gnu.org/pipermail/gcc-patches/2007-March/thread.html#212171 >>>>>> but __builtin wasn't discussed. >>>>>> >>>>>> Apparently this naming convention follows the MSVC precedent: >>>>>> http://msdn2.microsoft.com/en-us/library/ms177194.aspx >>>>>> >>>>>> I notice some discussion of this pattern around Clang adding various >>>>>> built-ins in https://github.com/llvm/llvm-project/issues/61852 >>>>>> indicating that this is a policy based on precedent. >>>>>> >>>>>> But I don't see any actual reason for this pattern other than that it's >>>>>> what Paolo happened to do in 2007. >>>>>> >>>>>> I'm not sure what the right way forward is. Perhaps we're stuck with >>>>>> the questionable choices of the past. >>>>>> >>>>> >>>>> Hmm, I personally prefer the __builtin prefix. However, it seems that >>>>> we need to reach a consensus across MSVC, Clang, and GCC. Would this >>>>> be realistically possible? >>>>> >>>>> Until then, I think it would be better to use __ for all built-in >>>>> traits. What do you think? >>>> >>>> My 0.02: __builtin as a prefix doesn't serve much of a purpose. >>>> Consider __is_constructible. It doesn't add value >>>> to make that __builtin_is_constructible. It's a built-in. Of course >>>> it's a built-in. It's a compiler-implemented trait, and >>>> this is just the intrinsic that implements it. >>> >>> FWIW, I also like __is_constructible better than __builtin_is_constructible. >> >> So, updating all existing built-in trait names does not seem >> realistic. I think there are two options: >> >> 1. As Jason said, we can use the same name as Clang does and otherwise >> use __builtin. >> 2. Or we can simply use __ for all built-in traits to keep consistency >> with other built-in traits. >> >> Then, I feel option 2 would sound better since it's consistent across >> all built-in type traits and it might confuse Clang when they >> implement the same built-in. Also, it would be easier for me to >> implement built-in traits as I don't need to dig into the Clang code >> every time I add a new one. > > I agree, being consistent with the status-quo is valuable, some decisions > might have not be the best ones - but I think it would be terribly confusing > to mix __ and __builtin (it immediately makes the reader wonder whar the > difference is). This seems to be the prevailing sentiment, so let's continue that way. Thanks for the input. Jason >>>> Most of the existing builtins for traits don't use a __builtin prefix. >>>> Not in GCC, not in other compilers. They are indeed >>>> just double-underscored versions of the traits. I think that's fine, >>>> and consistent. There's precedent for this >>>> across Embarcadero, Clang, MSVC, and GCC. See >>>> https://clang.llvm.org/docs/LanguageExtensions.html >>>> >>>> Yes, I know it's inconsistent with other built-ins that aren't C++ >>>> library traits. But the water's been flowing under >>>> the bridge on that question for a while now. >>>> >>>> I would also prefer at least considering mimicking a trait builtin's >>>> name if some other compiler did it first. That's not a hill >>>> to die on, we don't need to be 100% compatible including the naming, >>>> but if we can, we should just use a name that was >>>> chosen by someone else already. It's just nice to have the same name >>>> if the traits do exactly the same thing. If they don't, >>>> then it's good and in fact very good to give our trait a different name. >>>> >>> >>> Marek >
On Thu, May 2, 2024 at 12:49 PM Jason Merrill <jason@redhat.com> wrote: > > On 5/2/24 15:36, Iain Sandoe wrote: > > > > > >> On 2 May 2024, at 20:30, Ken Matsui <kmatsui@cs.washington.edu> wrote: > >> > >> On Thu, May 2, 2024 at 10:54 AM Marek Polacek <polacek@redhat.com> wrote: > >>> > >>> On Thu, May 02, 2024 at 08:37:53PM +0300, Ville Voutilainen wrote: > >>>> On Thu, 2 May 2024 at 20:25, Ken Matsui <kmatsui@cs.washington.edu> wrote: > >>>>>> There was some discussion of how to name the built-ins back in > >>>>>> https://gcc.gnu.org/pipermail/gcc-patches/2007-March/thread.html#212171 > >>>>>> but __builtin wasn't discussed. > >>>>>> > >>>>>> Apparently this naming convention follows the MSVC precedent: > >>>>>> http://msdn2.microsoft.com/en-us/library/ms177194.aspx > >>>>>> > >>>>>> I notice some discussion of this pattern around Clang adding various > >>>>>> built-ins in https://github.com/llvm/llvm-project/issues/61852 > >>>>>> indicating that this is a policy based on precedent. > >>>>>> > >>>>>> But I don't see any actual reason for this pattern other than that it's > >>>>>> what Paolo happened to do in 2007. > >>>>>> > >>>>>> I'm not sure what the right way forward is. Perhaps we're stuck with > >>>>>> the questionable choices of the past. > >>>>>> > >>>>> > >>>>> Hmm, I personally prefer the __builtin prefix. However, it seems that > >>>>> we need to reach a consensus across MSVC, Clang, and GCC. Would this > >>>>> be realistically possible? > >>>>> > >>>>> Until then, I think it would be better to use __ for all built-in > >>>>> traits. What do you think? > >>>> > >>>> My 0.02: __builtin as a prefix doesn't serve much of a purpose. > >>>> Consider __is_constructible. It doesn't add value > >>>> to make that __builtin_is_constructible. It's a built-in. Of course > >>>> it's a built-in. It's a compiler-implemented trait, and > >>>> this is just the intrinsic that implements it. > >>> > >>> FWIW, I also like __is_constructible better than __builtin_is_constructible. > >> > >> So, updating all existing built-in trait names does not seem > >> realistic. I think there are two options: > >> > >> 1. As Jason said, we can use the same name as Clang does and otherwise > >> use __builtin. > >> 2. Or we can simply use __ for all built-in traits to keep consistency > >> with other built-in traits. > >> > >> Then, I feel option 2 would sound better since it's consistent across > >> all built-in type traits and it might confuse Clang when they > >> implement the same built-in. Also, it would be easier for me to > >> implement built-in traits as I don't need to dig into the Clang code > >> every time I add a new one. > > > > I agree, being consistent with the status-quo is valuable, some decisions > > might have not be the best ones - but I think it would be terribly confusing > > to mix __ and __builtin (it immediately makes the reader wonder whar the > > difference is). > > This seems to be the prevailing sentiment, so let's continue that way. > Thanks for the input. I actually found that we have two built-in type traits prefixed with __builtin: __builtin_is_corresponding_member and __builtin_is_pointer_interconvertible_with_class. Do we want to update these to use __ instead for consistency? > > Jason > > >>>> Most of the existing builtins for traits don't use a __builtin prefix. > >>>> Not in GCC, not in other compilers. They are indeed > >>>> just double-underscored versions of the traits. I think that's fine, > >>>> and consistent. There's precedent for this > >>>> across Embarcadero, Clang, MSVC, and GCC. See > >>>> https://clang.llvm.org/docs/LanguageExtensions.html > >>>> > >>>> Yes, I know it's inconsistent with other built-ins that aren't C++ > >>>> library traits. But the water's been flowing under > >>>> the bridge on that question for a while now. > >>>> > >>>> I would also prefer at least considering mimicking a trait builtin's > >>>> name if some other compiler did it first. That's not a hill > >>>> to die on, we don't need to be 100% compatible including the naming, > >>>> but if we can, we should just use a name that was > >>>> chosen by someone else already. It's just nice to have the same name > >>>> if the traits do exactly the same thing. If they don't, > >>>> then it's good and in fact very good to give our trait a different name. > >>>> > >>> > >>> Marek > > >
On Thu, May 02, 2024 at 12:52:59PM -0700, Ken Matsui wrote: > > This seems to be the prevailing sentiment, so let's continue that way. > > Thanks for the input. > > I actually found that we have two built-in type traits prefixed with > __builtin: __builtin_is_corresponding_member and That is a FE builtin, not a trait, and is very much different from the __is_* traits, is varargs with extra processing, I don't think any of the normal traits accepts pointer to members. > __builtin_is_pointer_interconvertible_with_class. Do we want to > update these to use __ instead for consistency? No, I think we want to keep them as is. Jakub
On Thu, May 2, 2024 at 1:09 PM Jakub Jelinek <jakub@redhat.com> wrote: > > On Thu, May 02, 2024 at 12:52:59PM -0700, Ken Matsui wrote: > > > This seems to be the prevailing sentiment, so let's continue that way. > > > Thanks for the input. > > > > I actually found that we have two built-in type traits prefixed with > > __builtin: __builtin_is_corresponding_member and > > That is a FE builtin, not a trait, and is very much different from the > __is_* traits, is varargs with extra processing, I don't think any of > the normal traits accepts pointer to members. > > > __builtin_is_pointer_interconvertible_with_class. Do we want to > > update these to use __ instead for consistency? > > No, I think we want to keep them as is. I see. Thank you. > > Jakub >
diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc index 000df847342..23ea66d9c12 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -3870,6 +3870,9 @@ diagnose_trait_expr (tree expr, tree args) case CPTK_IS_VOLATILE: inform (loc, " %qT is not a volatile type", t1); break; + case CPTK_RANK: + inform (loc, " %qT cannot yield a rank", t1); + break; case CPTK_REF_CONSTRUCTS_FROM_TEMPORARY: inform (loc, " %qT is not a reference that binds to a temporary " "object of type %qT (direct-initialization)", t1, t2); diff --git a/gcc/cp/cp-trait.def b/gcc/cp/cp-trait.def index 2d1cb7c227c..85056c8140b 100644 --- a/gcc/cp/cp-trait.def +++ b/gcc/cp/cp-trait.def @@ -99,6 +99,7 @@ DEFTRAIT_EXPR (IS_TRIVIALLY_COPYABLE, "__is_trivially_copyable", 1) DEFTRAIT_EXPR (IS_UNBOUNDED_ARRAY, "__is_unbounded_array", 1) DEFTRAIT_EXPR (IS_UNION, "__is_union", 1) DEFTRAIT_EXPR (IS_VOLATILE, "__is_volatile", 1) +DEFTRAIT_EXPR (RANK, "__rank", 1) DEFTRAIT_EXPR (REF_CONSTRUCTS_FROM_TEMPORARY, "__reference_constructs_from_temporary", 2) DEFTRAIT_EXPR (REF_CONVERTS_FROM_TEMPORARY, "__reference_converts_from_temporary", 2) DEFTRAIT_TYPE (REMOVE_ALL_EXTENTS, "__remove_all_extents", 1) diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc index 45dc509855a..7242db75248 100644 --- a/gcc/cp/semantics.cc +++ b/gcc/cp/semantics.cc @@ -12550,6 +12550,9 @@ trait_expr_value (cp_trait_kind kind, tree type1, tree type2) case CPTK_IS_DEDUCIBLE: return type_targs_deducible_from (type1, type2); + /* __rank is handled in finish_trait_expr. */ + case CPTK_RANK: + #define DEFTRAIT_TYPE(CODE, NAME, ARITY) \ case CPTK_##CODE: #include "cp-trait.def" @@ -12622,7 +12625,10 @@ finish_trait_expr (location_t loc, cp_trait_kind kind, tree type1, tree type2) if (processing_template_decl) { tree trait_expr = make_node (TRAIT_EXPR); - TREE_TYPE (trait_expr) = boolean_type_node; + if (kind == CPTK_RANK) + TREE_TYPE (trait_expr) = size_type_node; + else + TREE_TYPE (trait_expr) = boolean_type_node; TRAIT_EXPR_TYPE1 (trait_expr) = type1; TRAIT_EXPR_TYPE2 (trait_expr) = type2; TRAIT_EXPR_KIND (trait_expr) = kind; @@ -12714,6 +12720,7 @@ finish_trait_expr (location_t loc, cp_trait_kind kind, tree type1, tree type2) case CPTK_IS_UNBOUNDED_ARRAY: case CPTK_IS_UNION: case CPTK_IS_VOLATILE: + case CPTK_RANK: break; case CPTK_IS_LAYOUT_COMPATIBLE: @@ -12745,8 +12752,18 @@ finish_trait_expr (location_t loc, cp_trait_kind kind, tree type1, tree type2) gcc_unreachable (); } - tree val = (trait_expr_value (kind, type1, type2) - ? boolean_true_node : boolean_false_node); + tree val; + if (kind == CPTK_RANK) + { + size_t rank = 0; + for (; TREE_CODE (type1) == ARRAY_TYPE; type1 = TREE_TYPE (type1)) + ++rank; + val = build_int_cst (size_type_node, rank); + } + else + val = (trait_expr_value (kind, type1, type2) + ? boolean_true_node : boolean_false_node); + return maybe_wrap_with_location (val, loc); } diff --git a/gcc/testsuite/g++.dg/ext/has-builtin-1.C b/gcc/testsuite/g++.dg/ext/has-builtin-1.C index 3aca273aad6..7f7b27f7aa7 100644 --- a/gcc/testsuite/g++.dg/ext/has-builtin-1.C +++ b/gcc/testsuite/g++.dg/ext/has-builtin-1.C @@ -179,6 +179,9 @@ #if !__has_builtin (__is_volatile) # error "__has_builtin (__is_volatile) failed" #endif +#if !__has_builtin (__rank) +# error "__has_builtin (__rank) failed" +#endif #if !__has_builtin (__reference_constructs_from_temporary) # error "__has_builtin (__reference_constructs_from_temporary) failed" #endif diff --git a/gcc/testsuite/g++.dg/ext/rank.C b/gcc/testsuite/g++.dg/ext/rank.C new file mode 100644 index 00000000000..28894184387 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/rank.C @@ -0,0 +1,24 @@ +// { dg-do compile { target c++11 } } + +#include <cstddef> + +#define SA(X) static_assert((X),#X) + +class ClassType { }; + +SA(__rank(int) == 0); +SA(__rank(int[2]) == 1); +SA(__rank(int[][4]) == 2); +SA(__rank(int[2][2][4][4][6][6]) == 6); +SA(__rank(ClassType) == 0); +SA(__rank(ClassType[2]) == 1); +SA(__rank(ClassType[][4]) == 2); +SA(__rank(ClassType[2][2][4][4][6][6]) == 6); + +template<class T> void f(T) = delete; +void f(size_t); + +template<class T> +void g() { f(__rank(T)); } + +template void g<int>();
This patch implements built-in trait for std::rank. gcc/cp/ChangeLog: * cp-trait.def: Define __rank. * constraint.cc (diagnose_trait_expr): Handle CPTK_RANK. * semantics.cc (trait_expr_value): Likewise. (finish_trait_expr): Likewise. gcc/testsuite/ChangeLog: * g++.dg/ext/has-builtin-1.C: Test existence of __rank. * g++.dg/ext/rank.C: New test. Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org> --- gcc/cp/constraint.cc | 3 +++ gcc/cp/cp-trait.def | 1 + gcc/cp/semantics.cc | 23 ++++++++++++++++++++--- gcc/testsuite/g++.dg/ext/has-builtin-1.C | 3 +++ gcc/testsuite/g++.dg/ext/rank.C | 24 ++++++++++++++++++++++++ 5 files changed, 51 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/g++.dg/ext/rank.C