[PATCH] D47957: [clangd] Add option to fold overloads into a single completion item.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Oh, sorry, I forgot to submit the comments yesterday :-(



================
Comment at: clangd/CodeComplete.cpp:245
+        // Methods are simply grouped by name.
+        return hash_combine('M', IndexResult->Name);
+      case index::SymbolKind::Function:
----------------
Members never come from the index for completion, right? Maybe add an assert here instead?


================
Comment at: clangd/CodeComplete.cpp:265
+                                   : StringRef(D->getDeclName().getAsString()));
+    return hash_combine('F', StringRef(D->getQualifiedNameAsString()),
+                        headerToInsertIfNotPresent().getValueOr(""));
----------------
`getQualifiedNameAsString` has a fixme, suggesting it's deprecated. Maybe we should call `printName` directly instead? And we have `Scratch` for storage anyway, so it might be a little faster too.


================
Comment at: clangd/CodeComplete.cpp:366
+        Opts.EnableSnippets ? (Name + "(${0})").str() : Name.str();
+    First.label = (Name + "(...)").str();
+    First.detail = llvm::formatv("[{0} overloads]", Bundle.size());
----------------
Maybe use a unicode ellipsis char (…) here?  Or even come up with something else. `...` is a valid syntax in C++, might be little confusing. Especially in VSCode, where detail is not shown.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47957