ilya-biryukov marked 3 inline comments as done.
ilya-biryukov added inline comments.


================
Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:463
+  // Check all-scopes completions too.
+  Opts.AllScopes = true;
+  Results = completions(R"cpp(
----------------
kadircet wrote:
> I believe `AllScopes` and this feature is orthogonal what exactly is this 
> part of the test checking for?
There are two different code paths in code completion that trigger here:
1. one coming from `ParseOptionalCXXScopeSpecifier`, this is checked with 
`using ns::^`
2. one coming from `ParseUsingDeclarator`, this is checked with `using ^`.

I haven't checked, but the second one shouldn't provide completions from the 
same namespace, it's a bit more reliable long-term to assume we only provide 
results from other scopes.
Although maybe I'm overthinking, our index is definitely not smart enough to 
filter out results from the current scope in that situation (at least now)


================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5376
 
-  ResultBuilder Results(
-      *this, CodeCompleter->getAllocator(),
-      CodeCompleter->getCodeCompletionTUInfo(),
-      CodeCompletionContext(CodeCompletionContext::CCC_Symbol, PreferredType));
+  CodeCompletionContext CC(CodeCompletionContext::CCC_Symbol, PreferredType);
+  CC.setCXXScopeSpecifier(SS);
----------------
kadircet wrote:
> `CC` in here and above(in the `SS.isInvalid` case) seems to be the same, why 
> not use only a single one?
Good point, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69382/new/

https://reviews.llvm.org/D69382



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to