[PATCH] D49543: [clangd] Penalize non-instance members when accessed via class instances.
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 20 09:24:07 PDT 2018
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clangd/CodeComplete.h:148
bool HasMore = false;
+ CodeCompletionContext::Kind SemaContext = CodeCompletionContext::CCC_Other;
};
----------------
nit: "sema" is a bit down-in-the-weeds here, maybe just 'Context' or 'DetectedContext'?
================
Comment at: clangd/CodeComplete.h:148
bool HasMore = false;
+ CodeCompletionContext::Kind SemaContext = CodeCompletionContext::CCC_Other;
};
----------------
sammccall wrote:
> nit: "sema" is a bit down-in-the-weeds here, maybe just 'Context' or 'DetectedContext'?
can you add an a test to CodeCompleteTests for this new API?
================
Comment at: clangd/Quality.cpp:146
+ return false;
+ if (const auto *CM = dyn_cast<CXXMethodDecl>(ND))
+ return !CM->isStatic();
----------------
I think we also have to consider template functions too?
And ideally I think we want to exclude constructors (but include destructors).
================
Comment at: clangd/Quality.cpp:325
+ SemaContext == CodeCompletionContext::CCC_ArrowMemberAccess)) {
+ Score *= 0.5;
+ }
----------------
could be even harsher here, but this is fine for now
================
Comment at: clangd/Quality.h:102
+ CodeCompletionContext::Kind SemaContext = CodeCompletionContext::CCC_Other;
+
----------------
again, I'd drop "sema" here. It's not as though we can get several contexts from different sources.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D49543
More information about the cfe-commits
mailing list