Hi PA,
* I believe that all prep + middle end patches are approved and ready to go
* Pending for C and C++ are the three patches, attached to the email
I am hereby replying to - for the C and the C++ front end and the
common C/C++ testcases.
→ Those 3 patches LGTM - thanks!
Related but not part of this review:
* Follow-up work regarding preserving the location_t,
(a) build_x_modify_expr and (b) cp_parser_omp_var_list_no_open
* The Fortran patch - see 5/7 for the patch + today's review comments
* Updating libgomp.texi (marking as implemented)
* * *
Paul-Antoine Arras:
+ /* TODO: build_x_modify_expr doesn't honor the location,
+ so we must set it here. */
+ expr.set_location (loc);
It is actually honored most of the time, e.g. viabuild_min_nt_loc or
build2_loc. However, for some template handling it doesn't.
Can you check whether the following works — and, if so, put it into
a separate patch and submit it to gcc-patches, CCing the two C++
maintainers, Jason and Nathan?
...
Will do.
Thanks!
* * *
I also noticed that there is a location issue. Both 'gcc' and g++' point
to the wrong variable (i.e. 'zzz' and not 'xxx' is underlined):
foo.c:2:123: error: ‘xxx’ is specified more than once
2 | #pragma omp declare variant(f) match(construct={dispatch})
adjust_args(need_device_ptr: xxx) adjust_args(need_device_ptr: zzz, xxx)
...
As discussed off-list, this will be handled by a separate, follow-up
patch. Left a TODO in the code.
Thanks.
(Discussed to store it in cp_parser_omp_var_list_no_open
for kind == 0alias kind == OMP_CLAUSE_ERROR in the value
part of tree_cons using 'build_empty_stmt (token->location)'.)
* * *
Otherwise, LGTM.
Now the C, C++ and C/C++ testsuite patches LGTM. :-)
Thanks!
Tobias
