hokein added a comment.
Looks good mostly, a few nits. We should make sure all related comments are
updated accordingly
================
Comment at: clangd/CodeComplete.cpp:198
+static std::vector<CodeCompletionResult>
+getVirtualNonOverridenMethods(const DeclContext *DC, Sema *S) {
+ const auto *CR = llvm::dyn_cast<CXXRecordDecl>(DC);
----------------
Since we are returning `CodeCompletionResult`, maybe naming it like
`getNonOverridenCompleteionResults` is clearer?
================
Comment at: clangd/CodeComplete.cpp:222
+ const std::string Name = Method->getNameAsString();
+ const auto it = Overrides.find(Name);
+ bool IsOverriden = false;
----------------
nit: Can't we use `Overrides.find(Method->getName())` and the other places as
well?
================
Comment at: clangd/CodeComplete.cpp:224
+ bool IsOverriden = false;
+ if (it != Overrides.end())
+ for (auto *MD : it->second) {
----------------
nit: use `{}` around the body of `if` -- the one-line for statement is across
line, adding `{}` around it will improve the readability.
================
Comment at: clangd/CodeComplete.cpp:1238
: SymbolSlab();
// Merge Sema and Index results, score them, and pick the winners.
+ const auto Overrides = getVirtualNonOverridenMethods(
----------------
nit: we need to update the comment accordingly.
================
Comment at: clangd/CodeComplete.cpp:1281
// Groups overloads if desired, to form CompletionCandidate::Bundles.
// The bundles are scored and top results are returned, best to worst.
std::vector<ScoredBundle>
----------------
here as well.
================
Comment at: clangd/CodeComplete.cpp:1322
AddToBundles(&SemaResult, CorrespondingIndexResult(SemaResult));
+ for (auto &OverrideResult : OverrideResults)
+ AddToBundles(&OverrideResult, CorrespondingIndexResult(OverrideResult),
----------------
IIUC, we are treating the override results the same `SemaResult`, it is safe as
long as Sema doesn't provide these overrides in its code completion results,
otherwise we will have duplicated completion items?
I think we probably need a proper comment explaining what's going on here.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50898
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
