[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

This revision was automatically updated to reflect the committed changes. Closed by commit rC361007: [Lex] Allow to consume tokens while preprocessing (authored by ibiryukov, committed by ). Changed prior to commit: https://reviews.llvm.org/D59885?vs=11=22#toc Repository: rC Clang

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov updated this revision to Diff 11. ilya-biryukov added a comment. - Fix compilation of a clang-tidy check, add a FIXME to stop reporting those tokens Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59885/new/

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

rsmith accepted this revision. rsmith marked an inline comment as done. rsmith added a comment. This revision is now accepted and ready to land. Thanks, LGTM! Comment at: clang/lib/Lex/Pragma.cpp:370 // Push the tokens onto the stack. - EnterTokenStream(TokArray,

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov added inline comments. Comment at: clang/lib/Lex/PPDirectives.cpp:1521 Tok[0].setAnnotationValue(AnnotationVal); - EnterTokenStream(std::move(Tok), 1, true); + EnterTokenStream(std::move(Tok), 1, true, /*IsReinject*/ true); } rsmith wrote: >

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov updated this revision to Diff 199775. ilya-biryukov marked 3 inline comments as done. ilya-biryukov added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59885/new/ https://reviews.llvm.org/D59885 Files:

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

rsmith added inline comments. Comment at: clang/lib/Lex/PPDirectives.cpp:1521 Tok[0].setAnnotationValue(AnnotationVal); - EnterTokenStream(std::move(Tok), 1, true); + EnterTokenStream(std::move(Tok), 1, true, /*IsReinject*/ true); } I think it'd be more

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov added inline comments. Comment at: clang/lib/Lex/Preprocessor.cpp:955 --LexLevel; + if (OnToken && LexLevel == 0 && !Result.getFlag(Token::IsReinjected)) +OnToken(Result); Could probably remove the `IsReinjected` check from here. It's fine

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov updated this revision to Diff 199639. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Improve comment, remove redundant braces Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59885/new/

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov added a comment. I've added a new parameter to `EnterToken` and `EnterTokenStream`. There are a bunch of interesting cases. `Preprocessor::AnnotateCachedTokens` is one. It consumes some `CachedTokens` (which are always considered re-injected) and replaces them with a single

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov updated this revision to Diff 199637. ilya-biryukov added a comment. Herald added a subscriber: eraman. - Properly mark tokens as reinjected where necessary. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59885/new/

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov added a comment. Will do. Also think reporting annotation tokens is ok, one can easily tell them apart and filter them by looking at the corresponding flags in `clang::Token`, will put it into the docs, though Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

rsmith added a comment. In D59885#1501774 , @ilya-biryukov wrote: > The suggested approach looks promising. The difference seems to be within the > noise levels on my machine: :) Awesome! > I guess it would make more sense to do the following before

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov updated this revision to Diff 199486. ilya-biryukov added a comment. - Remove the now redundant 'public:' spec. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59885/new/ https://reviews.llvm.org/D59885 Files:

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov added a comment. The suggested approach looks promising. The difference seems to be within the noise levels on my machine: Before the change (baseline upstream revision): Time (mean ± σ): 159.1 ms ± 8.7 ms[User: 138.3 ms, System: 20.6 ms] Range (min … max):

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov updated this revision to Diff 199485. ilya-biryukov added a comment. - Use a flag inside clang::Token instead Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59885/new/ https://reviews.llvm.org/D59885 Files:

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

rsmith added a comment. Thinking about this some more: distinguishing between "macro expansion" and other cases seems like a proxy for "this token came from inside the preprocessor / lexer" versus "this token was provided by the user", which is also exactly what `IsNewToken` is supposed to

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. I'll try to explore bringing the overhead down. The fact that `CachingLex` is happening at `LexLevel==1` might help, thanks for pointing that out! Comment at: clang/lib/Lex/PPCaching.cpp:64

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

rsmith added a comment. In D59885#1496734 , @ilya-biryukov wrote: > Overall, the performance cost in empty-callback case seems to be lower than > `5%`. This is significant, but hopefully acceptable. 5% is a lot to pay for something that we won't be

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov added a comment. Here are some numbers from running `clang -cc1 -Eonly` on `SemaExpr.cpp`, it includes a whole ton of headers, including a rather large `TreeTransform.h`. This was run on my machine, so keep in mind it's rather noisy. Minimal times should be somewhat

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

rsmith added inline comments. Comment at: clang/lib/Lex/Preprocessor.cpp:870-900 + TokenSource Source; do { +Source = TokenSource(); + switch (CurLexerKind) { case CLK_Lexer: + Source.InDirective = CurLexer->ParsingPreprocessorDirective;

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov updated this revision to Diff 198663. ilya-biryukov added a comment. - Get rid of an accidental change in how LexAfterModuleImport is handled Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59885/new/ https://reviews.llvm.org/D59885

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov added a comment. I've managed to get what I needed by returning a flag from `Lex`. @rsmith, could you take a look? Is there a better way to do the equivalent? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59885/new/

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov updated this revision to Diff 198662. ilya-biryukov added a comment. Herald added subscribers: jsji, kbarton, nemanjai. - Propagate whether the token came from a token stream through CachingLex. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov marked an inline comment as not done. ilya-biryukov added inline comments. Comment at: clang/lib/Lex/Preprocessor.cpp:956-957 --LexLevel; + if (OnToken) +OnToken(Result, Source); } ilya-biryukov wrote: > ilya-biryukov wrote: > > rsmith

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov marked an inline comment as not done. ilya-biryukov added inline comments. Comment at: clang/lib/Lex/Preprocessor.cpp:956-957 --LexLevel; + if (OnToken) +OnToken(Result, Source); } ilya-biryukov wrote: > rsmith wrote: > > This seems like

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov updated this revision to Diff 197778. ilya-biryukov added a comment. - Simplify the interface, get only the expanded token stream - An attempt to not report tokens from delayed parsing twice, almost works Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clang/lib/Lex/Preprocessor.cpp:956-957 --LexLevel; + if (OnToken) +OnToken(Result, Source); } rsmith wrote: > This seems like it's going to be extremely

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. In D59885#1485159 , @rsmith wrote: > I'd like to understand more about the intended use cases of this > functionality. What information do clients want? We are aiming to

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

rsmith added a comment. I'd like to understand more about the intended use cases of this functionality. What information do clients want? Comment at: clang/lib/Lex/Preprocessor.cpp:870-900 + TokenSource Source; do { +Source = TokenSource(); + switch

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov updated this revision to Diff 197278. ilya-biryukov added a comment. - Revamp TokenSource, make it more principled Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59885/new/ https://reviews.llvm.org/D59885 Files:

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov added inline comments. Comment at: clang/include/clang/Lex/Preprocessor.h:120 +/// callback that records tokens. +enum class TokenSource { + File, // a token coming directly from a file that is not a macro directive, This is supposed to provide

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov updated this revision to Diff 193901. ilya-biryukov added a comment. Pass an enum indicating where the token comes from. The enum is ad-hoc at the moment, will require some thought to turn it into a reasonable abstraction. The consumer of the token stream actually needs to be able

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov added a comment. I'm happy to assess the performance costs of this change if needed (the function is obviously on the hot path), but I hope this won't add any significant performance costs. Also happy to consider alternative approaches to collect the tokens when preprocessing,

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov created this revision. ilya-biryukov added a reviewer: rsmith. Herald added a project: clang. By adding a hook to consume all tokens produced by the preprocessor. The intention of this change is to make it possible to consume the expanded tokens without re-runnig the preprocessor