v.g.vassilev added a comment. In D148997#4561068 <https://reviews.llvm.org/D148997#4561068>, @bnbarham wrote:
> In D148997#4561022 <https://reviews.llvm.org/D148997#4561022>, @v.g.vassilev > wrote: > >> I meant that I'd like to figure out if we could use the >> `annot_repl_input_end` before considering a new flag. > > Oh sure, that's why I'm here :). Just trying to figure out what we think the > best way forward is. > >>> The issue is that all these actions (and the parser checks) can run with >>> and without `isIncrementalProcessingEnabled`, so they would need to check >>> both `eof` and `annot_repl_input_end`. For some concrete locations (but no >>> where near complete): >>> https://github.com/llvm/llvm-project/blob/df6b35e329ebecad6dc3bfb83183e482eb7a0020/clang/lib/Frontend/FrontendActions.cpp#L82 >>> https://github.com/llvm/llvm-project/blob/df6b35e329ebecad6dc3bfb83183e482eb7a0020/clang/lib/Frontend/FrontendActions.cpp#L955 >>> https://github.com/llvm/llvm-project/blob/df6b35e329ebecad6dc3bfb83183e482eb7a0020/clang/lib/Frontend/Rewrite/InclusionRewriter.cpp#L542 >> >> These three seem to be useful for `clang-repl` too, so we might want to >> extend it with like `!(eof || annot_repl_input_end)` > > Sure, though to be clear this isn't all of them, there's also the dump > actions too (though not sure that makes sense for `clang-repl`) The dump actions are also somewhat interesting because in that case we will dump the delta that was accumulated and repeat. and a few in other files (`VerifyDiagnosticConsumer.cpp`, `PrintPreprocessedOutput.cpp`). These ones are also interesting. >>> https://github.com/llvm/llvm-project/blob/df6b35e329ebecad6dc3bfb83183e482eb7a0020/clang/lib/Parse/ParseExprCXX.cpp#L4070 >> >> That looks a bit obscure to me. Looks like we are trying to reach some error >> recovery anchor but do you happen to have some use case at hand? In >> principle we could do the same as for the other 3. > > This was just one I picked at random from my grep over the parser. It's > unclear how often this would be hit, but I have to assume it (and the other > references) can, even if they're obscure/unlikely. My main point is that > `!eof` is being used somewhat frequently to end loops, but with this change > they will now all never end. Since `eof` there were several `eof`-like tokens that were added: `annot_module_begin`, `annot_module_end`, `tok::annot_module_include` which are commonly handled in `isEofOrEom`. We could update these uses with `isEofOrEom` but I am pretty sure that @rsmith will point out cases where that's not a good very good idea :) We could either experiment with using `isEofOrEom` or have a similar utility function only for the two tokens (say `isEoI` -- end of input whatever that means). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148997/new/ https://reviews.llvm.org/D148997 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
