On Sat, Jul 20, 2024 at 02:42:24PM -0600, Sandra Loosemore wrote:
> + const char *old_name = IDENTIFIER_POINTER (name);
> + char *new_name = (char *) alloca (strlen (old_name) + 32);
XALLOCAVEC like for the C FE patch.
> + /* FIXME: I believe it is an unimplemented feature rather
> + than a user error to have non-constant expressions
> + inside "declare variant". */
> + t = metadirective_p
> + ? cp_parser_expression (parser)
> + : cp_parser_constant_expression (parser);
> if (t != error_mark_node)
> {
> t = fold_non_dependent_expr (t);
> - if (!value_dependent_expression_p (t)
> + if (!metadirective_p
> + && !value_dependent_expression_p (t)
> && (!INTEGRAL_TYPE_P (TREE_TYPE (t))
> || !tree_fits_shwi_p (t)))
> error_at (token->location, "property must be "
> "constant integer expression");
> + if (metadirective_p
> + && !INTEGRAL_TYPE_P (TREE_TYPE (t)))
Shouldn't this be && !type_dependent_expression_p (t) before the
!INTEGRAL_TYPE_P check?
I mean
template <typename T, T N>
void
foo ()
{
#pragma omp metadirective ... user={condition(N)} ...
...
}
should be valid, or just typename T and foo (T x) and condition(x).
> + error_at (token->location,
> + "property must be integer expression");
> --- a/gcc/cp/parser.h
> +++ b/gcc/cp/parser.h
> @@ -450,6 +450,13 @@ struct GTY(()) cp_parser {
> /* Pointer to state for parsing omp_loops. Managed by
> cp_parser_omp_for_loop in parser.cc and not used outside that file. */
> struct omp_for_parse_data * GTY((skip)) omp_for_parse_state;
> +
> + /* Set if we are processing a statement body associated with a
> + metadirective variant. */
> + bool in_metadirective_body;
> +
> + vec<tree> * GTY((skip)) metadirective_body_labels;
> + unsigned metadirective_region_num;
Again, there is unnecessary padding here (pointer, 8-bit bool, pointer,
32-bit unsigned) and maybe put the stuff into a separate structure and just
use a pointer to it? Like the omp_for_parse_state. Though, it is less
important than in the C++ FE.
> };
>
> /* In parser.cc */
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 108e929b8ee..109121be501 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -17851,6 +17851,79 @@ tsubst_omp_clauses (tree clauses, enum
> c_omp_region_type ort,
> return new_clauses;
> }
>
> +/* Like tsubst_copy, but specifically for OpenMP context selectors. */
> +static tree
> +tsubst_omp_context_selector (tree ctx, tree args, tsubst_flags_t complain,
> + tree in_decl)
> +{
> + tree new_ctx = NULL_TREE;
> + for (tree set = ctx; set; set = TREE_CHAIN (set))
> + {
> + tree selectors = NULL_TREE;
> + for (tree sel = OMP_TSS_TRAIT_SELECTORS (set); sel;
> + sel = TREE_CHAIN (sel))
> + {
> + enum omp_ts_code code = OMP_TS_CODE (sel);
> + tree properties = NULL_TREE;
> + tree score = OMP_TS_SCORE (sel);
> + tree t;
> +
> + if (score)
> + {
> + score = tsubst_expr (score, args, complain, in_decl);
> + score = fold_non_dependent_expr (score);
I think for partial template specialization processing_template_decl
can be true, so wonder if in that case it shouldn't again not diagnose
anything if score is still value dependent expression.
> + switch (omp_ts_map[OMP_TS_CODE (sel)].tp_type)
> + {
> + case OMP_TRAIT_PROPERTY_DEV_NUM_EXPR:
> + case OMP_TRAIT_PROPERTY_BOOL_EXPR:
> + t = tsubst_expr (OMP_TP_VALUE (OMP_TS_PROPERTIES (sel)),
> + args, complain, in_decl);
> + t = fold_non_dependent_expr (t);
> + if (!INTEGRAL_TYPE_P (TREE_TYPE (t)))
> + error_at (cp_expr_loc_or_input_loc (t),
> + "property must be integer expression");
And similarly here. Also, where do we do instantiation of the declare
variant selectors (if we do that at all)?
Jakub
