[cfe-commits] [PATCH] New libclang API function, clang_codeCompleteGetContexts

[Douglas Gregor]

On Jul 5, 2011, at 5:41 PM, Connor Wakamo wrote:

> Hello,
> 
> I have implemented a new API function for libclang, clang_codeCompleteGetContexts, that returns a bitfield with the various code completion result types that are valid for the current code completion.  (The values for this bitfield are defined in enum CXCompletionContext in include/clang-c/Index.h.)
> 
> In support of this change, I also added a few new values to enum CodeCompletionContext::Kind in include/clang/Sema/CodeCompleteConsumer.h to provide better information for certain Objective-C contexts that were previously included in the "Other" context kind.
> 
> I also c-index-test to provide this context information when run with the -code-completion-at option.  I also updated the tests in test/Index/code-completion.cpp and test/Index/complete-natural.m to test the new output.
> 
> Finally, I also updated tools/libclang/libclang.exports and tools/libclang/libclang.darwin.exports to reflect the added API.
> 
> As this is my first submission to Clang, please let me know if I've overlooked something or if I need to make additional changes.

Very cool. Some comments on your patch:

+  /**
+   * \brief The context for completions is unknown or unexposed.
+   */
+  CXCompletionContext_Unknown = 0,

How should the client interpret this bit? Is it the same as setting all of the bits in the result set, and if so, do we need it?

+  /**
+   * \brief Completions for fields of the member being accessed should be
+   * included in the results.
+   */
+  CXCompletionContext_MemberAccess = 1 << 5,

This doesn't seem specific enough. For example, it doesn't allow one to tell the difference between -> and . on an Objective-C object, so the client wouldn't know whether to produce ivars or properties.

+  /**
+   * \brief Completions for Objective-C instance methods should be included
+   * in the results.
+   */
+  CXCompletionContext_ObjCInstanceMethod = 1 << 14,
+  /**
+   * \brief Completions for Objective-C class methods should be included in
+   * the results.
+   */
+  CXCompletionContext_ObjCClassMethod = 1 << 15,

These seem to be tied up to CCC_ObjCInstanceMessage/CCC_ObjCClassMessage, which are for message sends, e.g.,

	[x initWithFoo:foo andBar:bar]

but the names and comments seem to imply that we're talking about, e.g.,

	- (id)initWithFoo:(Foo*)foo andBar:(Bar*)bar]

?

If it's message sends that we're targeting, we're going to need to expose the selector pieces that have already been 

@@ -235,6 +236,19 @@
   
   /// \brief Allocator used to store code completion results.
   clang::CodeCompletionAllocator CodeCompletionAllocator;
+  
+  /// \brief Context under which completion occurred.
+  enum clang::CodeCompletionContext::Kind ContextKind;
+  
+  /// \brief A bitfield representing the acceptable completions for the
+  /// current context.
+  unsigned long long Contexts;
+  
+  /// \brief Base type for member access (CCC_MemberAccess) completions.
+  clang::QualType BaseType;
+  
+  /// \brief The preferred type for the current completion context.
+  clang::QualType PreferredType;

Hrm, interesting. There's a problem here, which is that the code completion results can actually outline the CXTranslationUnit that the QualType points into, so if we're going to save this information, we'll have to do it in some way that doesn't reference back into the ASTContext. Since this information isn't exposed in the API now, I suggest dropping it.

@@ -273,6 +287,126 @@
   
 } // end extern "C"
 
+static unsigned long long getContextsForContextKind(
+                                          enum CodeCompletionContext::Kind kind, 
+                                                    Sema &S) {
+  unsigned long long contexts = 0;

Wherever "contexts" includes CXCompletionContext_AnyType, it seems that we also want to include CXCompletionContext_EnumTag/CXCompletionContext_UnionTag/CXCompletionContext_StructTag when we're in C++ mode, for, e.g.,

	struct A { };
	A *ptr;

(We don't want the client to have to think about that)

Similarly, we can have a qualified name just about everywhere in C++, which makes things a bit interesting... perhaps we want a bit for "nested name specifiers", like ASTUnit does, and the client can provide completions for namespaces/classes/etc. followed by "::".

+    case CodeCompletionContext::CCC_Other:
+    case CodeCompletionContext::CCC_ObjCInterface:
+    case CodeCompletionContext::CCC_ObjCImplementation:
+    case CodeCompletionContext::CCC_Name:
+    case CodeCompletionContext::CCC_PotentiallyQualifiedName:

CCC_PotentiallyQualifiedName seems like it makes onto the "nested name specifiers" bit I mentioned above.

+    case CodeCompletionContext::CCC_MacroName:

This maps to CXCompletionContext_MacroName?

+    case CodeCompletionContext::CCC_PreprocessorExpression:
+    case CodeCompletionContext::CCC_PreprocessorDirective:
+    case CodeCompletionContext::CCC_TypeQualifiers:

CCC_TypeQualifiers is one of those cases where we probably want to return a completely empty set.

 // CHECK-OVERLOAD: NotImplemented:{ResultType int &}{Text overloaded}{LeftParen (}{Text Z z}{Comma , }{CurrentParameter int second}{RightParen )}
 // CHECK-OVERLOAD: NotImplemented:{ResultType float &}{Text overloaded}{LeftParen (}{Text int i}{Comma , }{CurrentParameter long second}{RightParen )}
 // CHECK-OVERLOAD: NotImplemented:{ResultType double &}{Text overloaded}{LeftParen (}{Text float f}{Comma , }{CurrentParameter int second}{RightParen )}
+// CHECK-OVERLOAD: Completion contexts:
+// CHECK-OVERLOAD: Any type
+// CHECK-OVERLOAD: Any value
+// CHECK-OVERLOAD: Objective-C interface

You could use CHECK-OVERLOAD-NEXT for the last three of these lines, to make sure that the test fails if any additional contexts sneak in. 

Great start, thanks for working on this!

	- Doug