https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98072
Thomas Schwinge <tschwinge at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |dmalcolm at gcc dot gnu.org
--- Comment #3 from Thomas Schwinge <tschwinge at gcc dot gnu.org> ---
(In reply to Arseny Solokha from comment #0)
> g++-11.0.0-alpha20201129 snapshot
> (g:bb67ad5cff58a707aaae645d4f45a913d8511c86) ICEs when compiling the
> following testcase, reduced from test/OpenMP/depobj_ast_print.cpp from the
> clang 11.0.0 test suite, w/ -fopenmp:
Thanks for doing such testing.
> % g++-11.0.0 -fopenmp -c wmttbiko.cpp
> wmttbiko.cpp: In function 'void dh(int*, int, int)':
> wmttbiko.cpp:4:65: internal compiler error: in
> cp_parser_omp_var_list_no_open, at cp/parser.c:34843
> 4 | #pragma omp depobj (pm) depend (iterator (ca = 0 : *vp), in: vp[ca])
> | ^~
> 0x9e4b1b cp_parser_omp_var_list_no_open
>
> /var/tmp/portage/sys-devel/gcc-11.0.0_alpha20201129/work/gcc-11-20201129/
> gcc/cp/parser.c:34843
> 0x9ec406 cp_parser_omp_clause_depend
>
> /var/tmp/portage/sys-devel/gcc-11.0.0_alpha20201129/work/gcc-11-20201129/
> The failing assert has been introduced in
> g:c0c7270cc4efd896fe99f8ad5409dbef089a407f.
ACK, and that means (a) missing testsuite coverage, and (b) likely mis-handling
these clauses (at least concerning validity checking, diagnostics) -- so, I'm
glad that the 'assert' has found this additional case.
(In reply to Jakub Jelinek from comment #2)
> Created attachment 49656 [details]
> gcc11-pr98072.patch
>
> Untested fix.
ACK. I suggest you also add the standard source code comment that we use in
other similar places.
Alternatively (separate change), I wondered whether we might actually move the
sentinel into 'cp_parser_omp_var_list_no_open', supposing that's the one only
place where this matters? (But I haven't verified that.)
> The ultimate right fix is of course get rid of these
> sentinels from OpenMP/OpenACC parsing and deal with location wrappers when
> handling the clauses.
ACK. (Or, get rid of (this implementation of) these explicit location wrapper
nodes. I understand the intention behind the location wrappers, but maybe this
should be implemented differently?)
