Message ID | 20190124194021.GS30353@tucnak |
---|---|
State | New |
Headers | show |
Series | Fix __has_include error recovery in libcpp (PR preprocessor/88974) | expand |
On 1/24/19 2:40 PM, Jakub Jelinek wrote: > Hi! > > On the following testcase, > _cpp_bracket_include -> glue_header_name emits > if (token->type == CPP_EOF) > { > cpp_error (pfile, CPP_DL_ERROR, "missing terminating > character"); > break; > } > which works, that last token has proper src_loc etc., but then > parse_has_include as _cpp_bracket_include caller will do cpp_get_token > (pfile) again and emit another error, which is both IMHO unnecessary when > we've emitted the earlier one, but also is a different CPP_EOF from the > earlier one and so it has uninitializer src_loc etc., so diagnostics on it > doesn't work. Looking around elsewhere in libcpp, we have in directives.c > #define SEEN_EOL() (pfile->cur_token[-1].type == CPP_EOF) > macro and if SEEN_EOL, we omit some diagnostics. > > If pfile->cur_token[-1].type is CPP_EOF, we must have emitted some error > already, either the: > cpp_error (pfile, CPP_DL_ERROR, > "operator \"__has_include__\" requires a header string"); > one or > cpp_error (pfile, CPP_DL_ERROR, "missing terminating > character"); > otherwise pfile->cur_token[-1].type must be CPP_GREATER, CPP_STRING or > CPP_HEADER_NAME, so there is no point getting another token when we know we > are at CPP_EOF and emitting another diagnostics. > > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for > trunk? > > 2019-01-24 Jakub Jelinek <jakub@redhat.com> > > PR preprocessor/88974 > * expr.c (parse_has_include): Don't cpp_get_token if EOL has been > seen. > > * c-c++-common/cpp/pr88974.c: New test. > > --- libcpp/expr.c.jj 2019-01-01 12:38:16.132007335 +0100 > +++ libcpp/expr.c 2019-01-24 14:07:10.080774120 +0100 > @@ -2238,7 +2238,9 @@ parse_has_include (cpp_reader *pfile, en > XDELETEVEC (fname); > } > > - if (paren && cpp_get_token (pfile)->type != CPP_CLOSE_PAREN) > + if (paren > + && pfile->cur_token[-1].type != CPP_EOF Is there a reason not to use the SEEN_EOL macro here, too (first moving it into a header)? Do we still give this diagnostic if there is a closing > but no )? Jason
On Thu, Jan 24, 2019 at 05:16:52PM -0500, Jason Merrill wrote: > > --- libcpp/expr.c.jj 2019-01-01 12:38:16.132007335 +0100 > > +++ libcpp/expr.c 2019-01-24 14:07:10.080774120 +0100 > > @@ -2238,7 +2238,9 @@ parse_has_include (cpp_reader *pfile, en > > XDELETEVEC (fname); > > } > > - if (paren && cpp_get_token (pfile)->type != CPP_CLOSE_PAREN) > > + if (paren > > + && pfile->cur_token[-1].type != CPP_EOF > > Is there a reason not to use the SEEN_EOL macro here, too (first moving it > into a header)? I can move it if you want to internal.h. > Do we still give this diagnostic if there is a closing > but no )? Yes: #if __has_include (<abc.h> #endif a.c:1:26: error: unterminated argument list invoking macro "__has_include" 1 | #if __has_include (<abc.h> | ^ pfile->cur_token[-1].type in that case is CPP_GREATER, or for __has_include ("abc.h" is CPP_STRING etc. Jakub
On 1/25/19 12:15 AM, Jakub Jelinek wrote: > On Thu, Jan 24, 2019 at 05:16:52PM -0500, Jason Merrill wrote: >>> --- libcpp/expr.c.jj 2019-01-01 12:38:16.132007335 +0100 >>> +++ libcpp/expr.c 2019-01-24 14:07:10.080774120 +0100 >>> @@ -2238,7 +2238,9 @@ parse_has_include (cpp_reader *pfile, en >>> XDELETEVEC (fname); >>> } >>> - if (paren && cpp_get_token (pfile)->type != CPP_CLOSE_PAREN) >>> + if (paren >>> + && pfile->cur_token[-1].type != CPP_EOF >> >> Is there a reason not to use the SEEN_EOL macro here, too (first moving it >> into a header)? > > I can move it if you want to internal.h. OK with that change. Jason
On Fri, Jan 25, 2019 at 09:31:18AM -0500, Jason Merrill wrote: > On 1/25/19 12:15 AM, Jakub Jelinek wrote: > > On Thu, Jan 24, 2019 at 05:16:52PM -0500, Jason Merrill wrote: > > > > --- libcpp/expr.c.jj 2019-01-01 12:38:16.132007335 +0100 > > > > +++ libcpp/expr.c 2019-01-24 14:07:10.080774120 +0100 > > > > @@ -2238,7 +2238,9 @@ parse_has_include (cpp_reader *pfile, en > > > > XDELETEVEC (fname); > > > > } > > > > - if (paren && cpp_get_token (pfile)->type != CPP_CLOSE_PAREN) > > > > + if (paren > > > > + && pfile->cur_token[-1].type != CPP_EOF > > > > > > Is there a reason not to use the SEEN_EOL macro here, too (first moving it > > > into a header)? > > > > I can move it if you want to internal.h. > > OK with that change. Here is what I've committed after another bootstrap/regtest on {x86_64,i686}-linux. Thanks. 2019-01-26 Jakub Jelinek <jakub@redhat.com> PR preprocessor/88974 * directives.c (SEEN_EOL): Move macro to ... * internal.h (SEEN_EOL): ... here. * expr.c (parse_has_include): Don't cpp_get_token if SEEN_EOL (). * c-c++-common/cpp/pr88974.c: New test. --- libcpp/directives.c.jj 2019-01-24 19:08:45.000000000 +0100 +++ libcpp/directives.c 2019-01-25 21:31:22.755442351 +0100 @@ -204,8 +204,6 @@ static const directive linemarker_dir = do_linemarker, UC"#", 1, KANDR, IN_I }; -#define SEEN_EOL() (pfile->cur_token[-1].type == CPP_EOF) - /* Skip any remaining tokens in a directive. */ static void skip_rest_of_line (cpp_reader *pfile) --- libcpp/internal.h.jj 2019-01-24 19:08:45.000000000 +0100 +++ libcpp/internal.h 2019-01-25 21:33:00.339379400 +0100 @@ -593,6 +593,8 @@ struct cpp_reader #define is_nvspace(x) IS_NVSPACE(x) #define is_space(x) IS_SPACE_OR_NUL(x) +#define SEEN_EOL() (pfile->cur_token[-1].type == CPP_EOF) + /* This table is constant if it can be initialized at compile time, which is the case if cpp was compiled with GCC >=2.7, or another compiler that supports C99. */ --- libcpp/expr.c.jj 2019-01-24 19:08:45.147042475 +0100 +++ libcpp/expr.c 2019-01-25 21:34:22.098500546 +0100 @@ -2238,7 +2238,7 @@ parse_has_include (cpp_reader *pfile, en XDELETEVEC (fname); } - if (paren && cpp_get_token (pfile)->type != CPP_CLOSE_PAREN) + if (paren && !SEEN_EOL () && cpp_get_token (pfile)->type != CPP_CLOSE_PAREN) cpp_error (pfile, CPP_DL_ERROR, "missing ')' after \"__has_include__\""); --- gcc/testsuite/c-c++-common/cpp/pr88974.c.jj 2019-01-24 14:25:02.271100711 +0100 +++ gcc/testsuite/c-c++-common/cpp/pr88974.c 2019-01-24 14:24:48.098334156 +0100 @@ -0,0 +1,6 @@ +/* PR preprocessor/88974 */ +/* { dg-do preprocess } */ + +#if __has_include (<pr88974.h) +/* { dg-error "missing terminating > character" "" { target *-*-* } .-1 } */ +#endif Jakub
--- libcpp/expr.c.jj 2019-01-01 12:38:16.132007335 +0100 +++ libcpp/expr.c 2019-01-24 14:07:10.080774120 +0100 @@ -2238,7 +2238,9 @@ parse_has_include (cpp_reader *pfile, en XDELETEVEC (fname); } - if (paren && cpp_get_token (pfile)->type != CPP_CLOSE_PAREN) + if (paren + && pfile->cur_token[-1].type != CPP_EOF + && cpp_get_token (pfile)->type != CPP_CLOSE_PAREN) cpp_error (pfile, CPP_DL_ERROR, "missing ')' after \"__has_include__\""); --- gcc/testsuite/c-c++-common/cpp/pr88974.c.jj 2019-01-24 14:25:02.271100711 +0100 +++ gcc/testsuite/c-c++-common/cpp/pr88974.c 2019-01-24 14:24:48.098334156 +0100 @@ -0,0 +1,6 @@ +/* PR preprocessor/88974 */ +/* { dg-do preprocess } */ + +#if __has_include (<pr88974.h) +/* { dg-error "missing terminating > character" "" { target *-*-* } .-1 } */ +#endif