[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.
Ilya Biryukov via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 23 05:22:19 PDT 2018
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
More information about the cfe-commits
mailing list