[PATCH] D50898: [clangd] Suggest code-completions for overriding base class virtual methods.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

I think it is reasonable to make Sema support suggesting override methods, instead of implementing it in clangd side?



================
Comment at: clangd/CodeComplete.cpp:206
+  llvm::StringMap<std::vector<FunctionDecl *>> Overrides;
+  for (auto *Method : dyn_cast<CXXRecordDecl>(DC)->methods()) {
+    if (!Method->isVirtual())
----------------
nit: CR->methods().


================
Comment at: clangd/CodeComplete.cpp:210
+    const std::string Name = Method->getNameAsString();
+    const auto it = Overrides.find(Name);
+    if (it == Overrides.end())
----------------
nit: we can simplify the code like `Overrides[Name].push_back(Method)`.


================
Comment at: clangd/CodeComplete.cpp:219
+  for (const auto &Base : CR->bases()) {
+    const auto *BR = Base.getType().getTypePtr()->getAsCXXRecordDecl();
+    for (auto *Method : BR->methods()) {
----------------
I think we should check whether `BR == nullptr` here.


================
Comment at: clangd/CodeComplete.cpp:1233
+    // struct/class/union definition.
+    const auto Overrides = getVirtualNonOverridenMethods(
+        Recorder->CCSema->CurContext, Recorder->CCSema);
----------------
It seems that we treat it as a special case, the code path here runs out of the `ranking` path.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50898