[PATCH] D38048: [clangd] Add textDocument/signatureHelp

[Raoul Wols via Phabricator via cfe-commits]

rwols marked 4 inline comments as done.
rwols added a comment.

There were some failing tests, probably because we use an extra digit for the `sortText` property now. I haven't touched those tests.



================
Comment at: clangd/ClangdUnit.cpp:610
+        ParameterInformation Info;
+        OptionalParameterLabel = getOptionalString(*Chunk.Optional);
+        Result.label += OptionalParameterLabel;
----------------
ilya-biryukov wrote:
> Are we first concatenating the `CodeCompletionString` inside optional chunks and then trying to parse them again here?
> Can we extract `ProcessChunks` function and recursively call it with `Chunk.Optional->Chunks`?
Yes, this was hokey. Fixed now (see the function `getOptionalParameters`)


================
Comment at: test/clangd/signature-help.test:39
+# I'm just putting the questionable result in here now as the expected result.
+# CHECK-DAG: {"label":"bar(float x = 0, int y = 42) -> void","parameters":[{"label":"float x = 0, int y = 42"}]}
+# 
----------------
ilya-biryukov wrote:
> rwols wrote:
> > When there are multiple defaulted parameters after each other, the CK_Optional chunk consists of *all* of those parameters, instead of a CK_Optional chunk per parameter. This might require us to dive into SemaCodeComplete.cpp to fix this. I'm just leaving it as-is right now.
> But does `CodeCompletionString` inside `Chunk->Optional` contain those extra parameters as a separate chunk?
> 
Yes, they do. Thanks for the idea!


https://reviews.llvm.org/D38048