From f74ed83e287dbaa20e9649df6cda631ee461ecf5 Mon Sep 17 00:00:00 2001 From: Sandra Loosemore Date: Tue, 11 Mar 2025 16:36:22 +0000 Subject: [PATCH] OpenMP/C: Store location in cp_parser_omp_var_list for kind=0 [PR118579] This patch is the C equivalent of commit r15-6512-gcf94ba812ca496 for C++, to improve the location information for individual items in an OpenMP variable list. gcc/c/ChangeLog PR c/118579 * c-parser.cc (c_parser_omp_variable_list): Capture location information when KIND is OMP_CLAUSE_ERROR. (c_parser_oacc_data_clause_deviceptr): Use the improved location for diagnostics, and remove the FIXME. (c_finish_omp_declare_variant): Likewise. (c_parser_omp_threadprivate): Likewise. gcc/testsuite/ChangeLog PR c/118579 * c-c++-common/gomp/pr118579.c: New testcase. --- gcc/c/c-parser.cc | 36 +++++++++------------- gcc/testsuite/c-c++-common/gomp/pr118579.c | 25 +++++++++++++++ 2 files changed, 39 insertions(+), 22 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/gomp/pr118579.c diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc index 7672e06fdd0..911e7edd481 100644 --- a/gcc/c/c-parser.cc +++ b/gcc/c/c-parser.cc @@ -16282,8 +16282,9 @@ c_parser_oacc_wait_list (c_parser *parser, location_t clause_loc, tree list) decl in OMP_CLAUSE_DECL and add the node to the head of the list. If KIND is nonzero, CLAUSE_LOC is the location of the clause. - If KIND is zero, create a TREE_LIST with the decl in TREE_PURPOSE; - return the list created. + If KIND is zero (= OMP_CLAUSE_ERROR), create a TREE_LIST with the decl + in TREE_PURPOSE and the location in TREE_VALUE (accessible using + EXPR_LOCATION); return the list created. The optional ALLOW_DEREF argument is true if list items can use the deref (->) operator. */ @@ -16312,6 +16313,7 @@ c_parser_omp_variable_list (c_parser *parser, while (1) { tree t = NULL_TREE; + location_t tloc = c_parser_peek_token (parser)->location; if (kind == OMP_CLAUSE_DEPEND || kind == OMP_CLAUSE_AFFINITY) { @@ -16512,7 +16514,7 @@ c_parser_omp_variable_list (c_parser *parser, if (t == error_mark_node) ; - else if (kind != 0) + else if (kind != 0) /* kind != OMP_CLAUSE_ERROR */ { switch (kind) { @@ -16686,8 +16688,8 @@ c_parser_omp_variable_list (c_parser *parser, list = u; } } - else - list = tree_cons (t, NULL_TREE, list); + else /* kind == OMP_CLAUSE_ERROR */ + list = tree_cons (t, build_empty_stmt (tloc), list); if (kind == OMP_CLAUSE_DEPEND || kind == OMP_CLAUSE_AFFINITY) { @@ -16847,7 +16849,6 @@ c_parser_oacc_data_clause (c_parser *parser, pragma_omp_clause c_kind, static tree c_parser_oacc_data_clause_deviceptr (c_parser *parser, tree list) { - location_t loc = c_parser_peek_token (parser)->location; tree vars, t; /* Can't use OMP_CLAUSE_MAP here (that is, can't use the generic @@ -16857,12 +16858,7 @@ c_parser_oacc_data_clause_deviceptr (c_parser *parser, tree list) for (t = vars; t && t; t = TREE_CHAIN (t)) { tree v = TREE_PURPOSE (t); - - /* FIXME diagnostics: Ideally we should keep individual - locations for all the variables in the var list to make the - following errors more precise. Perhaps - c_parser_omp_var_list_parens() should construct a list of - locations to go along with the var list. */ + location_t loc = EXPR_LOCATION (TREE_VALUE (t)); if (!VAR_P (v) && TREE_CODE (v) != PARM_DECL) error_at (loc, "%qD is not a variable", v); @@ -27031,6 +27027,7 @@ c_finish_omp_declare_variant (c_parser *parser, tree fndecl, tree parms) for (tree c = list; c != NULL_TREE; c = TREE_CHAIN (c)) { tree decl = TREE_PURPOSE (c); + location_t arg_loc = EXPR_LOCATION (TREE_VALUE (c)); int idx; for (arg = parms, idx = 0; arg != NULL; arg = TREE_CHAIN (arg), idx++) @@ -27038,14 +27035,15 @@ c_finish_omp_declare_variant (c_parser *parser, tree fndecl, tree parms) break; if (arg == NULL_TREE) { - error_at (loc, "%qD is not a function argument", + error_at (arg_loc, + "%qD is not a function argument", decl); goto fail; } if (adjust_args_list.contains (arg)) { - // TODO fix location - error_at (loc, "%qD is specified more than once", + error_at (arg_loc, + "%qD is specified more than once", decl); goto fail; } @@ -29495,19 +29493,13 @@ c_parser_omp_threadprivate (c_parser *parser) location_t loc; c_parser_consume_pragma (parser); - loc = c_parser_peek_token (parser)->location; vars = c_parser_omp_var_list_parens (parser, OMP_CLAUSE_ERROR, NULL); /* Mark every variable in VARS to be assigned thread local storage. */ for (t = vars; t; t = TREE_CHAIN (t)) { tree v = TREE_PURPOSE (t); - - /* FIXME diagnostics: Ideally we should keep individual - locations for all the variables in the var list to make the - following errors more precise. Perhaps - c_parser_omp_var_list_parens() should construct a list of - locations to go along with the var list. */ + loc = EXPR_LOCATION (TREE_VALUE (t)); /* If V had already been marked threadprivate, it doesn't matter whether it had been used prior to this point. */ diff --git a/gcc/testsuite/c-c++-common/gomp/pr118579.c b/gcc/testsuite/c-c++-common/gomp/pr118579.c new file mode 100644 index 00000000000..2a960858400 --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/pr118579.c @@ -0,0 +1,25 @@ +/* { dg-do compile } */ + +/* Make sure errors in variable-lists are diagnosed in the right place. */ + +void fvar(int *, int *); +#pragma omp declare variant(fvar) \ + match(construct={dispatch}) \ + adjust_args(need_device_ptr: yyy, xxx, xxx) +/* { dg-error "37: .xxx. is specified more than once" "" { target *-*-* } .-1 } */ +void f(int *xxx, int*yyy); + + +extern void frobnicate (int); +void g (int x, int y) +{ + int l = x + y; + static int s = 42; + frobnicate (s); +#pragma omp threadprivate (l, s) +/* { dg-error "28: automatic variable .l. cannot be .threadprivate." "" { target *-*-* } .-1 } */ +/* { dg-error "31: .s. declared .threadprivate. after first use" "" { target *-*-* } .-2 } */ + { + f (&l, &s); + } +}