[PATCH] D65866: Code completion should not ignore default parameters in functions.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang-tools-extra/clangd/CodeCompletionStrings.cpp:35
 
+void appendCodeCompletionString(const CodeCompletionString &CCS, std::string *Out) {
+  for (const CodeCompletionString::Chunk &C : CCS) {
----------------
maybe appendOptionalChunk? if in doubt, better to describe the semantics of the param than its type, since the type appears in the signature


================
Comment at: clang-tools-extra/clangd/CodeCompletionStrings.cpp:156
     case CodeCompletionString::CK_Optional:
+      assert(Chunk.Optional &&
+             "Expected the optional code completion string to be non-null.");      
----------------
please drop the extra message from the assert unless there's something additional to say (it just echoes the expression here)

(I don't have a strong opinion on this assert - I'm fine with keeping it but you could also drop it as we're enforcing someone else's clearly documented invariant)


================
Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:943
+
+TEST(CompletionTest, DefaultArgs) {
+  clangd::CodeCompleteOptions Opts;
----------------
for completeness, could you also add a unit test to CodeCompletionStringsTests.cpp?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65866