arphaman created this revision.
arphaman added reviewers: manmanren, doug.gregor.
arphaman added a subscriber: cfe-commits.
arphaman set the repository for this revision to rL LLVM.

This patch depends on https://reviews.llvm.org/D25519.

This patch changes the way that Objective-C block properties are code complete: 
clang now suggests completion with an additional '=' and the block literal 
placeholder when providing member access completions for appropriate readwrite 
block properties. This patch uses a simple heuristic to determine if it's 
appropriate to suggest a setter completion for block properties: if we are 
completing member access that is a standalone statement, we provide setter 
completion. Otherwise we fallback to the default property completion.

The following example illustrates when the setter completion is triggered:

  @interface Test : Obj
  @property (readonly, nonatomic, copy) void (^onReadonly)(int *someParameter);
  @end
  @implementation Test
  - foo {
    self.<COMPLETION> // will complete with ‘=‘ and block
  }
  - fooNot {
    // These will code complete normally:
    (self.<COMPLETION>)
    return self.<COMPLETION>
    [self foo: self.<COMPLETION>]
    if (self.<COMPLETION>) { }
  }
  @end


Repository:
  rL LLVM

https://reviews.llvm.org/D25520

Files:
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseStmt.cpp
  lib/Sema/SemaCodeComplete.cpp
  test/Index/complete-block-property-assignment.m

Index: test/Index/complete-block-property-assignment.m
===================================================================
--- /dev/null
+++ test/Index/complete-block-property-assignment.m
@@ -0,0 +1,66 @@
+// Note: the run lines follow their respective tests, since line/column
+// matter in this test.
+
+// rdar://28481726
+
+void func(int x);
+typedef int Foo;
+typedef void (^FooBlock)(Foo *someParameter);
+
+@interface Obj
+@property (readwrite, nonatomic, copy) void (^onAction)(Obj *object);
+@property (readwrite, nonatomic) int foo;
+@end
+
+@interface Test : Obj
+@property (readwrite, nonatomic, copy) FooBlock onEventHandler;
+@property (readonly, nonatomic, copy) void (^onReadonly)(int *someParameter);
+@property (readonly, nonatomic, strong) Obj *obj;
+@end
+
+@implementation Test
+
+#define SELFY self
+
+- (void)test {
+  self.foo = 2;
+  [self takeInt: 2]; self.foo = 2;
+  /* Comment */ self.foo = 2;
+  SELFY.foo = 2
+}
+
+// RUN: c-index-test -code-completion-at=%s:26:8 %s | FileCheck -check-prefix=CHECK-CC1 %s
+// RUN: c-index-test -code-completion-at=%s:27:27 %s | FileCheck -check-prefix=CHECK-CC1 %s
+// RUN: c-index-test -code-completion-at=%s:28:22 %s | FileCheck -check-prefix=CHECK-CC1 %s
+// RUN: c-index-test -code-completion-at=%s:29:9 %s | FileCheck -check-prefix=CHECK-CC1 %s
+// CHECK-CC1: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35)
+// CHECK-CC1-NEXT: ObjCPropertyDecl:{ResultType Obj *}{TypedText obj} (35)
+// CHECK-CC1-NEXT: ObjCPropertyDecl:{ResultType void (^)(Obj *)}{TypedText onAction}{Equal  = }{Placeholder ^(Obj *object)} (35)
+// CHECK-CC1-NEXT: ObjCPropertyDecl:{ResultType FooBlock}{TypedText onEventHandler}{Equal  = }{Placeholder ^(Foo *someParameter)} (35)
+// CHECK-CC1-NEXT: ObjCPropertyDecl:{ResultType void (^)(int *)}{TypedText onReadonly} (35)
+
+- (void) takeInt:(int)x { }
+
+- (int) testFailures {
+  (self.foo);
+  int x = self.foo;
+  [self takeInt: self.foo];
+  if (self.foo) {
+    func(self.foo);
+  }
+  return self.foo;
+}
+
+// RUN: c-index-test -code-completion-at=%s:45:9 %s | FileCheck -check-prefix=CHECK-NO %s
+// RUN: c-index-test -code-completion-at=%s:46:16 %s | FileCheck -check-prefix=CHECK-NO %s
+// RUN: c-index-test -code-completion-at=%s:47:23 %s | FileCheck -check-prefix=CHECK-NO %s
+// RUN: c-index-test -code-completion-at=%s:48:12 %s | FileCheck -check-prefix=CHECK-NO %s
+// RUN: c-index-test -code-completion-at=%s:49:15 %s | FileCheck -check-prefix=CHECK-NO %s
+// RUN: c-index-test -code-completion-at=%s:51:15 %s | FileCheck -check-prefix=CHECK-NO %s
+// CHECK-NO: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35)
+// CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType Obj *}{TypedText obj} (35)
+// CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType void (^)(Obj *)}{TypedText onAction} (35)
+// CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType FooBlock}{TypedText onEventHandler} (35)
+// CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType void (^)(int *)}{TypedText onReadonly} (35)
+
+@end
Index: lib/Sema/SemaCodeComplete.cpp
===================================================================
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -2206,6 +2206,7 @@
 static std::string
 formatBlockPlaceholder(const PrintingPolicy &Policy, const NamedDecl *BlockDecl,
                        FunctionTypeLoc &Block, FunctionProtoTypeLoc &BlockProto,
+                       bool SuppressBlockName = false,
                        bool SuppressBlock = false,
                        Optional<ArrayRef<QualType>> ObjCSubsts = None);
 
@@ -2271,14 +2272,15 @@
     
   // We have the function prototype behind the block pointer type, as it was
   // written in the source.
-  return formatBlockPlaceholder(Policy, Param, Block, BlockProto, SuppressBlock,
+  return formatBlockPlaceholder(Policy, Param, Block, BlockProto,
+                                /*SuppressBlockName=*/false, SuppressBlock,
                                 ObjCSubsts);
 }
 
 static std::string
 formatBlockPlaceholder(const PrintingPolicy &Policy, const NamedDecl *BlockDecl,
                        FunctionTypeLoc &Block, FunctionProtoTypeLoc &BlockProto,
-                       bool SuppressBlock,
+                       bool SuppressBlockName, bool SuppressBlock,
                        Optional<ArrayRef<QualType>> ObjCSubsts) {
   std::string Result;
   QualType ResultType = Block.getTypePtr()->getReturnType();
@@ -2314,16 +2316,16 @@
   if (SuppressBlock) {
     // Format as a parameter.
     Result = Result + " (^";
-    if (BlockDecl->getIdentifier())
+    if (!SuppressBlockName && BlockDecl->getIdentifier())
       Result += BlockDecl->getIdentifier()->getName();
     Result += ")";
     Result += Params;
   } else {
     // Format as a block literal argument.
     Result = '^' + Result;
     Result += Params;
 
-    if (BlockDecl->getIdentifier())
+    if (!SuppressBlockName && BlockDecl->getIdentifier())
       Result += BlockDecl->getIdentifier()->getName();
   }
 
@@ -3596,21 +3598,58 @@
 
 static void AddObjCProperties(const CodeCompletionContext &CCContext,
                               ObjCContainerDecl *Container,
-                              bool AllowCategories,
-                              bool AllowNullaryMethods,
+                              bool AllowCategories, bool AllowNullaryMethods,
                               DeclContext *CurContext,
                               AddedPropertiesSet &AddedProperties,
-                              ResultBuilder &Results) {
+                              ResultBuilder &Results,
+                              bool IsBaseExprStatement = false) {
   typedef CodeCompletionResult Result;
 
   // Retrieve the definition.
   Container = getContainerDef(Container);
   
   // Add properties in this container.
-  for (const auto *P : Container->instance_properties())
-    if (AddedProperties.insert(P->getIdentifier()).second)
-      Results.MaybeAddResult(Result(P, Results.getBasePriority(P), nullptr),
-                             CurContext);
+  for (const auto *P : Container->instance_properties()) {
+    if (!AddedProperties.insert(P->getIdentifier()).second)
+      continue;
+
+    // Provide block setter completion iff the base expression is a statement.
+    if (!P->isReadOnly() && IsBaseExprStatement &&
+        P->getType().getTypePtr()->isBlockPointerType()) {
+      FunctionTypeLoc BlockLoc;
+      FunctionProtoTypeLoc BlockProtoLoc;
+      findTypeLocationForBlockDecl(P->getTypeSourceInfo(), BlockLoc,
+                                   BlockProtoLoc);
+
+      // Provide block setter completion only when we are able to find
+      // the FunctionProtoTypeLoc with parameter names for the block.
+      if (BlockLoc) {
+        CodeCompletionBuilder Builder(Results.getAllocator(),
+                                      Results.getCodeCompletionTUInfo());
+        AddResultTypeChunk(Container->getASTContext(),
+                           getCompletionPrintingPolicy(Results.getSema()), P,
+                           CCContext.getBaseType(), Builder);
+        Builder.AddTypedTextChunk(
+            Results.getAllocator().CopyString(P->getName()));
+        Builder.AddChunk(CodeCompletionString::CK_Equal);
+
+        std::string PlaceholderStr = formatBlockPlaceholder(
+            getCompletionPrintingPolicy(Results.getSema()), P, BlockLoc,
+            BlockProtoLoc, /*SuppressBlockName=*/true);
+        // Add the placeholder string.
+        Builder.AddPlaceholderChunk(
+            Builder.getAllocator().CopyString(PlaceholderStr));
+
+        Results.MaybeAddResult(
+            Result(Builder.TakeString(), P, Results.getBasePriority(P)),
+            CurContext);
+        continue;
+      }
+    }
+
+    Results.MaybeAddResult(Result(P, Results.getBasePriority(P), nullptr),
+                           CurContext);
+  }
 
   // Add nullary methods
   if (AllowNullaryMethods) {
@@ -3639,37 +3678,41 @@
   if (ObjCProtocolDecl *Protocol = dyn_cast<ObjCProtocolDecl>(Container)) {
     for (auto *P : Protocol->protocols())
       AddObjCProperties(CCContext, P, AllowCategories, AllowNullaryMethods,
-                        CurContext, AddedProperties, Results);
+                        CurContext, AddedProperties, Results,
+                        IsBaseExprStatement);
   } else if (ObjCInterfaceDecl *IFace = dyn_cast<ObjCInterfaceDecl>(Container)){
     if (AllowCategories) {
       // Look through categories.
       for (auto *Cat : IFace->known_categories())
         AddObjCProperties(CCContext, Cat, AllowCategories, AllowNullaryMethods,
-                          CurContext, AddedProperties, Results);
+                          CurContext, AddedProperties, Results,
+                          IsBaseExprStatement);
     }
 
     // Look through protocols.
     for (auto *I : IFace->all_referenced_protocols())
       AddObjCProperties(CCContext, I, AllowCategories, AllowNullaryMethods,
-                        CurContext, AddedProperties, Results);
-    
+                        CurContext, AddedProperties, Results,
+                        IsBaseExprStatement);
+
     // Look in the superclass.
     if (IFace->getSuperClass())
       AddObjCProperties(CCContext, IFace->getSuperClass(), AllowCategories,
-                        AllowNullaryMethods, CurContext, 
-                        AddedProperties, Results);
+                        AllowNullaryMethods, CurContext, AddedProperties,
+                        Results, IsBaseExprStatement);
   } else if (const ObjCCategoryDecl *Category
                                     = dyn_cast<ObjCCategoryDecl>(Container)) {
     // Look through protocols.
     for (auto *P : Category->protocols())
       AddObjCProperties(CCContext, P, AllowCategories, AllowNullaryMethods,
-                        CurContext, AddedProperties, Results);
+                        CurContext, AddedProperties, Results,
+                        IsBaseExprStatement);
   }
 }
 
 void Sema::CodeCompleteMemberReferenceExpr(Scope *S, Expr *Base,
-                                           SourceLocation OpLoc,
-                                           bool IsArrow) {
+                                           SourceLocation OpLoc, bool IsArrow,
+                                           bool IsBaseExprStatement) {
   if (!Base || !CodeCompleter)
     return;
   
@@ -3751,13 +3794,14 @@
       assert(ObjCPtr && "Non-NULL pointer guaranteed above!");
       AddObjCProperties(CCContext, ObjCPtr->getInterfaceDecl(), true,
                         /*AllowNullaryMethods=*/true, CurContext,
-                        AddedProperties, Results);
+                        AddedProperties, Results, IsBaseExprStatement);
     }
 
     // Add properties from the protocols in a qualified interface.
     for (auto *I : BaseType->getAs<ObjCObjectPointerType>()->quals())
       AddObjCProperties(CCContext, I, true, /*AllowNullaryMethods=*/true,
-                        CurContext, AddedProperties, Results);
+                        CurContext, AddedProperties, Results,
+                        IsBaseExprStatement);
   } else if ((IsArrow && BaseType->isObjCObjectPointerType()) ||
              (!IsArrow && BaseType->isObjCObjectType())) {
     // Objective-C instance variable access.
Index: lib/Parse/ParseStmt.cpp
===================================================================
--- lib/Parse/ParseStmt.cpp
+++ lib/Parse/ParseStmt.cpp
@@ -396,6 +396,8 @@
   // If a case keyword is missing, this is where it should be inserted.
   Token OldToken = Tok;
 
+  ExprStatementTokLoc = Tok.getLocation();
+
   // expression[opt] ';'
   ExprResult Expr(ParseExpression());
   if (Expr.isInvalid()) {
Index: lib/Parse/ParseExpr.cpp
===================================================================
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -1646,9 +1646,10 @@
 
       if (Tok.is(tok::code_completion)) {
         // Code completion for a member access expression.
-        Actions.CodeCompleteMemberReferenceExpr(getCurScope(), LHS.get(),
-                                                OpLoc, OpKind == tok::arrow);
-        
+        Actions.CodeCompleteMemberReferenceExpr(
+            getCurScope(), LHS.get(), OpLoc, OpKind == tok::arrow,
+            ExprStatementTokLoc == LHS.get()->getLocStart());
+
         cutOffParsing();
         return ExprError();
       }
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -9427,8 +9427,8 @@
   void CodeCompleteExpression(Scope *S,
                               const CodeCompleteExpressionData &Data);
   void CodeCompleteMemberReferenceExpr(Scope *S, Expr *Base,
-                                       SourceLocation OpLoc,
-                                       bool IsArrow);
+                                       SourceLocation OpLoc, bool IsArrow,
+                                       bool IsBaseExprStatement);
   void CodeCompletePostfixExpression(Scope *S, ExprResult LHS);
   void CodeCompleteTag(Scope *S, unsigned TagSpec);
   void CodeCompleteTypeQualifiers(DeclSpec &DS);
Index: include/clang/Parse/Parser.h
===================================================================
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -247,6 +247,11 @@
 
   bool SkipFunctionBodies;
 
+  /// The location of the expression statement that is being parsed right now.
+  /// Used to determine if an expression that is being parsed is a statement or
+  /// just a regular sub-expression.
+  SourceLocation ExprStatementTokLoc;
+
 public:
   Parser(Preprocessor &PP, Sema &Actions, bool SkipFunctionBodies);
   ~Parser() override;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to