ilya-biryukov added a comment.

One major limitation of the current workaround is that IIRC clang can actually 
call code completion callbacks, including this method, multiple times (e.g. 
when completing inside a macro definition that gets expanded later in the file 
or during parser recovery).
The workaround will work at the first call, but won't trigger at the repeated 
calls. We could find a way around that by resetting the flag to false at the 
right points (probably at every top-level function call?) WDYT?



================
Comment at: include/clang/Parse/Parser.h:217
 
+  /// Gets set to true after calling CodeCompleteCall, it is for a hack to make
+  /// signature help working even when it is triggered inside a token.
----------------
NIT: would go with something less harsh, like "**workaround** to make sure we 
only call CodeCompleteCall on the deepest call expression"


================
Comment at: lib/Parse/ParseExpr.cpp:1661
+          auto Completer = [&] {
+            Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs);
+            CalledOverloadCompletion = true;
----------------
Shouldn't we ignore `CodeCompleteCall` if `CalledOverloadCompletion` is set to 
true?


================
Comment at: lib/Parse/ParseExpr.cpp:1672
+            // let the parser do it instead.
+            if (!getLangOpts().ObjC1 && PP.isCodeCompletionReached() &&
+                !CalledOverloadCompletion)
----------------
What's the special handling in ObjC? Can we unify with our workaround?


================
Comment at: lib/Parse/ParseExpr.cpp:1674
+                !CalledOverloadCompletion)
+              Completer();
             LHS = ExprError();
----------------
Why don't we handle the `CalledOverloadCompletion` flag inside the 
`Completer()` itself?
We're currently special-casing the middle identifier and error case, why not 
have the same code path for `CodeCompleteCall()` calls at the start of the 
identifier?


Repository:
  rC Clang

https://reviews.llvm.org/D51038



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to