[PATCH] D148997: [clang] Add a new annotation token: annot_repl_input_end

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D148997#4561516 <https://reviews.llvm.org/D148997#4561516>, @bnbarham wrote:

> In D148997#4561211 <https://reviews.llvm.org/D148997#4561211>, @v.g.vassilev wrote:
>
>>>>> 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).
>
> My other concern here is that it seems our use cases are somewhat different, eg. we wouldn't want any parsing differences and while I don't know why this is yet, the removal of:

I think I understand now. We basically want a mode which keeps some of the clang objects alive and ready to process more input. And on top, for clang-repl we want to enable some special parsing mode for the top-level statement declarations.

>   // Skip over the EOF token, flagging end of previous input for incremental
>   // processing
>   if (PP.isIncrementalProcessingEnabled() && Tok.is(tok::eof))
>     ConsumeToken();
>
> is causing issues for us as well.

Yes, that now seems to be moved on the user side as we thought it would be dead code. We can bring that one back if that's the only issue you see with that approach.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148997/new/

https://reviews.llvm.org/D148997