This came up in the context of dispatch patching, where the location
was very confusing (pointing to the first letter of the first variable).
The OpenMP variable-list parser has two modes, one is used to directly
parse a clause, in which case a tree node is created with the proper
location. And another one is used to just parse the list of variables.
As those are usually DECL_P, they don't have an expression location,
causing diagnostic issues.
In the parser functions, that's indicated by kind == 0, but all callers
use OMP_CLAUSE_ERROR (with value 0) for this. This patch now added a
couple of comments to make clear that 0 and OMP_CLAUSE_ERROR are the
same, which hopefully helps reading the code.
Examples:
foo.C:4:38: error: ‘threadprivate’ ‘int fff()’ is not file, namespace or block
scope variable
4 | #pragma omp threadprivate(fff, myvar)
| ^
New:
4 | #pragma omp threadprivate(fff, myvar)
| ^~~
Or even:
foo.C:3:59: error: ‘xxxx’ is specified more than once
3 | adjust_args(nothing: xxxx)
adjust_args(need_device_ptr:zzzz,yyyy,xxxx)
| ^~~~
New:
3 | adjust_args(nothing: xxxx)
adjust_args(need_device_ptr:zzzz,yyyy,xxxx)
| ^~~~
The new implementation stores the location in the TREE_VALUE part
of the three list; as it cannot sensible be stored directly, we
allocate memory just for this short-term purpose, using build_empty_stmt,
which seems to be a reasonable compromise.
Any comments before I commit it?
Tobias
PS: I have not updated the testcases to check for the column numbers, but
could do so, if deemed sensible.
OpenMP/C++: Store location in cp_parser_omp_var_list for kind=0
cp_parser_omp_var_list and cp_parser_omp_var_list_no_open have a special
modus: kind = 0 alias kind = OMP_CLAUSE_ERROR, which returns a simple tree
list; however, for a decl, no location is associated with that variable,
yielding to confusing error locations. With this patch, also for kind=0,
a reasonable error location is stored, albeit with creating a tree node
(build_empty_stmt), which is otherwise not used.
gcc/cp/ChangeLog:
* parser.cc (cp_parser_omp_var_list_no_open,
cp_parser_omp_var_list): For kind=0 (= OMP_CLAUSE_ERROR),
store also the expression location in the tree list.
(cp_parser_oacc_data_clause_deviceptr,
cp_finish_omp_declare_variant): Use that location instead or
input_location/the before-parsing location.
* semantics.cc (finish_omp_threadprivate): Likewise.
gcc/cp/parser.cc | 27 +++++++++++++--------------
gcc/cp/semantics.cc | 15 ++++++++-------
2 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 3ec9e414e62..0ee0e5b33e1 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -38698,8 +38698,9 @@ check_no_duplicate_clause (tree clauses, enum omp_clause_code code,
If KIND is nonzero, create the appropriate node and install the decl
in OMP_CLAUSE_DECL and add the node to the head of the list.
- 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.
COLON can be NULL if only closing parenthesis should end the list,
or pointer to bool which will receive false if the list is terminated
@@ -38836,7 +38837,7 @@ cp_parser_omp_var_list_no_open (cp_parser *parser, enum omp_clause_code kind,
goto build_clause;
}
token = cp_lexer_peek_token (parser->lexer);
- if (kind != 0
+ if (kind != 0 /* kind != OMP_CLAUSE_ERROR */
&& cp_parser_is_keyword (token, RID_THIS))
{
decl = finish_this_expr ();
@@ -38894,7 +38895,7 @@ cp_parser_omp_var_list_no_open (cp_parser *parser, enum omp_clause_code kind,
decl = process_outer_var_ref (decl, tf_warning_or_error);
if (decl == error_mark_node)
;
- else if (kind != 0)
+ else if (kind != 0) /* kind != OMP_CLAUSE_ERROR */
{
switch (kind)
{
@@ -39040,8 +39041,8 @@ cp_parser_omp_var_list_no_open (cp_parser *parser, enum omp_clause_code kind,
OMP_CLAUSE_CHAIN (u) = list;
list = u;
}
- else
- list = tree_cons (decl, NULL_TREE, list);
+ else /* kind == 0 alias kind == OMP_CLAUSE_ERROR */
+ list = tree_cons (decl, build_empty_stmt (token->location), list);
get_comma:
if (cp_lexer_next_token_is_not (parser->lexer, CPP_COMMA))
@@ -39088,17 +39089,17 @@ cp_parser_omp_var_list (cp_parser *parser, enum omp_clause_code kind, tree list,
{
if (parser->lexer->in_omp_decl_attribute)
{
+ location_t loc = cp_lexer_peek_token (parser->lexer)->location;
if (kind)
{
- location_t loc = cp_lexer_peek_token (parser->lexer)->location;
tree u = build_omp_clause (loc, kind);
OMP_CLAUSE_DECL (u) = parser->lexer->in_omp_decl_attribute;
OMP_CLAUSE_CHAIN (u) = list;
return u;
}
- else
- return tree_cons (parser->lexer->in_omp_decl_attribute, NULL_TREE,
- list);
+ else /* kind == OMP_CLAUSE_ERROR */
+ return tree_cons (parser->lexer->in_omp_decl_attribute,
+ build_empty_stmt (loc), list);
}
if (cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN))
@@ -39213,7 +39214,6 @@ cp_parser_oacc_data_clause (cp_parser *parser, pragma_omp_clause c_kind,
static tree
cp_parser_oacc_data_clause_deviceptr (cp_parser *parser, tree list)
{
- location_t loc = cp_lexer_peek_token (parser->lexer)->location;
tree vars, t;
/* Can't use OMP_CLAUSE_MAP here (that is, can't use the generic
@@ -39223,6 +39223,7 @@ cp_parser_oacc_data_clause_deviceptr (cp_parser *parser, tree list)
for (t = vars; t; t = TREE_CHAIN (t))
{
tree v = TREE_PURPOSE (t);
+ location_t loc = EXPR_LOCATION (TREE_VALUE (t));
tree u = build_omp_clause (loc, OMP_CLAUSE_MAP);
OMP_CLAUSE_SET_MAP_KIND (u, GOMP_MAP_FORCE_DEVICEPTR);
OMP_CLAUSE_DECL (u) = v;
@@ -50228,8 +50229,6 @@ cp_finish_omp_declare_variant (cp_parser *parser, cp_token *pragma_tok,
{
cp_lexer_consume_token (parser->lexer); // need_device_ptr
cp_lexer_consume_token (parser->lexer); // :
- location_t arg_loc
- = cp_lexer_peek_token (parser->lexer)->location;
tree arg;
tree list
@@ -50239,6 +50238,7 @@ cp_finish_omp_declare_variant (cp_parser *parser, cp_token *pragma_tok,
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++)
@@ -50253,7 +50253,6 @@ cp_finish_omp_declare_variant (cp_parser *parser, cp_token *pragma_tok,
arg = TREE_VALUE (arg);
if (adjust_args_list.contains (arg))
{
- // TODO fix location
error_at (arg_loc, "%qD is specified more than once",
decl);
continue;
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index e7ae9ce5ea4..15840e10620 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -10196,26 +10196,27 @@ finish_omp_threadprivate (tree vars)
for (t = vars; t; t = TREE_CHAIN (t))
{
tree v = TREE_PURPOSE (t);
+ location_t loc = EXPR_LOCATION (TREE_VALUE (t));
if (error_operand_p (v))
;
else if (!VAR_P (v))
- error ("%<threadprivate%> %qD is not file, namespace "
- "or block scope variable", v);
+ error_at (loc, "%<threadprivate%> %qD is not file, namespace "
+ "or block scope variable", v);
/* If V had already been marked threadprivate, it doesn't matter
whether it had been used prior to this point. */
else if (TREE_USED (v)
&& (DECL_LANG_SPECIFIC (v) == NULL
|| !CP_DECL_THREADPRIVATE_P (v)))
- error ("%qE declared %<threadprivate%> after first use", v);
+ error_at (loc, "%qE declared %<threadprivate%> after first use", v);
else if (! TREE_STATIC (v) && ! DECL_EXTERNAL (v))
- error ("automatic variable %qE cannot be %<threadprivate%>", v);
+ error_at (loc, "automatic variable %qE cannot be %<threadprivate%>", v);
else if (! COMPLETE_TYPE_P (complete_type (TREE_TYPE (v))))
- error ("%<threadprivate%> %qE has incomplete type", v);
+ error_at (loc, "%<threadprivate%> %qE has incomplete type", v);
else if (TREE_STATIC (v) && TYPE_P (CP_DECL_CONTEXT (v))
&& CP_DECL_CONTEXT (v) != current_class_type)
- error ("%<threadprivate%> %qE directive not "
- "in %qT definition", v, CP_DECL_CONTEXT (v));
+ error_at (loc, "%<threadprivate%> %qE directive not "
+ "in %qT definition", v, CP_DECL_CONTEXT (v));
else
{
/* Allocate a LANG_SPECIFIC structure for V, if needed. */
