sammccall added inline comments.
================
Comment at: clangd/CodeComplete.cpp:558
+ if (const auto *NS = dyn_cast<NamespaceDecl>(Ctx))
+ return NS->getQualifiedNameAsString() + "::";
+ return llvm::None;
----------------
does this do the right thing if it's anonymous or inline, or a parent is?
For indexing, we call a function in AST.h, we should probably do something
similar.
The catch is that function relies on NamedDecl::printQualifiedName, maybe we
need to factor out the relevant part so we can call it on a DeclContext.
================
Comment at: clangd/CodeComplete.cpp:559
+ return NS->getQualifiedNameAsString() + "::";
+ return llvm::None;
+}
----------------
shouldn't this be ""? That's a definite scope, not a failure to find one.
(It's not a *namespace* scope, but I'm not sure why that's important)
================
Comment at: clangd/Quality.cpp:246
+float QueryScopeProximity::proximity(llvm::StringRef Scope) const {
+ if (QueryScopes.empty())
----------------
If you're going to decide these numbers directly...
a) I think you should just return a multiplier here, rather than a 0-1 score
b) we should more aggressively downrank non-preferred symbols: currently by
only ~1/3 vs symbols from a preferred scopes
e.g. I'd suggest returning 1, 2, 1.5, 1, 1, 1.5, 0.3 or similar
================
Comment at: clangd/Quality.cpp:301
+ // results can be tricky (e.g. class/function scope), and sema results are
+ // always in the accessible scope.
+ SemaScopeProximityScore = 1.0;
----------------
I don't think "always in the accessible scope" is sufficient justification to
give a score of 1.
However "we don't load top-level symbols from the preamble" might be?
================
Comment at: clangd/Quality.h:92
+ private:
+ std::vector<std::string> QueryScopes;
+};
----------------
hmm, can/should we use FileProximity for this, and just transform the strings?
This isn't going to cache for symbols sharing a namespace, isn't going to
handle "symbol is in a child namespace of an included namespace", and some
other combinations.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53131
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
