[continued] Add a way to get the CXCursor that corresponds to a completion result

[svenbrauch]

Summary

See https://reviews.llvm.org/D13599, I cannot update that because it has a different author.

Added a very simplistic auto test for the use case that motivated addition of the feature.

[milianw]

lgtm, but could we get some more tests rather than only this one for templates?

test/CodeCompletion/templates.cpp

28

isn't the source location going to be printed everywhere now? are really no other test files affected by this?

[svenbrauch]

I'll add some more, just wanted to post this to see if people think it's the right way to go.

Since the location is added at the end, all the other tests still pass, FileCheck just looks for the string and doesn't care about the extra suffix.

[svenbrauch]

I added a few more tests and found two issues:

• Cursors are also provided for implicit declarations. This is confusing and useless and shouldn't happen (e.g. for the implicit destructor, you don't want to get a cursor pointing to the class declaration). Instead, we return a null cursor now.
• clang_sortCodeCompletionResults sorts the results vector, but leaves the cursors vector untouched. That destroys the proper association between result and cursor ... instead, we now store the cursor in the result.

[svenbrauch]

Oh, one thing: I would like to remove the CursorKind member of the CXCompletionResult struct now since it is obsolete (same as clang_getCursorKind(Result->Cursor)). But, is that compatible with your API stability guarantees?

[milianw]

I don't think this upholds the ABI guarantee, but maybe I'm wrong? Can someone else chime in please? @klimek ?

include/clang-c/Index.h

4738

can you add context to the patch please? i.e. re-upload via arc?

I think what you are doing here breaks the ABI though, and that was the reason for why I used a separate vector the last time, afaik.

[svenbrauch]

Sorry for the wait.

Any opinions on whether this compatibility promises? If it does, I can also fix Milian's variant with the extra list but it will be a bit slower.

[milianw]

I'm pretty sure this is not ABI compatible. You change the size of CXCompletionResult, so when anyone is iterating over the array in CXCompletionResults it will break. If you don't want to, I can try to find some time to take this over again ;-)

[svenbrauch]

I know it breaks ABI compatiblity, I just don't know whether it needs to be kept (e.g. for clang-6.0). The alternative requires sorting by a separate index list which is a bit ugly (and slower). If you feel like working on it in the next few days that would of course be great, otherwise if it is clear that this solution doesn't work, I will pick it up again later. Thanks!

include/clang-c/Index.h

    * The cursor kind will be a macro, keyword, or a declaration (one of the
    * *Decl cursor kinds), describing the entity that the completion is
    * referring to.
-   *
-   * \todo In the future, we would like to provide a full cursor, to allow
-   * the client to extract additional information from declaration.
    */
   enum CXCursorKind CursorKind;
 
+  /**
+   * \brief The cursor (of kind CursorKind) for this result.
+   */
+  CXCursor Cursor;
+
   /**
    * \brief The code-completion string that describes how to insert this
    * code-completion result into the editing buffer.

   unsigned NumResults;
 } CXCodeCompleteResults;
 
+/**
+ * \brief Retrieve a cursor corresponding to the completion result at the given index.
+ *
+ * \param result The result for which the cursor should be obtained.
+ *
+ * \returns The cursor that represents the completion result at the given index,
+ *          or a null cursor when the result does not represent a cursor. This
+ *          will happen e.g. for language keywords.
+ */
+CINDEX_LINKAGE CXCursor
+clang_getCompletionCursor(CXCompletionResult *Result);
+
 /**
  * \brief Flags that can be passed to \c clang_codeCompleteAt() to
  * modify its behavior.

test/CodeCompletion/member-access.cpp

   // CHECK-CC1: memfun2 : [#void#][#Base3::#]memfun2(<#int#>)
   // CHECK-CC1: memfun3 : [#int#]memfun3(<#int#>)
 
+  // RUN: c-index-test -code-completion-at=%s:29:6 %s -o - |FileCheck -check-prefix=CHECK-CC2 %s
+  // CHECK-CC2: FieldDecl:{ResultType int}{Informative Base1::}{TypedText member1} (37) (source location: 2:7)
+  // CHECK-CC2: CXXMethod:{ResultType Base1 &}{Text Base1::}{TypedText operator=}{LeftParen (}{Placeholder const Base1 &}{RightParen )} (36) (source location: 0:0)
+  // CHECK-CC2: CXXDestructor:{ResultType void}{TypedText ~Derived}{LeftParen (}{RightParen )} (34) (source location: 0:0)
+
 // Make sure this doesn't crash
 // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:36:7 %s -verify
 

                               TemplateClass<int, S> *object2) {
   object.field;
   object2->field;
-// CHECK-CC2: baseTemplateField : [#T#][#BaseTemplate<T>::#]baseTemplateField
-// CHECK-CC2: baseTemplateFunction : [#T#][#BaseTemplate<T>::#]baseTemplateFunction()
-// CHECK-CC2: field : [#T#]field
-// CHECK-CC2: function : [#T#]function()
-// CHECK-CC2: member1 : [#int#][#Base1::#]member1
-// CHECK-CC2: member2 : [#float#][#Base1::#]member2
-// CHECK-CC2: overload1 : [#void#]overload1(<#const T &#>)
-// CHECK-CC2: overload1 : [#void#]overload1(<#const S &#>)
-
-// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:92:10 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
-// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:93:12 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
+// CHECK-CC3: baseTemplateField : [#T#][#BaseTemplate<T>::#]baseTemplateField
+// CHECK-CC3: baseTemplateFunction : [#T#][#BaseTemplate<T>::#]baseTemplateFunction()
+// CHECK-CC3: field : [#T#]field
+// CHECK-CC3: function : [#T#]function()
+// CHECK-CC3: member1 : [#int#][#Base1::#]member1
+// CHECK-CC3: member2 : [#float#][#Base1::#]member2
+// CHECK-CC3: overload1 : [#void#]overload1(<#const T &#>)
+// CHECK-CC3: overload1 : [#void#]overload1(<#const S &#>)
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:97:10 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:98:12 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
 }
 
 

                                          TemplateClass<int, double> *object2) {
   object.field;
   object2->field;
-// CHECK-CC3: baseTemplateField : [#int#][#BaseTemplate<int>::#]baseTemplateField
-// CHECK-CC3: baseTemplateFunction : [#int#][#BaseTemplate<int>::#]baseTemplateFunction()
-// CHECK-CC3: field : [#int#]field
-// CHECK-CC3: function : [#int#]function()
-// CHECK-CC3: member1 : [#int#][#Base1::#]member1
-// CHECK-CC3: member2 : [#float#][#Base1::#]member2
-// CHECK-CC3: overload1 : [#void#]overload1(<#const int &#>)
-// CHECK-CC3: overload1 : [#void#]overload1(<#const double &#>)
-
-// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:110:10 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
-// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:111:12 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
+// CHECK-CC4: baseTemplateField : [#int#][#BaseTemplate<int>::#]baseTemplateField
+// CHECK-CC4: baseTemplateFunction : [#int#][#BaseTemplate<int>::#]baseTemplateFunction()
+// CHECK-CC4: field : [#int#]field
+// CHECK-CC4: function : [#int#]function()
+// CHECK-CC4: member1 : [#int#][#Base1::#]member1
+// CHECK-CC4: member2 : [#float#][#Base1::#]member2
+// CHECK-CC4: overload1 : [#void#]overload1(<#const int &#>)
+// CHECK-CC4: overload1 : [#void#]overload1(<#const double &#>)
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:115:10 %s -o - | FileCheck -check-prefix=CHECK-CC4 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:116:12 %s -o - | FileCheck -check-prefix=CHECK-CC4 %s
 }
 
 template <typename T>

 
   void function() {
     o1.baseTemplateField;
-// CHECK-CC4: BaseTemplate : BaseTemplate::
-// CHECK-CC4: baseTemplateField : [#int#]baseTemplateField
-// CHECK-CC4: baseTemplateFunction : [#int#]baseTemplateFunction()
-// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:132:8 %s -o - | FileCheck -check-prefix=CHECK-CC4 %s
-    o2.baseTemplateField;
 // CHECK-CC5: BaseTemplate : BaseTemplate::
-// CHECK-CC5: baseTemplateField : [#T#]baseTemplateField
-// CHECK-CC5: baseTemplateFunction : [#T#]baseTemplateFunction()
+// CHECK-CC5: baseTemplateField : [#int#]baseTemplateField
+// CHECK-CC5: baseTemplateFunction : [#int#]baseTemplateFunction()
 // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:137:8 %s -o - | FileCheck -check-prefix=CHECK-CC5 %s
+    o2.baseTemplateField;
+// CHECK-CC6: BaseTemplate : BaseTemplate::
+// CHECK-CC6: baseTemplateField : [#T#]baseTemplateField
+// CHECK-CC6: baseTemplateFunction : [#T#]baseTemplateFunction()
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:142:8 %s -o - | FileCheck -check-prefix=CHECK-CC6 %s
     this->o1;
-// CHECK-CC6: [#void#]function()
-// CHECK-CC6: o1 : [#BaseTemplate<int>#]o1
-// CHECK-CC6: o2 : [#BaseTemplate<T>#]o2
-// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:142:11 %s -o - | FileCheck -check-prefix=CHECK-CC6 %s
+// CHECK-CC7: [#void#]function()
+// CHECK-CC7: o1 : [#BaseTemplate<int>#]o1
+// CHECK-CC7: o2 : [#BaseTemplate<T>#]o2
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:147:11 %s -o - | FileCheck -check-prefix=CHECK-CC7 %s
   }
 
   static void staticFn(T &obj);

 template<typename T>
 void dependentColonColonCompletion() {
   Template<T>::staticFn();
-// CHECK-CC7: function : [#void#]function()
-// CHECK-CC7: Nested : Nested
-// CHECK-CC7: o1 : [#BaseTemplate<int>#]o1
-// CHECK-CC7: o2 : [#BaseTemplate<T>#]o2
-// CHECK-CC7: staticFn : [#void#]staticFn(<#T &obj#>)
-// CHECK-CC7: Template : Template
-// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:156:16 %s -o - | FileCheck -check-prefix=CHECK-CC7 %s
+// CHECK-CC8: function : [#void#]function()
+// CHECK-CC8: Nested : Nested
+// CHECK-CC8: o1 : [#BaseTemplate<int>#]o1
+// CHECK-CC8: o2 : [#BaseTemplate<T>#]o2
+// CHECK-CC8: staticFn : [#void#]staticFn(<#T &obj#>)
+// CHECK-CC8: Template : Template
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:161:16 %s -o - | FileCheck -check-prefix=CHECK-CC8 %s
   typename Template<T>::Nested m;
-// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:164:25 %s -o - | FileCheck -check-prefix=CHECK-CC7 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:169:25 %s -o - | FileCheck -check-prefix=CHECK-CC8 %s
 }

test/CodeCompletion/operator.cpp

   // CHECK-CC1: N
   // CHECK-CC1: short
   // CHECK-CC1: T
+
+  // RUN: c-index-test -code-completion-at=%s:10:11 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
+  // CHECK-CC2: ClassDecl:{TypedText T} (50) (source location: 1:7)
+  // CHECK-CC2: NotImplemented:{TypedText |} (40) (source location: 0:0)

test/CodeCompletion/templates.cpp

   // CHECK-CC2: foo
   // CHECK-CC2: in_base
   // CHECK-CC2: stop
-  
+  // RUN: c-index-test -code-completion-at=%s:18:8 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
+  // CHECK-CC3: ClassTemplate:{TypedText allocator}{LeftAngle <}{Placeholder typename T}{RightAngle >} (50) (source location: 3:9)
+  // CHECK-CC3-NEXT: ClassTemplate:{TypedText vector}{LeftAngle <}{Placeholder typename T}{Optional {Comma , }{Placeholder typename Alloc}}{RightAngle >} (50) (source location: 9:9)
+
 

tools/c-index-test/c-index-test.c

 }
 
 static void print_completion_result(CXCompletionResult *completion_result,
-                                    FILE *file) {
+                                    FILE *file,
+                                    CXCodeCompleteResults *all_results,
+                                    int index_in_results) {
   CXString ks = clang_getCursorKindSpelling(completion_result->CursorKind);
   unsigned annotationCount;
   enum CXCursorKind ParentKind;
   CXString ParentName;
   CXString BriefComment;
   CXString Annotation;
+  CXCursor SourceCursor;
+  CXSourceLocation SourceLocation;
+  uint SourceLine = 0;
+  uint SourceColumn = 0;
+  CXFile SourceFile;
   const char *BriefCommentCString;
   
   fprintf(file, "%s:", clang_getCString(ks));

     fprintf(file, "(brief comment: %s)", BriefCommentCString);
   }
   clang_disposeString(BriefComment);
-  
+  SourceCursor = clang_getCompletionCursor(completion_result);
+  SourceLocation = clang_getCursorLocation(SourceCursor);
+  clang_getFileLocation(SourceLocation, &SourceFile, &SourceLine, &SourceColumn, 0);
+  fprintf(file, " (source location: %i:%i)", SourceLine, SourceColumn);
+
   fprintf(file, "\n");
 }
 

       clang_sortCodeCompletionResults(results->Results, results->NumResults);
 
       for (i = 0; i != n; ++i)
-        print_completion_result(results->Results + i, stdout);
+        print_completion_result(results->Results + i, stdout, results, i);
     }
     n = clang_codeCompleteGetNumDiagnostics(results);
     for (i = 0; i != n; ++i) {

tools/libclang/CIndexCodeCompletion.cpp

                                     unsigned NumResults) override {
       StoredResults.reserve(StoredResults.size() + NumResults);
       for (unsigned I = 0; I != NumResults; ++I) {
-        CodeCompletionString *StoredCompletion        
-          = Results[I].CreateCodeCompletionString(S, Context, getAllocator(),
+        auto& Result = Results[I];
+        CodeCompletionString *StoredCompletion
+          = Result.CreateCodeCompletionString(S, Context, getAllocator(),
                                                   getCodeCompletionTUInfo(),
                                                   includeBriefComments());
-        
+        CXCursor cursor = clang_getNullCursor();
+        switch (Result.Kind) {
+        case CodeCompletionResult::RK_Pattern:
+        case CodeCompletionResult::RK_Declaration:
+            if (auto Declaration = Result.Declaration) {
+              if ( !Declaration->isImplicit() )
+                cursor = cxcursor::MakeCXCursor(Declaration, *TU);
+            }
+            break;
+        case CodeCompletionResult::RK_Macro:
+            // TODO: build the CXCursor that represents the Resul.Macro
+            //       I have no idea how to get the MacroDefinitionRecord
+            //       corresponding to the IdentifierInfo
+            break;
+        case CodeCompletionResult::RK_Keyword:
+            // not representable by a cursor
+            break;
+        }
+
         CXCompletionResult R;
-        R.CursorKind = Results[I].CursorKind;
+        R.CursorKind = Result.CursorKind;
         R.CompletionString = StoredCompletion;
+        R.Cursor = cursor;
         StoredResults.push_back(R);
       }
       

                                    unsigned NumCandidates) override {
       StoredResults.reserve(StoredResults.size() + NumCandidates);
       for (unsigned I = 0; I != NumCandidates; ++I) {
+        const auto& Candidate = Candidates[I];
         CodeCompletionString *StoredCompletion
-          = Candidates[I].CreateSignatureString(CurrentArg, S, getAllocator(),
+          = Candidate.CreateSignatureString(CurrentArg, S, getAllocator(),
                                                 getCodeCompletionTUInfo(),
                                                 includeBriefComments());
         
+        auto FunctionDecl = Candidate.getFunction();
+        CXCursor cursor = FunctionDecl ? cxcursor::MakeCXCursor(FunctionDecl, *TU) : clang_getNullCursor();
+
         CXCompletionResult R;
         R.CursorKind = CXCursor_OverloadCandidate;
         R.CompletionString = StoredCompletion;
+        R.Cursor = cursor;
         StoredResults.push_back(R);
       }
     }

   };
 }
 
+CXCursor clang_getCompletionCursor(CXCompletionResult *Result)
+{
+  if (!Result)
+    return clang_getNullCursor();
+
+  return Result->Cursor;
+}
+
 void clang_sortCodeCompletionResults(CXCompletionResult *Results,
                                      unsigned NumResults) {
   std::stable_sort(Results, Results + NumResults, OrderCompletionResults());

tools/libclang/libclang.exports

 clang_getCanonicalType
 clang_getChildDiagnostics
 clang_getClangVersion
+clang_getCompletionCursor
 clang_getCompletionAnnotation
 clang_getCompletionAvailability
 clang_getCompletionBriefComment