On Wed, Jul 6, 2022 at 2:17 PM Jason Merrill <ja...@redhat.com> wrote: >> On 7/6/22 10:23, Lewis Hyatt wrote: > > Here is an updated patch addressing all your comments, thanks again for the > > review. Regarding the idea about rewinding tokens, it seems potentially > > problematic to try to make use of libcpp's backup token feature. I think > > there > > can be pragmas which allow macro expansion, and then libcpp is not able to > > step back through the expansion AFAIK. It would work fine for `#pragma GCC > > diagnostic' but would break if a similar approach was added for some other > > pragma allowing expansion in the future, and I think potentially also for > > _Pragma. But it seems pretty tenable to handle this case just by providing > > an > > interface in c-ppoutput.c to ask it to stream a given token that you have > > lexed external to the token streaming process. That's how I set it up here, > > it's much better than my first way IMHO, hopefully it looks OK? > > > > I also attached a diff of this version vs the previous version in case that > > makes it easier to review. The C++ parts were not touched other than adding > > the comment you suggested, the changes in this version are mostly in > > c-ppoutput.cc and c-pragma.cc. > > > > +/* When preprocessing only, pragma_lex() is not available, so obtain the > > tokens > > + directly from libcpp. We also need to inform the token streamer about > > all > > + tokens we lex ourselves here, so it outputs them too; this is done by > > calling > > + c_pp_stream_token () for each. */ > > Let's add to this comment > > ??? If we need to support more pragmas in the future, maybe initialize > this_parser with the pragma tokens and call pragma_lex? > > OK with that change. >
Thank you very much, done. -Lewis