Enabling constructor code completion

[jkorous-apple]

Summary

It seems like constructor code completion was intentionally disabled ages ago in this commit (and refactored later):

commit 33224e61bfca370850abae89bbd415a4dabe07fa
Author: Douglas Gregor <[e-mail]>
Date: Fri Sep 18 17:42:29 2009 +0000

For code completion in C++ member access expressions and tag names,
look into the current scope for anything that could start a
nested-names-specifier. These results are ranked worse than any of the
results actually found in the lexical scope.

Perform a little more pruning of the result set, eliminating
constructors, __va_list_tag, and any duplication of declarations in
the result set. For the latter, implemented
NamespaceDecl::getCanonicalDecl.

I am not sure if there is actually anything relying on the fact that constructors are not included in code completion results. The only test that seems affected is probably not really concerned about these.

rdar://problem/19190970

[arphaman]

This approach doesn't look right. We don't want to code-complete constructors after the .. Instead we want to complete the constructors/destructors in bodies of classes and in out-of-line declarations after ~, right?

[jkorous-apple]

I agree and it seems to be the case but I will upgrade my ad-hoc testing code snippet to a proper test as I should have done it in the first place. I also agree that this change looks really suspicious and I was basically surprised that it works.

[jkorous-apple]

I am able to enable ctor completion selectively in translate unit scope and namespace scope so ctor completion does not appear in dot access context. I haven't figured out how to explicitly specify that completion context needs to be qualified id though.

[arphaman]

It doesn't look like this works in the body of the class (at least there's no test for it). For that you might have to inject these kind results by performing another lookup when completing inside the body of the class.

lib/Sema/SemaCodeComplete.cpp

980

You can use !isa<T>

[arphaman]

Also,

lib/Sema/SemaCodeComplete.cpp

980

Have you checked if this check works correctly in cases like this:

// main tu
int x = foo::<code-complete>
foo bar;
int x = bar.<code-complete>

I suspect the scope will be TranslationUnitDecl there as well.

[jkorous-apple]

Sorry, what do you mean by "this works in the body of the class"?

Definition of constructor inside class definition has been working even before:

struct foo {
  foo();
  f<CODE_COMPLETE_HERE>
};

Do you mean that a test shall be added in order to check that it still works?

[arphaman]

In D39730#926108, @jkorous-apple wrote:

Sorry, what do you mean by "this works in the body of the class"?

Definition of constructor inside class definition has been working even before:

struct foo {
  foo();
  f<CODE_COMPLETE_HERE>
};

Do you mean that a test shall be added in order to check that it still works?

You're right, but we don't actually code-complete constructor in this case, but the struct name. I suppose we don't need to complete the constructor here, but we do need the destructor, i.e.:

struct foo {
~<CODE_COMPLETE_HERE>
};

[jkorous-apple]

Sorry for delays, I'm working on this on and off when there's nothing more important.

Let me specify a little narrower scope of this change:

• Make code completion contain constructor item for out-of-line constructor definition: struct foo { foo(); }; foo::<CODE_COMPLETE> // result should contain constructor foo()

I would rather leave issues with destructors for other patch.

I haven't previously realized issue with initialization you mentioned and there are other cases where completing after id qualified with class prefix shouldn't end up containing constructors (e. g. typedef foo::some_embedded_type bar;)

I assume the proper way is to add constructors to code completion results only in these contexts:

• http://nongnu.org/hcb/#translation-unit
  • http://nongnu.org/hcb/#declaration-seq
    • http://nongnu.org/hcb/#declaration
      • http://nongnu.org/hcb/#function-definition
        • CODE_COMPLETE_CTOR
      • http://nongnu.org/hcb/#namespace-definition
        • http://nongnu.org/hcb/#named-namespace-definition
          • http://nongnu.org/hcb/#original-namespace-definition | http://nongnu.org/hcb/#extension-namespace-definition
            • http://nongnu.org/hcb/#namespace-body
              • http://nongnu.org/hcb/#declaration-seq
                • ...
                  • CODE_COMPLETE_CTOR
        • http://nongnu.org/hcb/#unnamed-namespace-definition
          • http://nongnu.org/hcb/#namespace-body
            • http://nongnu.org/hcb/#declaration-seq
              • ...
                • CODE_COMPLETE_CTOR

Which I think is equivalent to saying that the only parent nodes in ASTContext should either be NamespaceDecl or TranslationUnitDecl.

If you think my assumptions are wrong here please let me know.

Basically I need to distinguish between this callstack (foo::<CODE_COMPLETE>)

* frame  #0: 0x000000010bab9e7e libclang.dylib` clang::Sema::CodeCompleteQualifiedId(this=0x000000011d8b3a00, S=0x000000011d736fe0, SS=0x0000700008072538, EnteringContext=true, AddCtorsToResult=false)  + 46 at SemaCodeComplete.cpp:4550
  frame  #1: 0x000000010f9a8343 libclang.dylib` clang::Parser::ParseOptionalCXXScopeSpecifier(this=0x000000011d8b7a00, SS=0x0000700008072538, ObjectType=(Ptr = 0x0000000000000000), EnteringContext=true, MayBePseudoDestructor=0x0000000000000000, IsTypename=false, LastII=0x0000000000000000, OnlyNamespace=false)  + 1987 at ParseExprCXX.cpp:253
  frame  #2: 0x000000010fa34d69 libclang.dylib` clang::Parser::TryAnnotateCXXScopeToken(this=0x000000011d8b7a00, EnteringContext=true)  + 457 at Parser.cpp:1861
  frame  #3: 0x000000010f9466d7 libclang.dylib` clang::Parser::ParseDeclarationSpecifiers(this=0x000000011d8b7a00, DS=0x0000700008073738, TemplateInfo=0x0000700008073668, AS=AS_none, DSContext=DSC_top_level, LateAttrs=0x0000000000000000)  + 5111 at ParseDecl.cpp:3224
  frame  #4: 0x000000010fa2f63b libclang.dylib` clang::Parser::ParseDeclOrFunctionDefInternal(this=0x000000011d8b7a00, attrs=0x0000700008073d08, DS=0x0000700008073738, AS=AS_none)  + 139 at Parser.cpp:922
  frame  #5: 0x000000010fa2f152 libclang.dylib` clang::Parser::ParseDeclarationOrFunctionDefinition(this=0x000000011d8b7a00, attrs=0x0000700008073d08, DS=0x0000000000000000, AS=AS_none)  + 194 at Parser.cpp:1003
  frame  #6: 0x000000010fa2e27f libclang.dylib` clang::Parser::ParseExternalDeclaration(this=0x000000011d8b7a00, attrs=0x0000700008073d08, DS=0x0000000000000000)  + 3599 at Parser.cpp:853
  frame  #7: 0x000000010f965a38 libclang.dylib` clang::Parser::ParseInnerNamespace(this=0x000000011d8b7a00, IdentLoc=size=1, Ident=size=1, NamespaceLoc=size=1, index=0, InlineLoc=0x00007000080743b8, attrs=0x0000700008074300, Tracker=0x00007000080741c0)  + 312 at ParseDeclCXX.cpp:221
  frame  #8: 0x000000010f9651a4 libclang.dylib` clang::Parser::ParseNamespace(this=0x000000011d8b7a00, Context=0, DeclEnd=0x0000700008074e38, InlineLoc=(ID = 0))  + 8180 at ParseDeclCXX.cpp:196
  frame  #9: 0x000000010f944d59 libclang.dylib` clang::Parser::ParseDeclaration(this=0x000000011d8b7a00, Context=0, DeclEnd=0x0000700008074e38, attrs=0x0000700008075030)  + 505 at ParseDecl.cpp:1727
  frame #10: 0x000000010fa2ddc2 libclang.dylib` clang::Parser::ParseExternalDeclaration(this=0x000000011d8b7a00, attrs=0x0000700008075030, DS=0x0000000000000000)  + 2386 at Parser.cpp:786
  frame #11: 0x000000010fa2cab8 libclang.dylib` clang::Parser::ParseTopLevelDecl(this=0x000000011d8b7a00, Result=0x0000700008075190)  + 1240 at Parser.cpp:621
  frame #12: 0x000000010f92d143 libclang.dylib` clang::ParseAST(S=0x000000011d8b3a00, PrintStats=false, SkipFunctionBodies=false)  + 963 at ParseAST.cpp:147
  frame #13: 0x000000010b5bb755 libclang.dylib` clang::ASTFrontendAction::ExecuteAction(this=0x000000011d720de0)  + 485 at FrontendAction.cpp:1005
  frame #14: 0x000000010b5ba630 libclang.dylib` clang::FrontendAction::Execute(this=0x000000011d720de0)  + 112 at FrontendAction.cpp:904
  frame #15: 0x000000010b4962fb libclang.dylib` clang::ASTUnit::CodeComplete(this=0x000000011f001200, File=(Data = "/Users/jankorous/src/issues/ctor-dtor-autocomplete/test-ctor-dtor-completion/test-ctor-dtor-completion/main.cpp", Length = 111), Line=34, Column=8, RemappedFiles=ArrayRef<std::__1::pair<std::__1::basic_string<char>, llvm::MemoryBuffer *> > @ 0x0000700008076640, IncludeMacros=true, IncludeCodePatterns=false, IncludeBriefComments=false, Consumer=0x0000700008076b78, PCHContainerOps=std::__1::shared_ptr<clang::PCHContainerOperations>::element_type @ 0x000000011d720978 strong=3 weak=1, Diag=0x000000011d8a1a00, LangOpts=0x000000011d862f90, SourceMgr=0x000000011d72a950, FileMgr=0x000000011d722880, StoredDiagnostics=0x000000011d862c10, OwnedBuffers=0x000000011d863130)  + 9259 at ASTUnit.cpp:2209
  frame #16: 0x000000010aa8e29a libclang.dylib` clang_codeCompleteAt_Impl(TU=0x000000011d721d80, complete_filename="/Users/jankorous/src/issues/ctor-dtor-autocomplete/test-ctor-dtor-completion/test-ctor-dtor-completion/main.cpp", complete_line=34, complete_column=8, unsaved_files=ArrayRef<CXUnsavedFile> @ 0x00007000080768d8, options=1)  + 2746 at CIndexCodeCompletion.cpp:694
  frame #17: 0x000000010aa8cade libclang.dylib` clang_codeCompleteAt::$_0::operator(this=0x0000700007873bc0)() const  + 126 at CIndexCodeCompletion.cpp:804
  frame #18: 0x000000010aa93f65 libclang.dylib` void llvm::function_ref<void ()>::callback_fn<clang_codeCompleteAt::$_0>(callable=123145428614080)  + 21 at STLExtras.h:96
  frame #19: 0x000000010ebf34a9 libclang.dylib` llvm::function_ref<void ()>::operator(this=0x0000700008076e78)() const  + 25 at STLExtras.h:111
  frame #20: 0x000000010ebf344d libclang.dylib` llvm::CrashRecoveryContext::RunSafely(this=0x0000700007873bb0, Fn=function_ref<void ()> @ 0x0000700008076e78)>)  + 221 at CrashRecoveryContext.cpp:359
  frame #21: 0x000000010ebf36ff libclang.dylib` RunSafelyOnThread_Dispatch(UserData=0x0000700007873ab0)  + 79 at CrashRecoveryContext.cpp:402
  frame #22: 0x000000010ed2bb10 libclang.dylib` ExecuteOnThread_Dispatch(Arg=0x0000700007873a20)  + 48 at Threading.inc:53
  frame #23: 0x00007fff6c83c6c1 libsystem_pthread.dylib` _pthread_body  + 340
  frame #24: 0x00007fff6c83c56d libsystem_pthread.dylib` _pthread_start  + 377
  frame #25: 0x00007fff6c83bc5d libsystem_pthread.dylib` thread_start  + 13

and this callstack which is initialization (int x = foo::<CODE_COMPLETE>):

* frame  #0: 0x000000010bab9e7e libclang.dylib` clang::Sema::CodeCompleteQualifiedId(this=0x000000011f80dc00, S=0x000000011e10c940, SS=0x000070000c556838, EnteringContext=false, AddCtorsToResult=false)  + 46 at SemaCodeComplete.cpp:4550
  frame  #1: 0x000000010f9a8343 libclang.dylib` clang::Parser::ParseOptionalCXXScopeSpecifier(this=0x000000011f844800, SS=0x000070000c556838, ObjectType=(Ptr = 0x0000000000000000), EnteringContext=false, MayBePseudoDestructor=0x0000000000000000, IsTypename=false, LastII=0x0000000000000000, OnlyNamespace=false)  + 1987 at ParseExprCXX.cpp:253
  frame  #2: 0x000000010fa34ac2 libclang.dylib` clang::Parser::TryAnnotateTypeOrScopeToken(this=0x000000011f844800)  + 2610 at Parser.cpp:1736
  frame  #3: 0x000000010f999e46 libclang.dylib` clang::Parser::ParseCastExpression(this=0x000000011f844800, isUnaryExpression=false, isAddressOfOperand=false, NotCastExpr=0x000070000c559646, isTypeCast=NotTypeCast, isVectorLiteral=false)  + 14582 at ParseExpr.cpp:921
  frame  #4: 0x000000010f993de5 libclang.dylib` clang::Parser::ParseCastExpression(this=0x000000011f844800, isUnaryExpression=false, isAddressOfOperand=false, isTypeCast=NotTypeCast, isVectorLiteral=false)  + 101 at ParseExpr.cpp:521
  frame  #5: 0x000000010f992263 libclang.dylib` clang::Parser::ParseAssignmentExpression(this=0x000000011f844800, isTypeCast=NotTypeCast)  + 243 at ParseExpr.cpp:168
  frame  #6: 0x000000010f976980 libclang.dylib` clang::Parser::ParseInitializer(this=0x000000011f844800)  + 64 at Parser.h:1684
  frame  #7: 0x000000010f94d233 libclang.dylib` clang::Parser::ParseDeclarationAfterDeclaratorAndAttributes(this=0x000000011f844800, D=0x000070000c55a1c8, TemplateInfo=0x000070000c559f38, FRI=0x0000000000000000)  + 2323 at ParseDecl.cpp:2302
  frame  #8: 0x000000010f94b14e libclang.dylib` clang::Parser::ParseDeclGroup(this=0x000000011f844800, DS=0x000070000c55aab8, Context=0, DeclEnd=0x0000000000000000, FRI=0x0000000000000000)  + 2174 at ParseDecl.cpp:2064
  frame  #9: 0x000000010fa2fb18 libclang.dylib` clang::Parser::ParseDeclOrFunctionDefInternal(this=0x000000011f844800, attrs=0x000070000c55b030, DS=0x000070000c55aab8, AS=AS_none)  + 1384 at Parser.cpp:987
  frame #10: 0x000000010fa2f152 libclang.dylib` clang::Parser::ParseDeclarationOrFunctionDefinition(this=0x000000011f844800, attrs=0x000070000c55b030, DS=0x0000000000000000, AS=AS_none)  + 194 at Parser.cpp:1003
  frame #11: 0x000000010fa2e27f libclang.dylib` clang::Parser::ParseExternalDeclaration(this=0x000000011f844800, attrs=0x000070000c55b030, DS=0x0000000000000000)  + 3599 at Parser.cpp:853
  frame #12: 0x000000010fa2cab8 libclang.dylib` clang::Parser::ParseTopLevelDecl(this=0x000000011f844800, Result=0x000070000c55b190)  + 1240 at Parser.cpp:621
  frame #13: 0x000000010f92d143 libclang.dylib` clang::ParseAST(S=0x000000011f80dc00, PrintStats=false, SkipFunctionBodies=false)  + 963 at ParseAST.cpp:147
  frame #14: 0x000000010b5bb755 libclang.dylib` clang::ASTFrontendAction::ExecuteAction(this=0x000000011e1038a0)  + 485 at FrontendAction.cpp:1005
  frame #15: 0x000000010b5ba630 libclang.dylib` clang::FrontendAction::Execute(this=0x000000011e1038a0)  + 112 at FrontendAction.cpp:904
  frame #16: 0x000000010b4962fb libclang.dylib` clang::ASTUnit::CodeComplete(this=0x000000011f801200, File=(Data = "/Users/jankorous/src/issues/ctor-dtor-autocomplete/test-ctor-dtor-completion/test-ctor-dtor-completion/main.cpp", Length = 111), Line=52, Column=14, RemappedFiles=ArrayRef<std::__1::pair<std::__1::basic_string<char>, llvm::MemoryBuffer *> > @ 0x000070000c55c640, IncludeMacros=true, IncludeCodePatterns=false, IncludeBriefComments=false, Consumer=0x000070000c55cb78, PCHContainerOps=std::__1::shared_ptr<clang::PCHContainerOperations>::element_type @ 0x000000011e020a08 strong=3 weak=1, Diag=0x000000011f801a00, LangOpts=0x000000011f800f90, SourceMgr=0x000000011e1000f0, FileMgr=0x000000011e023640, StoredDiagnostics=0x000000011f800c10, OwnedBuffers=0x000000011f801130)  + 9259 at ASTUnit.cpp:2209
  frame #17: 0x000000010aa8e29a libclang.dylib` clang_codeCompleteAt_Impl(TU=0x000000011e020db0, complete_filename="/Users/jankorous/src/issues/ctor-dtor-autocomplete/test-ctor-dtor-completion/test-ctor-dtor-completion/main.cpp", complete_line=52, complete_column=14, unsaved_files=ArrayRef<CXUnsavedFile> @ 0x000070000c55c8d8, options=1)  + 2746 at CIndexCodeCompletion.cpp:694
  frame #18: 0x000000010aa8cade libclang.dylib` clang_codeCompleteAt::$_0::operator(this=0x000070000bd59bc0)() const  + 126 at CIndexCodeCompletion.cpp:804
  frame #19: 0x000000010aa93f65 libclang.dylib` void llvm::function_ref<void ()>::callback_fn<clang_codeCompleteAt::$_0>(callable=123145500859328)  + 21 at STLExtras.h:96
  frame #20: 0x000000010ebf34a9 libclang.dylib` llvm::function_ref<void ()>::operator(this=0x000070000c55ce78)() const  + 25 at STLExtras.h:111
  frame #21: 0x000000010ebf344d libclang.dylib` llvm::CrashRecoveryContext::RunSafely(this=0x000070000bd59bb0, Fn=function_ref<void ()> @ 0x000070000c55ce78)>)  + 221 at CrashRecoveryContext.cpp:359
  frame #22: 0x000000010ebf36ff libclang.dylib` RunSafelyOnThread_Dispatch(UserData=0x000070000bd59ab0)  + 79 at CrashRecoveryContext.cpp:402
  frame #23: 0x000000010ed2bb10 libclang.dylib` ExecuteOnThread_Dispatch(Arg=0x000070000bd59a20)  + 48 at Threading.inc:53
  frame #24: 0x00007fff6c83c6c1 libsystem_pthread.dylib` _pthread_body  + 340
  frame #25: 0x00007fff6c83c56d libsystem_pthread.dylib` _pthread_start  + 377
  frame #26: 0x00007fff6c83bc5d libsystem_pthread.dylib` thread_start  + 13

I tried using ASTContext::getParents() but for some reason I never get any parents for my Decl node.

if (isa<CXXConstructorDecl>(R.Declaration)) {
  assert(CurContext != nullptr &&
  "Code completion result builder should be provided with non-null CurContext");

  const Decl* const CurContextDecl = dyn_cast<Decl>(CurContext);
  if (CurContextDecl == nullptr)
    return;

  const auto ParentNodes = CurContext->getParentASTContext().getParents(*CurContextDecl);

  for(const auto ParentNode : ParentNodes) {
    const Decl* const ParentDecl = ParentNode.get<Decl>();
    if( ParentDecl == nullptr
        ||
        (
             !isa<TranslationUnitDecl>(ParentDecl) 
          && !isa<NamespaceDecl>(ParentDecl)
          && !isa<CXXConstructorDecl>(ParentDecl)
        )
    )
      return;
  }
}

I am currently looking into how and when is actually ASTContext produced and into getParents() and ParentMapASTVisitor implementation but have no idea what's wrong yet.

[jkorous-apple]

No luck. If I understand it correctly all the information that would be needed to distinguish between:

foo::<CODE_COMPLETE>

and

int i = foo::<CODE_COMPLETE>

in ResultBuilder::AddResult() is missing since the context is represented only by DeclContext. Something like a DirectASTParentNode would be necessary instead.

[arphaman]

I'll take a look, but my guess is that you might have to use scoping information

lib/Sema/SemaCodeComplete.cpp

AddResult(Result(Using->getTargetDecl(),
                      R.Qualifier),
               CurContext, Hiding);
     return;
   }
   
   bool AsNestedNameSpecifier = false;
   if (!isInterestingDecl(R.Declaration, AsNestedNameSpecifier))
     return;
-  
-  // C++ constructors are never found by name lookup.
-  if (isa<CXXConstructorDecl>(R.Declaration))
+  if (isa<CXXConstructorDecl>(R.Declaration)
+      && dyn_cast<NamespaceDecl>(CurContext) == nullptr
+      && dyn_cast<TranslationUnitDecl>(CurContext) == nullptr
+    )
     return;
 
   if (Hiding && CheckHiddenResult(R, CurContext, Hiding))
     return;
 
   // Make sure that any given declaration only shows up in the result set once.
   if (!AllDeclsFound.insert(R.Declaration->getCanonicalDecl()).second)
     return;

test/CodeCompletion/ctor.cpp

+struct foo {
+  foo();
+  ~foo();
+
+  foo(const foo&) = delete;
+  foo(foo&&) = delete;
+  foo& operator=(const foo&) = delete;
+  foo& operator=(foo&&) = delete;
+
+  static bool faz;
+  int far;
+};
+
+foo::foo() { }
+// RUN: c-index-test -code-completion-at=%s:14:7 %s | FileCheck -check-prefix=CHECK-TRANSLATION-UNIT %s
+// CHECK-TRANSLATION-UNIT:CXXConstructor:{TypedText foo}{LeftParen (}{RightParen )} (35)
+// CHECK-TRANSLATION-UNIT:CXXConstructor:{TypedText foo}{LeftParen (}{Placeholder const foo &}{RightParen )} (35) (unavailable)
+// CHECK-TRANSLATION-UNIT:CXXConstructor:{TypedText foo}{LeftParen (}{Placeholder foo &&}{RightParen )} (35) (unavailable)
+
+namespace ns_foo {
+struct foo {
+  foo();
+  ~foo();
+
+  foo(const foo&) = delete;
+  foo(foo&&) = delete;
+  foo& operator=(const foo&) = delete;
+  foo& operator=(foo&&) = delete;
+
+  static bool faz;
+  int far;
+};
+
+foo::foo() { }
+// RUN: c-index-test -code-completion-at=%s:34:7 %s | FileCheck -check-prefix=CHECK-NAMESPACE %s
+// CHECK-NAMESPACE: CXXConstructor:{TypedText foo}{LeftParen (}{RightParen )} (35)
+// CHECK-NAMESPACE: CXXConstructor:{TypedText foo}{LeftParen (}{Placeholder const ns_foo::foo &}{RightParen )} (35) (unavailable)
+// CHECK-NAMESPACE: CXXConstructor:{TypedText foo}{LeftParen (}{Placeholder ns_foo::foo &&}{RightParen )} (35) (unavailable)
+
+}
+
+int main() {
+  foo baz;
+  baz.far = 5;
+  foo::faz = 42;
+}
+// RUN: c-index-test -code-completion-at=%s:44:7 %s | not grep "CXXConstructor"
+// RUN: c-index-test -code-completion-at=%s:44:8 %s | not grep "CXXConstructor"
+// RUN: c-index-test -code-completion-at=%s:45:8 %s | not grep "CXXConstructor"
+// RUN: c-index-test -code-completion-at=%s:45:9 %s | not grep "CXXConstructor"