[PATCH] D53191: [clang] Use Statement and Namespace instead of Name and PotentiallyQualifiedName
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 12 05:20:01 PDT 2018
sammccall added inline comments.
================
Comment at: lib/Sema/SemaCodeComplete.cpp:4825
if (SS.isInvalid()) {
- CodeCompletionContext CC(CodeCompletionContext::CCC_Name);
+ CodeCompletionContext CC(CodeCompletionContext::CCC_Statement);
CC.setCXXScopeSpecifier(SS);
----------------
I'm not sure statement is correct here.
First, `CCC_Expression` seems strictly better: this can't be `ns::while()` or such.
Secondly, ISTM there are other cases we might have a qualified ID in a non-expression context, like implementing a function `void foo::bar()` or so? Are we sure none of those hit this codepath?
No need to fix this now (this code is clearly an improvement) but maybe this is worth a comment like `// Usually qualified-IDs occur in expression context` or so.
================
Comment at: lib/Sema/SemaCodeComplete.cpp:4883
CodeCompleter->getCodeCompletionTUInfo(),
- CodeCompletionContext::CCC_PotentiallyQualifiedName,
+ CodeCompletionContext::CCC_Namespace,
&ResultBuilder::IsNestedNameSpecifier);
----------------
Hmm, I don't think `CCC_Namespace` is right either. This is a using declaration, not a using directive.
So ultimately we're completing a namespace member, not a namespace itself. But there's no enum value for that: `Expression` would be strings like `vector<T>` or `foo()`, and we want `vector` or `foo`.
I'd suggest either adding a new CCC enum value for this (PotentiallyQualifiedName is actually the best name for this! we'd need to come up with a new one), or use Expression with a comment that this isn't really accurate and someone should fix it :-)
Repository:
rC Clang
https://reviews.llvm.org/D53191
More information about the cfe-commits
mailing list