[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