[clang] Introduce new completion context types

[kadircet]

Summary

New name suggestions were being used in places where existing names should have been used, this patch tries to fix some of those situations.

[kadircet]

• Use statement as context for using decls.

[sammccall]

lib/Sema/SemaCodeComplete.cpp

4841

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.

4897–4898

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 :-)

[kadircet]

• Use new types
• Rebase

[kadircet]

There are still some contexts where both a new name and an existing name is permissible, going to add a comment on those to leave some traces for the next person who cares

[kadircet]

• Add comments.

[sammccall]

include/clang/Sema/CodeCompleteConsumer.h

277

It's not obvious what "name" means here, e.g. what distinguishes this from "expression".

I think the answer is "we want the name of *something*, but we don't know what". I think it's ok to say that explicitly. Or to call this CCC_Symbol, etc.

286

what distinguishes potentiallyqualifiedexistingname from existingname?

The one use of ExistingName is the invalid scope case. This seems a) pretty marginal, and b) not to matter to clangd. Does it matter to libclang?

(The behavior is detectable through the API, but the current behavior doesn't seem that sensible or obviously important)

[kadircet]

• Use Symol, SymbolOrNewName, NewName
• Address comments.

[kadircet]

include/clang/Sema/CodeCompleteConsumer.h

277

Renaming to CCC_Symbol

286

It kind of matters to libclang, but it is not that important as well. It is used when caching completion items, to decide whether they allow a name specifiers or not. But when I think about the use case, I believe nested name specifiers should be allowed in that context(when scope is invalid, but we still report for global code completion). So seems like it is redundant(not exactly, but currently we have no way of telling it apart due to mentioned fixme).

[sammccall]

Logic looks good. Any way we can exercise it with a test via c-index-test?

include/clang/Sema/CodeCompleteConsumer.h

286

nit: try to describe what a symbol is in the comment, rather than just repeating the term?
e.g. "where an existing name is expected (such as a type, function, or variable)"

[kadircet]

• Update comment.

[kadircet]

Since it doesn't change any CXCompletionContext's in CIndex it doesn't seem possible. There are tests being added to clangd(D53192), since it uses context provided by sema to decide whether query the index or not, we can check the behavior.

[sammccall]

(sorry, thought I had accepted this!)

[nik]

I've bisected https://bugs.llvm.org/show_bug.cgi?id=42646 to this change. Reverting it fixes the issue.

Please look into it.

include/clang/Sema/CodeCompleteConsumer.h

enum Kind {
     /// Code completion occurred where a namespace or namespace alias
     /// is expected.
     CCC_Namespace,
 
     /// Code completion occurred where a type name is expected.
     CCC_Type,
 
     /// Code completion occurred where a new name is expected.
-    CCC_Name,
+    CCC_NewName,
+
+    /// Code completion occurred where an existing name is expected.
+    CCC_ExistingName,
 
     /// Code completion occurred where a new name is expected and a
     /// qualified name is permissible.
-    CCC_PotentiallyQualifiedName,
+    CCC_PotentiallyQualifiedNewName,
+
+    /// Code completion occurred where an existing name is expected and a
+    /// qualified name is permissible.
+    CCC_PotentiallyQualifiedExistingName,
 
     /// Code completion occurred where an macro is being defined.
     CCC_MacroName,
 
     /// Code completion occurred where a macro name is expected
     /// (without any arguments, in the case of a function-like macro).
     CCC_MacroNameUse,
 

lib/Frontend/ASTUnit.cpp

 void ASTUnit::enableSourceFileDiagnostics() {
   assert(getDiagnostics().getClient() && Ctx &&
       "Bad context for source file");
   getDiagnostics().getClient()->BeginSourceFile(Ctx->getLangOpts(), PP.get());
 }
 
 /// Determine the set of code-completion contexts in which this
 /// declaration should be shown.
-static unsigned getDeclShowContexts(const NamedDecl *ND,
+static uint64_t getDeclShowContexts(const NamedDecl *ND,
                                     const LangOptions &LangOpts,
                                     bool &IsNestedNameSpecifier) {
   IsNestedNameSpecifier = false;
 
   if (isa<UsingShadowDecl>(ND))
     ND = ND->getUnderlyingDecl();
   if (!ND)
     return 0;
case Result::RK_Declaration: {
           | (1LL << CodeCompletionContext::CCC_ClassStructUnion)
           | (1LL << CodeCompletionContext::CCC_Statement)
           | (1LL << CodeCompletionContext::CCC_Expression)
           | (1LL << CodeCompletionContext::CCC_ObjCMessageReceiver)
           | (1LL << CodeCompletionContext::CCC_EnumTag)
           | (1LL << CodeCompletionContext::CCC_UnionTag)
           | (1LL << CodeCompletionContext::CCC_ClassOrStructTag)
           | (1LL << CodeCompletionContext::CCC_Type)
-          | (1LL << CodeCompletionContext::CCC_PotentiallyQualifiedName)
+          | (1LL << CodeCompletionContext::CCC_PotentiallyQualifiedExistingName)
+          | (1LL << CodeCompletionContext::CCC_PotentiallyQualifiedNewName)
           | (1LL << CodeCompletionContext::CCC_ParenthesizedExpression);
 
         if (isa<NamespaceDecl>(R.Declaration) ||
             isa<NamespaceAliasDecl>(R.Declaration))
           NNSContexts |= (1LL << CodeCompletionContext::CCC_Namespace);
 
-        if (unsigned RemainingContexts
+        if (uint64_t RemainingContexts
                                 = NNSContexts & ~CachedResult.ShowInContexts) {
           // If there any contexts where this completion can be a
           // nested-name-specifier but isn't already an option, create a
           // nested-name-specifier completion.
           R.StartsNestedNameSpecifier = true;
           CachedResult.Completion = R.CreateCodeCompletionString(
               *TheSema, CCContext, *CachedCompletionAllocator, CCTUInfo,
               IncludeBriefCommentsInCodeCompletion);
static void CalculateHiddenNames(const CodeCompletionContext &Context,
   case CodeCompletionContext::CCC_Statement:
   case CodeCompletionContext::CCC_Expression:
   case CodeCompletionContext::CCC_ObjCMessageReceiver:
   case CodeCompletionContext::CCC_DotMemberAccess:
   case CodeCompletionContext::CCC_ArrowMemberAccess:
   case CodeCompletionContext::CCC_ObjCPropertyAccess:
   case CodeCompletionContext::CCC_Namespace:
   case CodeCompletionContext::CCC_Type:
-  case CodeCompletionContext::CCC_Name:
-  case CodeCompletionContext::CCC_PotentiallyQualifiedName:
+  case CodeCompletionContext::CCC_PotentiallyQualifiedExistingName:
+  case CodeCompletionContext::CCC_ExistingName:
   case CodeCompletionContext::CCC_ParenthesizedExpression:
   case CodeCompletionContext::CCC_ObjCInterfaceName:
     break;
 
   case CodeCompletionContext::CCC_EnumTag:
   case CodeCompletionContext::CCC_UnionTag:
   case CodeCompletionContext::CCC_ClassOrStructTag:
     OnlyTagNames = true;
     break;
 
   case CodeCompletionContext::CCC_ObjCProtocolName:
   case CodeCompletionContext::CCC_MacroName:
   case CodeCompletionContext::CCC_MacroNameUse:
   case CodeCompletionContext::CCC_PreprocessorExpression:
   case CodeCompletionContext::CCC_PreprocessorDirective:
   case CodeCompletionContext::CCC_NaturalLanguage:
   case CodeCompletionContext::CCC_SelectorName:
   case CodeCompletionContext::CCC_TypeQualifiers:
   case CodeCompletionContext::CCC_Other:
   case CodeCompletionContext::CCC_OtherWithMacros:
   case CodeCompletionContext::CCC_ObjCInstanceMessage:
   case CodeCompletionContext::CCC_ObjCClassMessage:
   case CodeCompletionContext::CCC_ObjCCategoryName:
   case CodeCompletionContext::CCC_IncludedFile:
+  case CodeCompletionContext::CCC_PotentiallyQualifiedNewName:
+  case CodeCompletionContext::CCC_NewName:
     // We're looking for nothing, or we're looking for names that cannot
     // be hidden.
     return;
   }
 
   using Result = CodeCompletionResult;
   for (unsigned I = 0; I != NumResults; ++I) {
     if (Results[I].Kind != Result::RK_Declaration)

lib/Sema/CodeCompleteConsumer.cpp

 
 bool CodeCompletionContext::wantConstructorResults() const {
   switch (CCKind) {
   case CCC_Recovery:
   case CCC_Statement:
   case CCC_Expression:
   case CCC_ObjCMessageReceiver:
   case CCC_ParenthesizedExpression:
+  case CCC_PotentiallyQualifiedExistingName:
+  case CCC_ExistingName:
     return true;
 
   case CCC_TopLevel:
   case CCC_ObjCInterface:
   case CCC_ObjCImplementation:
   case CCC_ObjCIvarList:
   case CCC_ClassStructUnion:
   case CCC_DotMemberAccess:
   case CCC_ArrowMemberAccess:
   case CCC_ObjCPropertyAccess:
   case CCC_EnumTag:
   case CCC_UnionTag:
   case CCC_ClassOrStructTag:
   case CCC_ObjCProtocolName:
   case CCC_Namespace:
   case CCC_Type:
-  case CCC_Name:
-  case CCC_PotentiallyQualifiedName:
+  case CCC_PotentiallyQualifiedNewName:
+  case CCC_NewName:
   case CCC_MacroName:
   case CCC_MacroNameUse:
   case CCC_PreprocessorExpression:
   case CCC_PreprocessorDirective:
   case CCC_NaturalLanguage:
   case CCC_SelectorName:
   case CCC_TypeQualifiers:
   case CCC_Other:
StringRef clang::getCompletionKindString(CodeCompletionContext::Kind Kind) {
   case CCKind::CCC_ClassOrStructTag:
     return "ClassOrStructTag";
   case CCKind::CCC_ObjCProtocolName:
     return "ObjCProtocolName";
   case CCKind::CCC_Namespace:
     return "Namespace";
   case CCKind::CCC_Type:
     return "Type";
-  case CCKind::CCC_Name:
-    return "Name";
-  case CCKind::CCC_PotentiallyQualifiedName:
-    return "PotentiallyQualifiedName";
+  case CCKind::CCC_NewName:
+    return "NewName";
+  case CCKind::CCC_ExistingName:
+    return "ExistingName";
+  case CCKind::CCC_PotentiallyQualifiedNewName:
+    return "PotentiallyQualifiedNewName";
+  case CCKind::CCC_PotentiallyQualifiedExistingName:
+    return "CCC_PotentiallyQualifiedExistingName";
   case CCKind::CCC_MacroName:
     return "MacroName";
   case CCKind::CCC_MacroNameUse:
     return "MacroNameUse";
   case CCKind::CCC_PreprocessorExpression:
     return "PreprocessorExpression";
   case CCKind::CCC_PreprocessorDirective:
     return "PreprocessorDirective";

lib/Sema/SemaCodeComplete.cpp

static void AddClassMessageCompletions(Sema &SemaRef, Scope *S,
                                        bool AtArgumentExpression,
                                        bool IsSuper,
                                        ResultBuilder &Results);
 
 void Sema::CodeCompleteDeclSpec(Scope *S, DeclSpec &DS,
                                 bool AllowNonIdentifiers,
                                 bool AllowNestedNameSpecifiers) {
   typedef CodeCompletionResult Result;
-  ResultBuilder Results(*this, CodeCompleter->getAllocator(),
-                        CodeCompleter->getCodeCompletionTUInfo(),
-                        AllowNestedNameSpecifiers
-                          ? CodeCompletionContext::CCC_PotentiallyQualifiedName
-                          : CodeCompletionContext::CCC_Name);
+  ResultBuilder Results(
+      *this, CodeCompleter->getAllocator(),
+      // FIXME: Try to separate codepath leading here to deduce whether we need
+      // an existing symbol or a new one. Current code leans towards new
+      // identifiers, since other case is rareish. Like, when we try to define
+      // an extern variable or initialize a static member.
+          ? CodeCompletionContext::CCC_PotentiallyQualifiedNewName
+          : CodeCompletionContext::CCC_NewName);
   Results.EnterNewScope();
 
   // Type qualifiers can come after names.
   Results.AddResult(Result("const"));
   Results.AddResult(Result("volatile"));
   if (getLangOpts().C99)
     Results.AddResult(Result("restrict"));
 
void Sema::CodeCompleteQualifiedId(Scope *S, CXXScopeSpec &SS,
   if (SS.isEmpty() || !CodeCompleter)
     return;
 
   // We want to keep the scope specifier even if it's invalid (e.g. the scope
   // "a::b::" is not corresponding to any context/namespace in the AST), since
   // it can be useful for global code completion which have information about
   // contexts/symbols that are not in the AST.
   if (SS.isInvalid()) {
-    CodeCompletionContext CC(CodeCompletionContext::CCC_Name);
+    CodeCompletionContext CC(CodeCompletionContext::CCC_ExistingName);
     CC.setCXXScopeSpecifier(SS);
     HandleCodeCompleteResults(this, CodeCompleter, CC, nullptr, 0);
     return;
   }
   // Always pretend to enter a context to ensure that a dependent type
   // resolves to a dependent record.
   DeclContext *Ctx = computeDeclContext(SS, /*EnteringContext=*/true);
   if (!Ctx)
     return;
 
   // Try to instantiate any non-dependent declaration contexts before
   // we look in them.
   if (!isDependentScopeSpecifier(SS) && RequireCompleteDeclContext(SS, Ctx))
     return;
 
-  ResultBuilder Results(*this, CodeCompleter->getAllocator(),
-                        CodeCompleter->getCodeCompletionTUInfo(),
-                        CodeCompletionContext::CCC_Name);
+  ResultBuilder Results(
+      *this, CodeCompleter->getAllocator(),
+      CodeCompletionContext::CCC_PotentiallyQualifiedExistingName);
   Results.EnterNewScope();
 
   // The "template" keyword can follow "::" in the grammar, but only
   // put it into the grammar if the nested-name-specifier is dependent.
   NestedNameSpecifier *NNS = SS.getScopeRep();
   if (!Results.empty() && NNS->isDependent())
     Results.AddResult("template");
 
void Sema::CodeCompleteQualifiedId(Scope *S, CXXScopeSpec &SS,
 
   HandleCodeCompleteResults(this, CodeCompleter, CC, Results.data(),
                             Results.size());
 }
 
 void Sema::CodeCompleteUsing(Scope *S) {
   if (!CodeCompleter)
     return;
 
-  ResultBuilder Results(*this, CodeCompleter->getAllocator(),
-                        CodeCompleter->getCodeCompletionTUInfo(),
-                        CodeCompletionContext::CCC_PotentiallyQualifiedName,
-                        &ResultBuilder::IsNestedNameSpecifier);
+  ResultBuilder Results(
+      *this, CodeCompleter->getAllocator(),
+      CodeCompletionContext::CCC_PotentiallyQualifiedExistingName,
   Results.EnterNewScope();
 
   // If we aren't in class scope, we could see the "namespace" keyword.
   if (!S->isClassScope())
     Results.AddResult(CodeCompletionResult("namespace"));
 
   // After "using", we can see anything that would start a
   // nested-name-specifier.
if (!ConstructorD)
     return;
 
   AdjustDeclIfTemplate(ConstructorD);
 
   CXXConstructorDecl *Constructor = dyn_cast<CXXConstructorDecl>(ConstructorD);
   if (!Constructor)
     return;
 
-  ResultBuilder Results(*this, CodeCompleter->getAllocator(),
-                        CodeCompleter->getCodeCompletionTUInfo(),
-                        CodeCompletionContext::CCC_PotentiallyQualifiedName);
+  ResultBuilder Results(
+      *this, CodeCompleter->getAllocator(),
+      CodeCompletionContext::CCC_PotentiallyQualifiedExistingName);
   Results.EnterNewScope();
 
   // Fill in any already-initialized fields or base classes.
   llvm::SmallPtrSet<FieldDecl *, 4> InitializedFields;
   llvm::SmallPtrSet<CanQualType, 4> InitializedBases;
   for (unsigned I = 0, E = Initializers.size(); I != E; ++I) {
     if (Initializers[I]->isBaseInitializer())
       InitializedBases.insert(

tools/libclang/CIndexCodeCompletion.cpp

switch (kind) {
     case CodeCompletionContext::CCC_ObjCProtocolName: {
       contexts = CXCompletionContext_ObjCProtocol;
       break;
     }
     case CodeCompletionContext::CCC_Namespace: {
       contexts = CXCompletionContext_Namespace;
       break;
     }
-    case CodeCompletionContext::CCC_PotentiallyQualifiedName: {
+    case CodeCompletionContext::CCC_PotentiallyQualifiedNewName:
+    case CodeCompletionContext::CCC_PotentiallyQualifiedExistingName: {
       contexts = CXCompletionContext_NestedNameSpecifier;
       break;
     }
     case CodeCompletionContext::CCC_MacroNameUse: {
       contexts = CXCompletionContext_MacroName;
       break;
     }
     case CodeCompletionContext::CCC_NaturalLanguage: {
switch (kind) {
     }
     case CodeCompletionContext::CCC_ObjCCategoryName: {
       contexts = CXCompletionContext_ObjCCategory;
       break;
     }
     case CodeCompletionContext::CCC_Other:
     case CodeCompletionContext::CCC_ObjCInterface:
     case CodeCompletionContext::CCC_ObjCImplementation:
-    case CodeCompletionContext::CCC_Name:
+    case CodeCompletionContext::CCC_NewName:
+    case CodeCompletionContext::CCC_ExistingName:
     case CodeCompletionContext::CCC_MacroName:
     case CodeCompletionContext::CCC_PreprocessorExpression:
     case CodeCompletionContext::CCC_PreprocessorDirective:
     case CodeCompletionContext::CCC_TypeQualifiers: {
       //Only Clang results should be accepted, so we'll set all of the other
       //context bits to 0 (i.e. the empty set)
       contexts = CXCompletionContext_Unexposed;
       break;