[PATCH] D25520: [CodeCompletion] Add block placeholders when completing member access for Objective-C block property setters

2016-10-18 Thread Alex Lorenz via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL284472: [CodeCompletion] Add a block property setter 
completion result (authored by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D25520?vs=74528=74977#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D25520

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

Index: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
===
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp
@@ -2212,6 +2212,7 @@
 static std::string
 formatBlockPlaceholder(const PrintingPolicy , const NamedDecl *BlockDecl,
FunctionTypeLoc , FunctionProtoTypeLoc ,
+   bool SuppressBlockName = false,
bool SuppressBlock = false,
Optional ObjCSubsts = None);
 
@@ -2277,7 +2278,8 @@
 
   // 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);
 }
 
@@ -2293,7 +2295,7 @@
 static std::string
 formatBlockPlaceholder(const PrintingPolicy , const NamedDecl *BlockDecl,
FunctionTypeLoc , FunctionProtoTypeLoc ,
-   bool SuppressBlock,
+   bool SuppressBlockName, bool SuppressBlock,
Optional ObjCSubsts) {
   std::string Result;
   QualType ResultType = Block.getTypePtr()->getReturnType();
@@ -2329,16 +2331,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();
   }
 
@@ -3611,21 +3613,59 @@
 
 static void AddObjCProperties(const CodeCompletionContext ,
   ObjCContainerDecl *Container,
-  bool AllowCategories,
-  bool AllowNullaryMethods,
+  bool AllowCategories, bool AllowNullaryMethods,
   DeclContext *CurContext,
   AddedPropertiesSet ,
-  ResultBuilder ) {
+  ResultBuilder ,
+  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;
+
+Results.MaybeAddResult(Result(P, Results.getBasePriority(P), nullptr),
+   CurContext);
+
+// Provide additional 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 =

[PATCH] D25520: [CodeCompletion] Add block placeholders when completing member access for Objective-C block property setters

2016-10-18 Thread Alex Lorenz via cfe-commits 
arphaman added a comment.

In https://reviews.llvm.org/D25520#569389, @akyrtzi wrote:

> >   What do you think of the following possible priority heuristic
>
> SGTM.
>
> Changes LGTM. I'd also recommend that as a follow-up patch it would be great 
> to extend the setter completion to variables as well (global variables, 
> fields, ivars, etc.)


Thanks, I will commit this as it is. I will post a follow-up patch that uses 
the priority heuristic that I described above, and will work on related patches 
that you mentioned as well.


Repository:
  rL LLVM

https://reviews.llvm.org/D25520



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25520: [CodeCompletion] Add block placeholders when completing member access for Objective-C block property setters

2016-10-13 Thread Argyrios Kyrtzidis via cfe-commits 
akyrtzi added a comment.

Another recommendation for follow-up. When invoking completion on the 
right-hand side of the assignment it should provide a block literal completion 
with high priority. For example, when completing like this:

`self.foo = `


Repository:
  rL LLVM

https://reviews.llvm.org/D25520



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25520: [CodeCompletion] Add block placeholders when completing member access for Objective-C block property setters

2016-10-13 Thread Argyrios Kyrtzidis via cfe-commits 
akyrtzi added a comment.

>   What do you think of the following possible priority heuristic

SGTM.

Changes LGTM. I'd also recommend that as a follow-up patch it would be great to 
extend the setter completion to variables as well (global variables, fields, 
ivars, etc.)


Repository:
  rL LLVM

https://reviews.llvm.org/D25520



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25520: [CodeCompletion] Add block placeholders when completing member access for Objective-C block property setters

2016-10-13 Thread Alex Lorenz via cfe-commits 
arphaman added a comment.

Right now I gave the setter completion a flat priority bump of 3. Should 
something different be used? What do you think of the following possible 
priority heuristic: when completing blocks properties that return `void` the 
default property completion result should show up before the setter, otherwise 
the setter should show up before the property (we normally don't want to ignore 
the result of the call, right)?


Repository:
  rL LLVM

https://reviews.llvm.org/D25520



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25520: [CodeCompletion] Add block placeholders when completing member access for Objective-C block property setters

2016-10-13 Thread Alex Lorenz via cfe-commits 
arphaman updated this revision to Diff 74528.
arphaman added a comment.

The updated patch now treats the block property setter completion as a 
completion result that's additional to the default property completion result.


Repository:
  rL LLVM

https://reviews.llvm.org/D25520

Files:
  include/clang/Parse/Parser.h
  include/clang/Sema/CodeCompleteConsumer.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,68 @@
+// 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} (35)
+// CHECK-CC1-NEXT: ObjCPropertyDecl:{ResultType void (^)(Obj *)}{TypedText onAction}{Equal  = }{Placeholder ^(Obj *object)} (38)
+// CHECK-CC1-NEXT: ObjCPropertyDecl:{ResultType FooBlock}{TypedText onEventHandler} (35)
+// CHECK-CC1-NEXT: ObjCPropertyDecl:{ResultType FooBlock}{TypedText onEventHandler}{Equal  = }{Placeholder ^(Foo *someParameter)} (38)
+// 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:47:9 %s | FileCheck -check-prefix=CHECK-NO %s
+// RUN: c-index-test -code-completion-at=%s:48:16 %s | FileCheck -check-prefix=CHECK-NO %s
+// RUN: c-index-test -code-completion-at=%s:49:23 %s | FileCheck -check-prefix=CHECK-NO %s
+// RUN: c-index-test -code-completion-at=%s:50:12 %s | FileCheck -check-prefix=CHECK-NO %s
+// RUN: c-index-test -code-completion-at=%s:51:15 %s | FileCheck -check-prefix=CHECK-NO %s
+// RUN: c-index-test -code-completion-at=%s:53: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
@@ -2212,6 +2212,7 @@
 static std::string
 formatBlockPlaceholder(const PrintingPolicy , const NamedDecl *BlockDecl,
FunctionTypeLoc , FunctionProtoTypeLoc ,
+   bool SuppressBlockName = false,
bool SuppressBlock = false,
Optional ObjCSubsts = None);
 
@@ -2277,7 +2278,8 @@
 
   // 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);
 }
 
@@ -2293,7 +2295,7 @@
 static std::string
 formatBlockPlaceholder(const PrintingPolicy , const NamedDecl *BlockDecl,
FunctionTypeLoc , FunctionProtoTypeLoc ,
-   bool SuppressBlock,
+   bool SuppressBlockName, bool SuppressBlock,
Optional ObjCSubsts) {
   std::string

[PATCH] D25520: [CodeCompletion] Add block placeholders when completing member access for Objective-C block property setters

2016-10-13 Thread Alex Lorenz via cfe-commits 
arphaman added a comment.

In https://reviews.llvm.org/D25520#568448, @akyrtzi wrote:

> What if the user just wants to invoke the block, this is as common or more, 
> like:
>
> `self.onEventHandler(10)`
>
> The assign literal completion is useful but it should be an additional entry 
> (with maybe lower priority) not replace the property completion.
>  BTW, it would be great if we had completion as if it was a function call, so 
> 2 entries, 1 for block invocation and 1 for assigning literal. Absence the 
> block invocation call it should be 1 for normal property completion and 1 for 
> assigning literal.


You're right, I forgot about the most common use of block properties. I will 
update the patch today.

It makes sense to complete block invocation as well, but that has to be done in 
a separate patch. I suppose that completion for block invocations should be 
provided for normal sub expressions as well then.


Repository:
  rL LLVM

https://reviews.llvm.org/D25520



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25520: [CodeCompletion] Add block placeholders when completing member access for Objective-C block property setters

2016-10-12 Thread Argyrios Kyrtzidis via cfe-commits 
akyrtzi added a comment.

What if the user just wants to invoke the block, this is as common or more, 
like:

`self.onEventHandler(10)`

The assign literal completion is useful but it should be an additional entry 
(with maybe lower priority) not replace the property completion.
BTW, it would be great we had completion as if it was a function call, so 2 
entries, 1 for block invocation and 1 for assigning literal. Absence the block 
invocation call it should be 1 for normal property completion and 1 for 
assigning literal.


Repository:
  rL LLVM

https://reviews.llvm.org/D25520



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25520: [CodeCompletion] Add block placeholders when completing member access for Objective-C block property setters

2016-10-12 Thread Alex Lorenz via cfe-commits 
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. // will complete with ‘=‘ and block
  }
  - fooNot {
// These will code complete normally:
(self.)
return self.
[self foo: self.]
if (self.) { }
  }
  @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 , const NamedDecl *BlockDecl,
FunctionTypeLoc , FunctionProtoTypeLoc ,
+   bool SuppressBlockName = false,
bool SuppressBlock = false,
Optional ObjCSubsts = None);
 
@@ -2271,14 +2272,15 @@
 
   // We have