| Message ID | 20150401112428.GL19273@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
On 04/01/2015 07:24 AM, Jakub Jelinek wrote: > Below is one possible fix for that, another option is introduce another > pfile->state or pfile field for that, I notice that pfile->state.skipping suppresses the line change as well as various diagnostics that also seem inappropriate for peeking. Perhaps we could use a new state field instead in those places, and only check state.skipping for the actual skipping behavior. But this patch is fine if you don't want to bother with that. Jason
On Wed, Apr 01, 2015 at 04:23:12PM -0400, Jason Merrill wrote: > On 04/01/2015 07:24 AM, Jakub Jelinek wrote: > >Below is one possible fix for that, another option is introduce another > >pfile->state or pfile field for that, > > I notice that pfile->state.skipping suppresses the line change as well as > various diagnostics that also seem inappropriate for peeking. Perhaps we > could use a new state field instead in those places, and only check > state.skipping for the actual skipping behavior. I was thinking about that, but e.g. warnings like warn_about_normalization only happen during _cpp_lex_direct and thus would not be reported at all if they were skipped during cpp_peek_token. Jakub
--- libcpp/lex.c.jj 2015-03-31 19:39:28.000000000 +0200 +++ libcpp/lex.c 2015-04-01 12:03:31.146846380 +0200 @@ -2080,6 +2080,12 @@ cpp_peek_token (cpp_reader *pfile, int i count = index; pfile->keep_tokens++; + /* For peeked tokens temporarily disable line_change reporting, + until the tokens are parsed for real. */ + void (*line_change) (cpp_reader *, const cpp_token *, int) + = pfile->cb.line_change; + pfile->cb.line_change = NULL; + do { peektok = _cpp_lex_token (pfile); @@ -2093,6 +2099,7 @@ cpp_peek_token (cpp_reader *pfile, int i _cpp_backup_tokens_direct (pfile, count - index); pfile->keep_tokens--; + pfile->cb.line_change = line_change; return peektok; } --- gcc/testsuite/gcc.target/powerpc/pr61977-1.c.jj 2015-04-01 12:15:15.341311939 +0200 +++ gcc/testsuite/gcc.target/powerpc/pr61977-1.c 2015-04-01 12:17:32.030102843 +0200 @@ -0,0 +1,8 @@ +/* PR preprocessor/61977 */ +/* { dg-do preprocess } */ +/* { dg-options "-mno-altivec -mno-vsx" } */ + +int y; vector +int x; + +/* { dg-final { scan-file "pr61977-1.i" "(^|\\n)int y; vector\\n" } } */ --- gcc/testsuite/gcc.target/powerpc/pr61977-2.c.jj 2015-04-01 12:15:18.125266339 +0200 +++ gcc/testsuite/gcc.target/powerpc/pr61977-2.c 2015-04-01 12:18:22.033321600 +0200 @@ -0,0 +1,8 @@ +/* PR preprocessor/61977 */ +/* { dg-do preprocess } */ +/* { dg-options "-maltivec" } */ + +int y; vector +int x; + +/* { dg-final { scan-file "pr61977-2.i" "(^|\\n)int y; __attribute__\\(\\(altivec\\(vector__\\)\\)\\)\\n" } } */
Hi! This is one possible way to fix another problem with the contextual macros. When preprocessing e.g. vector at the end of line (both with -mno-altivec and -maltivec), we cpp_peek_token at least the next token, and unfortunately during the peek we already call pfile->cb.line_change hook. So we get int y; \n emitted at that point, then e.g. when vector isn't expanded, we notice the current print.src_line is wrong for the token and thus emit \n## 5 "pr61977-1.c"\n vector and then when actually calling cpp_get_token_1 on the next token we emit the newline again from another pfile->cb.line_change hook. I believe we really should never call cb.line_change hook from within cpp_peek_token, the tokens are re-added to lookaheads and will be processed by _cpp_lex_token again at some point. Below is one possible fix for that, another option is introduce another pfile->state or pfile field for that, or another option would be to pass some flag from cpp_peek_token down to _cpp_lex_token. As cpp_peek_token is used very rarely, right now only in: 1) the contextual builtin macro stuff (powerpc*/spu only) 2) in __has_attribute/__has_cpp_attribute implementation 3) in genmatch build utility the attached patch, while perhaps not the cleanest, has the advantage of not affecting the more often used code path that might be performance critical. Ok for trunk if it passes testing on powerpc64{,le}-linux and {x86_64,i686}-linux? 2015-04-01 Jakub Jelinek <jakub@redhat.com> PR preprocessor/61977 * lex.c (cpp_peek_token): Temporarily clear pfile->cb.line_change. * gcc.target/powerpc/pr61977-1.c: New test. * gcc.target/powerpc/pr61977-2.c: New test. Jakub