HighCommander4 wrote:

Thanks!

One question: is the new flag `CodeCompletionResult::IsAddressOfOperand` giving 
us anything that setting `DeclaringEntity=false` and `FunctionCanBeCall=false` 
wouldn't?

My other high-level piece of feedback is that I think we could implement some 
of this functionality more cleanly at the time a `CodeCompletionString` is 
constructed. Specifically, in 
[`CodeCompletionResult::createCodeCompletionStringForDecl`](https://searchfox.org/llvm/rev/b397c9d24196fe4103ad7cfe6bbfdfc08496364b/clang/lib/Sema/SemaCodeComplete.cpp#3748),
 we have access to flags such as `DeclaringEntity` and `FunctionCanBeCall` 
(they are fields of `this`), and we can pass them in to helpers such as 
`AddFunctionParameterChunks` and `AddFunctionTypeQualsToCompletionString` and 
let them influence the string chunks directly. For example:

 * If `DeclaringEntity=true`, instead of adding `CK_Placeholder` chunks for 
function parameter, we can create `CK_Text` chunks.
 * For parameters with default values, instead of adding the parameter and 
default value as a single chunk, we can add them as two separate chunks, with 
the default value being `CK_Informative`.
    * Clangd puts `CK_Informative` chunks into the signature but not the 
snippet, which is what we want.
 * For the function qualifiers, we can add them as `CK_Text` if 
`DeclaringEntity=true`, and otherwise as `CK_Informative`. (We don't need to 
introduce the new `CK_FunctionQualifier` chunk kind.)

With this approach, most (all?) changes to the clangd `getSignature` function 
should become unnecessary, and other consumers of the Sema code completion API 
(e.g. libclang) get to benefit from our improvements as well.

WDYT?

https://github.com/llvm/llvm-project/pull/165916
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to