[PATCH] D42181: [clangd] Merge index-provided completions with those from Sema.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

looks good to me too, just a few nits.



================
Comment at: clangd/CodeComplete.cpp:308
+    return None;
+  }
+}
----------------
add `return None` to make compiler happier? I think it might trigger -Wreturn-type warning.


================
Comment at: clangd/CodeComplete.cpp:341
+  CodeCompletionContext CCContext;
+  Sema *Sema = nullptr; // The Sema that created the results.
+
----------------
I'd suggesting rename the variable `Sema` to another name, since `Sema` is already a class name (although naming is hard).


================
Comment at: clangd/CodeComplete.cpp:845
+    for (const auto &IndexResult : IndexResults) {
+      if (UsedIndexResults.count(&IndexResult))
+        continue;
----------------
consider using `llvm::set_difference` here?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42181