ioeric added a comment.
Looks like a great improvement on ranking!
================
Comment at: clangd/Quality.cpp:62
+ bool InClass;
+ for (const DeclContext *DC = D.getDeclContext(); DC != nullptr;
+ DC = DC->getParent()) {
----------------
nit: maybe stop when`!DC->isFileContext()`?
================
Comment at: clangd/Quality.cpp:66
+ return SymbolRelevanceSignals::FunctionScope;
+ InClass = InClass || isa<RecordDecl>(DC);
+ }
----------------
`DC->isRecord()`?
================
Comment at: clangd/Quality.cpp:70
+ return SymbolRelevanceSignals::ClassScope;
+ if (D.getLinkageInternal() < ExternalLinkage)
+ return SymbolRelevanceSignals::FileScope;
----------------
I wonder if this comparison is a common practice. Any reason not to use `!=`?
================
Comment at: clangd/Quality.cpp:71
+ if (D.getLinkageInternal() < ExternalLinkage)
+ return SymbolRelevanceSignals::FileScope;
+ return SymbolRelevanceSignals::GlobalScope;
----------------
Does `FileScope` indicate that `D` is defined in the current *translation
unit*? If so, I think this is a good signal (boosting symbols in the same TU),
but something like `TUScope` might be a bit clearer?
================
Comment at: clangd/Quality.cpp:76
+void SymbolRelevanceSignals::merge(const Symbol &IndexResult) {
+ // FIXME: Index results always assumed to be at global scope.
+}
----------------
What's here to be fixed? Do we plan to get rid of this assumption in the future?
================
Comment at: clangd/Quality.cpp:86
+ if (SemaCCResult.Kind == CodeCompletionResult::RK_Declaration)
+ Scope = std::min(Scope, ComputeScope(*SemaCCResult.Declaration));
}
----------------
nit: does this assume that "smaller" scopes are better? If so, it might make
sense to document.
================
Comment at: unittests/clangd/QualityTests.cpp:1
//===-- SourceCodeTests.cpp ------------------------------------*- C++
-*-===//
//
----------------
Could you also add a test case for code completion?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47762
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
