[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