[PATCH] D116294: [CodeCompletion] (mostly) fix completion in incomplete C++ ctor initializers.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang/lib/Parse/ParseCXXInlineMethods.cpp:153
+      // probably truncated, so don't eat more tokens.
+      if (!Toks.back().is(tok::code_completion))
+        SkipMalformedDecl();
----------------
kadircet wrote:
> i don't follow the logic here. maybe i am reading the comment wrong, but we are actually going to eat **more** tokens by calling `SkipMalformedDecl`, possibly the following one, right? for example in a scenario like:
> ```
> struct Foo {
>   Foo : ^b
>   int bar;
> }
> ```
> `ConsumeAndStoreFunctionPrologue` will actually put `b` following the code completion token (`^`) into `Toks` as well, hence when we skip, we actually skip until the next semicolon and throw away bar. But when the code completion token is after `b`, `ConsumeAndStoreFunctionPrologue` we'll have code completion token at the end of the `Toks` and won't skip anything
> 
>  Do we have cases that break miserably when we don't perform an extra skip here for the (possible) reminder of current initalizer?
> i don't follow the logic here. maybe i am reading the comment wrong, 

Neither the code nor the comment are very good, but I think they are consistent.

Baseline behavior: we're going to recover by letting SkipMalformedDecl() eat tokens.
Exception: if we already ate the code completion token *and* stopped right afterwards.
Reason: CC token followed by heuristic stop are consistent with the function being truncated at the code completion point.

This exception allows some motivating testcases to pass. I thought maybe further improvements were possible but didn't want to get into them in this patch. However....

>  Do we have cases that break miserably when we don't perform an extra skip here for the (possible) reminder of current initalizer?

Um, apparently not. I thought I did!
Never skipping is simple and intuitive and makes more testcases pass.
Let's try it, the risk seems low.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116294