[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL341824: [clang] Make sure codecompletion is called for calls 
even when inside a token. (authored by kadircet, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D51038

Files:
  cfe/trunk/include/clang/Parse/Parser.h
  cfe/trunk/lib/Parse/ParseDecl.cpp
  cfe/trunk/lib/Parse/ParseExpr.cpp
  cfe/trunk/lib/Parse/ParseExprCXX.cpp
  cfe/trunk/lib/Parse/ParseOpenMP.cpp
  cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp
  cfe/trunk/test/CodeCompletion/function-overloads.cpp
  cfe/trunk/test/Index/complete-block-property-assignment.m

Index: cfe/trunk/include/clang/Parse/Parser.h
===
--- cfe/trunk/include/clang/Parse/Parser.h
+++ cfe/trunk/include/clang/Parse/Parser.h
@@ -214,6 +214,11 @@
   /// should not be set directly.
   bool InMessageExpression;
 
+  /// Gets set to true after calling ProduceSignatureHelp, it is for a
+  /// workaround to make sure ProduceSignatureHelp is only called at the deepest
+  /// function call.
+  bool CalledSignatureHelp = false;
+
   /// The "depth" of the template parameters currently being parsed.
   unsigned TemplateParameterDepth;
 
Index: cfe/trunk/test/Index/complete-block-property-assignment.m
===
--- cfe/trunk/test/Index/complete-block-property-assignment.m
+++ cfe/trunk/test/Index/complete-block-property-assignment.m
@@ -60,13 +60,20 @@
 // RUN: c-index-test -code-completion-at=%s:51:16 %s | FileCheck -check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:52:23 %s | FileCheck -check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:53:12 %s | FileCheck -check-prefix=CHECK-NO %s
-// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck -check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:56: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)
 // CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType int (^)(int)}{TypedText processEvent} (35)
 
+// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck -check-prefix=CHECK-NO1 %s
+// CHECK-NO1: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType Obj *}{TypedText obj} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType void (^)(Obj *)}{TypedText onAction} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType FooBlock}{TypedText onEventHandler} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType void (^)(int *)}{TypedText onReadonly} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType int (^)(int)}{TypedText processEvent} (35)
+// CHECK-NO1-NEXT: OverloadCandidate:{ResultType void}{Text func}{LeftParen (}{CurrentParameter int x}{RightParen )} (1)
 @end
Index: cfe/trunk/test/CodeCompletion/function-overloads.cpp
===
--- cfe/trunk/test/CodeCompletion/function-overloads.cpp
+++ cfe/trunk/test/CodeCompletion/function-overloads.cpp
@@ -0,0 +1,27 @@
+int f(int i, int j = 2, int k = 5);
+int f(float x, float y...);
+
+class A {
+ public:
+  A(int, int, int);
+};
+
+void test() {
+  A a(f(1, 2, 3, 4), 2, 3);
+}
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:9 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:10 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:17 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:19 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:20 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:21 %s -o - | FileCheck -check-prefix=CHECK-CC4 %s
+// CHECK-CC1: OVERLOAD: [#int#]f(<#float x#>, float y)
+// CHECK-CC1: OVERLOAD: [#int#]f(<#int i#>)
+// CHECK-CC1-NOT, CHECK-CC2-NOT: OVERLOAD: A(
+// CHECK-CC2: OVERLOAD: [#int#]f(float x, float y)
+// CHECK-CC2-NOT: OVERLOAD: [#int#]f(int i)
+// CHECK-CC3: OVERLOAD: A(<#int#>, int, int)
+// CHECK-CC3: OVERLOAD: A(<#const A 

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 164661.
kadircet marked an inline comment as done.
kadircet added a comment.

- One last completer


Repository:
  rC Clang

https://reviews.llvm.org/D51038

Files:
  include/clang/Parse/Parser.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Sema/CodeCompleteConsumer.cpp
  test/CodeCompletion/function-overloads.cpp
  test/Index/complete-block-property-assignment.m

Index: test/Index/complete-block-property-assignment.m
===
--- test/Index/complete-block-property-assignment.m
+++ test/Index/complete-block-property-assignment.m
@@ -60,13 +60,20 @@
 // RUN: c-index-test -code-completion-at=%s:51:16 %s | FileCheck -check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:52:23 %s | FileCheck -check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:53:12 %s | FileCheck -check-prefix=CHECK-NO %s
-// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck -check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:56: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)
 // CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType int (^)(int)}{TypedText processEvent} (35)
 
+// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck -check-prefix=CHECK-NO1 %s
+// CHECK-NO1: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType Obj *}{TypedText obj} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType void (^)(Obj *)}{TypedText onAction} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType FooBlock}{TypedText onEventHandler} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType void (^)(int *)}{TypedText onReadonly} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType int (^)(int)}{TypedText processEvent} (35)
+// CHECK-NO1-NEXT: OverloadCandidate:{ResultType void}{Text func}{LeftParen (}{CurrentParameter int x}{RightParen )} (1)
 @end
Index: test/CodeCompletion/function-overloads.cpp
===
--- /dev/null
+++ test/CodeCompletion/function-overloads.cpp
@@ -0,0 +1,27 @@
+int f(int i, int j = 2, int k = 5);
+int f(float x, float y...);
+
+class A {
+ public:
+  A(int, int, int);
+};
+
+void test() {
+  A a(f(1, 2, 3, 4), 2, 3);
+}
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:9 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:10 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:17 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:19 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:20 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:21 %s -o - | FileCheck -check-prefix=CHECK-CC4 %s
+// CHECK-CC1: OVERLOAD: [#int#]f(<#float x#>, float y)
+// CHECK-CC1: OVERLOAD: [#int#]f(<#int i#>)
+// CHECK-CC1-NOT, CHECK-CC2-NOT: OVERLOAD: A(
+// CHECK-CC2: OVERLOAD: [#int#]f(float x, float y)
+// CHECK-CC2-NOT: OVERLOAD: [#int#]f(int i)
+// CHECK-CC3: OVERLOAD: A(<#int#>, int, int)
+// CHECK-CC3: OVERLOAD: A(<#const A 

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

Thanks! LGTM




Comment at: lib/Parse/ParseExprCXX.cpp:1687
 if (Tok.isNot(tok::r_paren)) {
-  if (ParseExpressionList(Exprs, CommaLocs, [&] {
-QualType PreferredType = Actions.ProduceConstructorSignatureHelp(
-getCurScope(), TypeRep.get()->getCanonicalTypeInternal(),
-DS.getEndLoc(), Exprs, T.getOpenLocation());
-Actions.CodeCompleteExpression(getCurScope(), PreferredType);
-  })) {
+  auto Completer = [&]() {
+QualType PreferredType = Actions.ProduceConstructorSignatureHelp(

NIT: inline this last completer.


Repository:
  rC Clang

https://reviews.llvm.org/D51038



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

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 164659.
kadircet added a comment.

- Inline completers.


Repository:
  rC Clang

https://reviews.llvm.org/D51038

Files:
  include/clang/Parse/Parser.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Sema/CodeCompleteConsumer.cpp
  test/CodeCompletion/function-overloads.cpp
  test/Index/complete-block-property-assignment.m

Index: test/Index/complete-block-property-assignment.m
===
--- test/Index/complete-block-property-assignment.m
+++ test/Index/complete-block-property-assignment.m
@@ -60,13 +60,20 @@
 // RUN: c-index-test -code-completion-at=%s:51:16 %s | FileCheck -check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:52:23 %s | FileCheck -check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:53:12 %s | FileCheck -check-prefix=CHECK-NO %s
-// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck -check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:56: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)
 // CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType int (^)(int)}{TypedText processEvent} (35)
 
+// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck -check-prefix=CHECK-NO1 %s
+// CHECK-NO1: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType Obj *}{TypedText obj} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType void (^)(Obj *)}{TypedText onAction} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType FooBlock}{TypedText onEventHandler} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType void (^)(int *)}{TypedText onReadonly} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType int (^)(int)}{TypedText processEvent} (35)
+// CHECK-NO1-NEXT: OverloadCandidate:{ResultType void}{Text func}{LeftParen (}{CurrentParameter int x}{RightParen )} (1)
 @end
Index: test/CodeCompletion/function-overloads.cpp
===
--- /dev/null
+++ test/CodeCompletion/function-overloads.cpp
@@ -0,0 +1,27 @@
+int f(int i, int j = 2, int k = 5);
+int f(float x, float y...);
+
+class A {
+ public:
+  A(int, int, int);
+};
+
+void test() {
+  A a(f(1, 2, 3, 4), 2, 3);
+}
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:9 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:10 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:17 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:19 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:20 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:21 %s -o - | FileCheck -check-prefix=CHECK-CC4 %s
+// CHECK-CC1: OVERLOAD: [#int#]f(<#float x#>, float y)
+// CHECK-CC1: OVERLOAD: [#int#]f(<#int i#>)
+// CHECK-CC1-NOT, CHECK-CC2-NOT: OVERLOAD: A(
+// CHECK-CC2: OVERLOAD: [#int#]f(float x, float y)
+// CHECK-CC2-NOT: OVERLOAD: [#int#]f(int i)
+// CHECK-CC3: OVERLOAD: A(<#int#>, int, int)
+// CHECK-CC3: OVERLOAD: A(<#const A 

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: lib/Parse/ParseExprCXX.cpp:2827
 if (Tok.isNot(tok::r_paren)) {
+  ParsedType TypeRep =
+  Actions.ActOnTypeName(getCurScope(), DeclaratorInfo).get();

ilya-biryukov wrote:
> ActOnTypeName is called at a different point now, please move it back into 
> the lambda.
As discussed offline, this actually LG.
It's weird that result of `ActOnTypeName` seems to be only needed for code 
completion and it's called even when completion is disabled.

However, this was the case before the patch as well


Repository:
  rC Clang

https://reviews.llvm.org/D51038



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

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: lib/Sema/CodeCompleteConsumer.cpp:622
 
+case CodeCompletionString::CK_Optional:
+  break;

ilya-biryukov wrote:
> This change is really sneaky and unrelated to the rest of the patch. It 
> should definitely be done separately from this patch.
> 
> Could you give more details on what fails exactly instead? Can we update the 
> test instead?
As Kadir mentioned offline this actually LG, we shouldn't read `C.Text` for 
CK_Optional chunks.

Maybe leave a FIXME that we could actually print the `CodeCompletionString` 
from the optional chunk? 


Repository:
  rC Clang

https://reviews.llvm.org/D51038



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

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: lib/Parse/ParseExpr.cpp:1663
 if (Tok.isNot(tok::r_paren)) {
-  if (ParseExpressionList(ArgExprs, CommaLocs, [&] {
-QualType PreferredType = Actions.ProduceCallSignatureHelp(
-getCurScope(), LHS.get(), ArgExprs, PT.getOpenLocation());
-Actions.CodeCompleteExpression(getCurScope(), PreferredType);
-  })) {
+  auto Completer = [&]() {
+QualType PreferredType = Actions.ProduceCallSignatureHelp(

NIT: inline completer.



Comment at: lib/Parse/ParseExpr.cpp:1663
 if (Tok.isNot(tok::r_paren)) {
-  if (ParseExpressionList(ArgExprs, CommaLocs, [&] {
-QualType PreferredType = Actions.ProduceCallSignatureHelp(
-getCurScope(), LHS.get(), ArgExprs, PT.getOpenLocation());
-Actions.CodeCompleteExpression(getCurScope(), PreferredType);
-  })) {
+  auto Completer = [&]() {
+QualType PreferredType = Actions.ProduceCallSignatureHelp(

ilya-biryukov wrote:
> NIT: inline completer.
Maybe inline this into lambda body again?
Now that it's not called outside it, we don't need a variable anymore.



Comment at: lib/Parse/ParseExprCXX.cpp:2827
 if (Tok.isNot(tok::r_paren)) {
+  ParsedType TypeRep =
+  Actions.ActOnTypeName(getCurScope(), DeclaratorInfo).get();

ActOnTypeName is called at a different point now, please move it back into the 
lambda.



Comment at: lib/Parse/ParseExprCXX.cpp:2830
   CommaLocsTy CommaLocs;
-  if (ParseExpressionList(ConstructorArgs, CommaLocs, [&] {
-ParsedType TypeRep = Actions.ActOnTypeName(getCurScope(),
-   DeclaratorInfo).get();
-QualType PreferredType = Actions.ProduceConstructorSignatureHelp(
-getCurScope(), TypeRep.get()->getCanonicalTypeInternal(),
-DeclaratorInfo.getEndLoc(), ConstructorArgs, 
ConstructorLParen);
-Actions.CodeCompleteExpression(getCurScope(), PreferredType);
-  })) {
+  auto Completer = [&]() {
+QualType PreferredType = Actions.ProduceConstructorSignatureHelp(

Same here: maybe inline the lambda into the call to keep the changes minimal?



Comment at: lib/Parse/ParseOpenMP.cpp:419
 SourceLocation LParLoc = T.getOpenLocation();
-if (ParseExpressionList(
-Exprs, CommaLocs, [this, OmpPrivParm, LParLoc, ] {
-  QualType PreferredType = Actions.ProduceConstructorSignatureHelp(
-  getCurScope(),
-  OmpPrivParm->getType()->getCanonicalTypeInternal(),
-  OmpPrivParm->getLocation(), Exprs, LParLoc);
-  Actions.CodeCompleteExpression(getCurScope(), PreferredType);
-})) {
+auto Completer = [this, OmpPrivParm, LParLoc, ]() {
+  QualType PreferredType = Actions.ProduceConstructorSignatureHelp(

Same here: maybe inline the lambda?


Repository:
  rC Clang

https://reviews.llvm.org/D51038



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

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 164649.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Resolve discussions.


Repository:
  rC Clang

https://reviews.llvm.org/D51038

Files:
  include/clang/Parse/Parser.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Sema/CodeCompleteConsumer.cpp
  test/CodeCompletion/function-overloads.cpp
  test/Index/complete-block-property-assignment.m

Index: test/Index/complete-block-property-assignment.m
===
--- test/Index/complete-block-property-assignment.m
+++ test/Index/complete-block-property-assignment.m
@@ -60,13 +60,20 @@
 // RUN: c-index-test -code-completion-at=%s:51:16 %s | FileCheck -check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:52:23 %s | FileCheck -check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:53:12 %s | FileCheck -check-prefix=CHECK-NO %s
-// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck -check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:56: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)
 // CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType int (^)(int)}{TypedText processEvent} (35)
 
+// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck -check-prefix=CHECK-NO1 %s
+// CHECK-NO1: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType Obj *}{TypedText obj} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType void (^)(Obj *)}{TypedText onAction} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType FooBlock}{TypedText onEventHandler} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType void (^)(int *)}{TypedText onReadonly} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType int (^)(int)}{TypedText processEvent} (35)
+// CHECK-NO1-NEXT: OverloadCandidate:{ResultType void}{Text func}{LeftParen (}{CurrentParameter int x}{RightParen )} (1)
 @end
Index: test/CodeCompletion/function-overloads.cpp
===
--- /dev/null
+++ test/CodeCompletion/function-overloads.cpp
@@ -0,0 +1,27 @@
+int f(int i, int j = 2, int k = 5);
+int f(float x, float y...);
+
+class A {
+ public:
+  A(int, int, int);
+};
+
+void test() {
+  A a(f(1, 2, 3, 4), 2, 3);
+}
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:9 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:10 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:17 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:19 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:20 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:21 %s -o - | FileCheck -check-prefix=CHECK-CC4 %s
+// CHECK-CC1: OVERLOAD: [#int#]f(<#float x#>, float y)
+// CHECK-CC1: OVERLOAD: [#int#]f(<#int i#>)
+// CHECK-CC1-NOT, CHECK-CC2-NOT: OVERLOAD: A(
+// CHECK-CC2: OVERLOAD: [#int#]f(float x, float y)
+// CHECK-CC2-NOT: OVERLOAD: [#int#]f(int i)
+// CHECK-CC3: OVERLOAD: A(<#int#>, int, int)
+// CHECK-CC3: OVERLOAD: A(<#const A 

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.
ilya-biryukov added inline comments.



Comment at: lib/Sema/CodeCompleteConsumer.cpp:622
 
+case CodeCompletionString::CK_Optional:
+  break;

This change is really sneaky and unrelated to the rest of the patch. It should 
definitely be done separately from this patch.

Could you give more details on what fails exactly instead? Can we update the 
test instead?


Repository:
  rC Clang

https://reviews.llvm.org/D51038



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

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: include/clang/Parse/Parser.h:217
 
+  /// Gets set to true after calling ProduceSignaturehelp, it is for a
+  /// workaround to make sure ProduceSignatureHelp is only called at the 
deepest

s/ProduceSignaturehelp/ProduceSignatureHelp



Comment at: lib/Parse/ParseDecl.cpp:2325
+  if (ThisVarDecl && PP.isCodeCompletionReached())
+ConstructorCompleter(true);
   Actions.ActOnInitializerError(ThisDecl);

Maybe call signature help here directly and remove param from lamdba?
Having a parameter in lambda adds both makes the lamdba body more complicated 
and adds extra state that we have to reason about.

Same for the other calls.


Repository:
  rC Clang

https://reviews.llvm.org/D51038



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

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In https://reviews.llvm.org/D51038#1228618, @ilya-biryukov wrote:

> > Currently the problem is, there are again some tests out there that rely on
> >  CodeCompeleteOrdinaryName to be called even when getting overloads at an 
> > unknown
> >  parameter type
>
> CodeCompleteExpression works just fine there. Unknown parameter type should 
> not block code completion.
>  The idea is that anywhere except the start of the argument expression, we 
> need to call **only** signature help. At the start of the argument, we have 
> to call both signature help and code completion.


Yeah you are right, I thought I could check whether I am inside a parameter or 
not by looking at qual type, which was apparently wrong :D

But, unfortunately, there were still tests failing which was caused by printing 
of completion strings for overloaded methods with optional parameters. It was 
trying to access Text field, which is not defined for Optional parameters.
Since it didn't print optional parameters in the original behavior I kept it 
that way. Can send it as a different patch if need be.


Repository:
  rC Clang

https://reviews.llvm.org/D51038



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

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 164639.
kadircet marked an inline comment as done.
kadircet added a comment.

- Change "inside parameter" checking and fix completion string printing for 
optional parameters.
- Update tests.


Repository:
  rC Clang

https://reviews.llvm.org/D51038

Files:
  include/clang/Parse/Parser.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Sema/CodeCompleteConsumer.cpp
  test/CodeCompletion/function-overloads.cpp
  test/Index/complete-block-property-assignment.m

Index: test/Index/complete-block-property-assignment.m
===
--- test/Index/complete-block-property-assignment.m
+++ test/Index/complete-block-property-assignment.m
@@ -60,13 +60,20 @@
 // RUN: c-index-test -code-completion-at=%s:51:16 %s | FileCheck -check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:52:23 %s | FileCheck -check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:53:12 %s | FileCheck -check-prefix=CHECK-NO %s
-// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck -check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:56: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)
 // CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType int (^)(int)}{TypedText processEvent} (35)
 
+// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck -check-prefix=CHECK-NO1 %s
+// CHECK-NO1: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType Obj *}{TypedText obj} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType void (^)(Obj *)}{TypedText onAction} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType FooBlock}{TypedText onEventHandler} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType void (^)(int *)}{TypedText onReadonly} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType int (^)(int)}{TypedText processEvent} (35)
+// CHECK-NO1-NEXT: OverloadCandidate:{ResultType void}{Text func}{LeftParen (}{CurrentParameter int x}{RightParen )} (1)
 @end
Index: test/CodeCompletion/function-overloads.cpp
===
--- /dev/null
+++ test/CodeCompletion/function-overloads.cpp
@@ -0,0 +1,27 @@
+int f(int i, int j = 2, int k = 5);
+int f(float x, float y...);
+
+class A {
+ public:
+  A(int, int, int);
+};
+
+void test() {
+  A a(f(1, 2, 3, 4), 2, 3);
+}
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:9 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:10 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:17 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:19 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:20 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:21 %s -o - | FileCheck -check-prefix=CHECK-CC4 %s
+// CHECK-CC1: OVERLOAD: [#int#]f(<#float x#>, float y)
+// CHECK-CC1: OVERLOAD: [#int#]f(<#int i#>)
+// CHECK-CC1-NOT, CHECK-CC2-NOT: OVERLOAD: A(
+// CHECK-CC2: OVERLOAD: [#int#]f(float x, float y)
+// CHECK-CC2-NOT: OVERLOAD: [#int#]f(int i)
+// CHECK-CC3: OVERLOAD: A(<#int#>, int, int)
+// CHECK-CC3: OVERLOAD: A(<#const A 

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

> Currently the problem is, there are again some tests out there that rely on
>  CodeCompeleteOrdinaryName to be called even when getting overloads at an 
> unknown
>  parameter type

CodeCompleteExpression works just fine there. Unknown parameter type should not 
block code completion.
The idea is that anywhere except the start of the argument expression, we need 
to call **only** signature help. At the start of the argument, we have to call 
both signature help and code completion.
All existing tests pass with the following diff:

  --- a/lib/Parse/ParseExpr.cpp
  +++ b/lib/Parse/ParseExpr.cpp
  @@ -1659,12 +1659,19 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) {
   
 if (OpKind == tok::l_paren || !LHS.isInvalid()) {
   if (Tok.isNot(tok::r_paren)) {
  -  if (ParseExpressionList(ArgExprs, CommaLocs, [&] {
  -QualType PreferredType = Actions.ProduceCallSignatureHelp(
  -getCurScope(), LHS.get(), ArgExprs, 
PT.getOpenLocation());
  -Actions.CodeCompleteExpression(getCurScope(), PreferredType);
  -  })) {
  +  auto Completer = [&] {
  +QualType PreferredType = Actions.ProduceCallSignatureHelp(
  +getCurScope(), LHS.get(), ArgExprs, PT.getOpenLocation());
  +Actions.CodeCompleteExpression(getCurScope(), PreferredType);
  +CalledOverloadCompletion = true;
  +  };
  +  if (ParseExpressionList(ArgExprs, CommaLocs, Completer)) {
   (void)Actions.CorrectDelayedTyposInExpr(LHS);
  +if (PP.isCodeCompletionReached() && !CalledOverloadCompletion) {
  +  CalledOverloadCompletion = true;
  +  Actions.ProduceCallSignatureHelp(
  +  getCurScope(), LHS.get(), ArgExprs, PT.getOpenLocation());
  +}
   LHS = ExprError();
 } else if (LHS.isInvalid()) {
   for (auto  : ArgExprs)

Please note there are other places where signature help methods 
(ProduceCallSignatureHelp and ProduceConstructorSignatureHelp) are called, we 
need to handle all of them. List of files that have those calls:

- lib/Parse/ParseExprCXX.cpp
- lib/Parse/ParseOpenMP.cpp
- lib/Parse/ParseExpr.cpp
- lib/Parse/ParseDecl.cpp




Comment at: include/clang/Parse/Parser.h:219
+  /// make sure CodeComleteCall is only called at the deepest level.
+  bool CalledOverloadCompletion = false;
+

NIT: rename to `CalledSignatureHelp`, this shouldn't affect completion


Repository:
  rC Clang

https://reviews.llvm.org/D51038



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

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 164477.
kadircet added a comment.

- Rebase and use new signature help.

Currently the problem is, there are again some tests out there that rely on
CodeCompeleteOrdinaryName to be called even when getting overloads at an unknown
parameter type. These tests are:

  Clang :: CodeCompletion/call.cpp
  Clang :: Index/code-completion.cpp
  Clang :: Index/complete-call.cpp
  Clang :: Index/complete-functor-call.cpp
  Clang :: Index/complete-optional-params.cpp
  Clang :: Index/complete-pointer-and-reference-to-functions.cpp

You can run something like this to check for the output(change ../clangd_bugs):

  ninja c-index-test && ./bin/c-index-test 
-code-completion-at=../clangd_bugs/tools/clang/test/Index/complete-call.cpp:53:12
 ../clan
  gd_bugs/tools/clang/test/Index/complete-call.cpp

As you can see current version generates only overloadcandidates, but tests
don't check them with a next loop, because they expect other completions, which
is not problematic but after checks for the items they also look for completion
context, which is not set as desired if CodeCompleteOrdinaryName or
CodeCompleteExpression is not called.

But for the first test case it is more complicated, it checks for code patterns
like dynamic_cast(EXPR); which is not generated if we don't call
CodeCompletionOrdinaryName or Expressions.


Repository:
  rC Clang

https://reviews.llvm.org/D51038

Files:
  include/clang/Parse/Parser.h
  lib/Parse/ParseExpr.cpp
  test/CodeCompletion/function-overloads-inside-param.cpp
  test/CodeCompletion/function-overloads.cpp
  test/Index/complete-block-property-assignment.m

Index: test/Index/complete-block-property-assignment.m
===
--- test/Index/complete-block-property-assignment.m
+++ test/Index/complete-block-property-assignment.m
@@ -60,13 +60,20 @@
 // RUN: c-index-test -code-completion-at=%s:51:16 %s | FileCheck -check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:52:23 %s | FileCheck -check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:53:12 %s | FileCheck -check-prefix=CHECK-NO %s
-// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck -check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:56: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)
 // CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType int (^)(int)}{TypedText processEvent} (35)
 
+// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck -check-prefix=CHECK-NO1 %s
+// CHECK-NO1: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType Obj *}{TypedText obj} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType void (^)(Obj *)}{TypedText onAction} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType FooBlock}{TypedText onEventHandler} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType void (^)(int *)}{TypedText onReadonly} (35)
+// CHECK-NO1-NEXT: ObjCPropertyDecl:{ResultType int (^)(int)}{TypedText processEvent} (35)
+// CHECK-NO1-NEXT: OverloadCandidate:{ResultType void}{Text func}{LeftParen (}{CurrentParameter int x}{RightParen )} (1)
 @end
Index: test/CodeCompletion/function-overloads.cpp
===
--- /dev/null
+++ test/CodeCompletion/function-overloads.cpp
@@ -0,0 +1,8 @@
+void f(int i, int j = 2, int k = 5);
+void f(float x, float y...);
+
+void test() {
+  ::f(
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:7 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+  // CHECK-CC1: f(<#int i#>{#, <#int j = 2#>{#, <#int k = 5#>#}#})
+  // CHECK-CC1: f(<#float x#>, <#float y, ...#>)
Index: test/CodeCompletion/function-overloads-inside-param.cpp
===
--- /dev/null
+++ test/CodeCompletion/function-overloads-inside-param.cpp
@@ -0,0 +1,8 @@
+void f(int i, int j = 2, int k = 5);
+void f(float x, float y...);
+
+void test() {
+  ::f(1
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:8 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+  // CHECK-CC1: f(<#int i#>{#, <#int j = 2#>{#, <#int k = 5#>#}#})
+  // CHECK-CC1: f(<#float x#>, <#float y, ...#>)
Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -1659,12 +1659,27 @@
 
   if (OpKind == tok::l_paren || !LHS.isInvalid()) {
 if (Tok.isNot(tok::r_paren)) {
-  if (ParseExpressionList(ArgExprs, CommaLocs, [&] {
-

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: test/Index/complete-block-property-assignment.m:71
 
+// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck 
-check-prefix=CHECK-NO1 %s
+// CHECK-NO1: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35)

ilya-biryukov wrote:
> ilya-biryukov wrote:
> > kadircet wrote:
> > > ilya-biryukov wrote:
> > > > ilya-biryukov wrote:
> > > > > Any idea why behavior changed in that case?
> > > > Looked at it more thoroughly now...
> > > > So we're now showing both the signature help and the completion, right?
> > > > If that the case, LG, but can we include the rest of completion items 
> > > > from CHECK-N0? 
> > > > 
> > > > Maybe also add a comment that the last item is from overload set 
> > > > completion, rather than the ordinary code completion? (To avoid 
> > > > confusion and make it clear why we need an extra check there)
> > > Yes, c-index-test binary collects results from both 
> > > ProcessOverloadCandidate and ProcessCodeCompleteResults. Therefore they 
> > > are merged. Adding the same set of results on the above is not enough for 
> > > that particular case, due to the clever nature of 
> > > CodeCompleteOverloadResult at 
> > > https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaCodeComplete.cpp#L4439.
> > > 
> > > It doesn't just try to tell you the overloads of that particular function 
> > > but also tries to complete the current argument. Therefore the list 
> > > simply gets huge by expansion of all the macros and other stuff.
> > > 
> > > But when looking into it I found out I was checking for wrong return 
> > > values fixing that with the new diff.
> > I don't think it's correct to call CodeCompleteExpression and 
> > CodeCompleteOrdinaryName in this context.
> > This completion assume we're at the start of the argument, which is not 
> > true anymore.
> > E.g. we can be producing weird results in the middle of expressions. Some 
> > examples from the top of my head:
> > ```
> > func(var.^); // <-- (1) we add top-level completions in addition to members 
> > of bar
> > func(&^); // <-- (2) we provide incorrect ParamType
> > ```
> > For (2) if ParamType is `int*`, we would incorrectly uprank items of type 
> > `int*` (should uprank items of type `int` instead).
> > 
> > I'll investigate a bit more to see if we can refactor the code to untangle 
> > signature help from code completion.
> D51782 removes completion-specific bits out of functions that produce 
> signature help. 
> After it lands, it should be significantly easier to produce results for 
> these cases.
> 
> Specifically, we will be able to only run signature help in the middle of the 
> args, and signature help followed by a normal completion at the start of the 
> args.
Landed D51782. This change should be easier now. 
We can now call both signature help and code completion at the start of the 
arguments and only signature help in the middle of the arguments.


Repository:
  rC Clang

https://reviews.llvm.org/D51038



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

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: test/Index/complete-block-property-assignment.m:71
 
+// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck 
-check-prefix=CHECK-NO1 %s
+// CHECK-NO1: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35)

ilya-biryukov wrote:
> kadircet wrote:
> > ilya-biryukov wrote:
> > > ilya-biryukov wrote:
> > > > Any idea why behavior changed in that case?
> > > Looked at it more thoroughly now...
> > > So we're now showing both the signature help and the completion, right?
> > > If that the case, LG, but can we include the rest of completion items 
> > > from CHECK-N0? 
> > > 
> > > Maybe also add a comment that the last item is from overload set 
> > > completion, rather than the ordinary code completion? (To avoid confusion 
> > > and make it clear why we need an extra check there)
> > Yes, c-index-test binary collects results from both 
> > ProcessOverloadCandidate and ProcessCodeCompleteResults. Therefore they are 
> > merged. Adding the same set of results on the above is not enough for that 
> > particular case, due to the clever nature of CodeCompleteOverloadResult at 
> > https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaCodeComplete.cpp#L4439.
> > 
> > It doesn't just try to tell you the overloads of that particular function 
> > but also tries to complete the current argument. Therefore the list simply 
> > gets huge by expansion of all the macros and other stuff.
> > 
> > But when looking into it I found out I was checking for wrong return values 
> > fixing that with the new diff.
> I don't think it's correct to call CodeCompleteExpression and 
> CodeCompleteOrdinaryName in this context.
> This completion assume we're at the start of the argument, which is not true 
> anymore.
> E.g. we can be producing weird results in the middle of expressions. Some 
> examples from the top of my head:
> ```
> func(var.^); // <-- (1) we add top-level completions in addition to members 
> of bar
> func(&^); // <-- (2) we provide incorrect ParamType
> ```
> For (2) if ParamType is `int*`, we would incorrectly uprank items of type 
> `int*` (should uprank items of type `int` instead).
> 
> I'll investigate a bit more to see if we can refactor the code to untangle 
> signature help from code completion.
D51782 removes completion-specific bits out of functions that produce signature 
help. 
After it lands, it should be significantly easier to produce results for these 
cases.

Specifically, we will be able to only run signature help in the middle of the 
args, and signature help followed by a normal completion at the start of the 
args.


Repository:
  rC Clang

https://reviews.llvm.org/D51038



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

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: test/Index/complete-block-property-assignment.m:71
 
+// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck 
-check-prefix=CHECK-NO1 %s
+// CHECK-NO1: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35)

kadircet wrote:
> ilya-biryukov wrote:
> > ilya-biryukov wrote:
> > > Any idea why behavior changed in that case?
> > Looked at it more thoroughly now...
> > So we're now showing both the signature help and the completion, right?
> > If that the case, LG, but can we include the rest of completion items from 
> > CHECK-N0? 
> > 
> > Maybe also add a comment that the last item is from overload set 
> > completion, rather than the ordinary code completion? (To avoid confusion 
> > and make it clear why we need an extra check there)
> Yes, c-index-test binary collects results from both ProcessOverloadCandidate 
> and ProcessCodeCompleteResults. Therefore they are merged. Adding the same 
> set of results on the above is not enough for that particular case, due to 
> the clever nature of CodeCompleteOverloadResult at 
> https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaCodeComplete.cpp#L4439.
> 
> It doesn't just try to tell you the overloads of that particular function but 
> also tries to complete the current argument. Therefore the list simply gets 
> huge by expansion of all the macros and other stuff.
> 
> But when looking into it I found out I was checking for wrong return values 
> fixing that with the new diff.
I don't think it's correct to call CodeCompleteExpression and 
CodeCompleteOrdinaryName in this context.
This completion assume we're at the start of the argument, which is not true 
anymore.
E.g. we can be producing weird results in the middle of expressions. Some 
examples from the top of my head:
```
func(var.^); // <-- (1) we add top-level completions in addition to members of 
bar
func(&^); // <-- (2) we provide incorrect ParamType
```
For (2) if ParamType is `int*`, we would incorrectly uprank items of type 
`int*` (should uprank items of type `int` instead).

I'll investigate a bit more to see if we can refactor the code to untangle 
signature help from code completion.


Repository:
  rC Clang

https://reviews.llvm.org/D51038



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

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: test/Index/complete-block-property-assignment.m:71
 
+// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck 
-check-prefix=CHECK-NO1 %s
+// CHECK-NO1: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35)

ilya-biryukov wrote:
> ilya-biryukov wrote:
> > Any idea why behavior changed in that case?
> Looked at it more thoroughly now...
> So we're now showing both the signature help and the completion, right?
> If that the case, LG, but can we include the rest of completion items from 
> CHECK-N0? 
> 
> Maybe also add a comment that the last item is from overload set completion, 
> rather than the ordinary code completion? (To avoid confusion and make it 
> clear why we need an extra check there)
Yes, c-index-test binary collects results from both ProcessOverloadCandidate 
and ProcessCodeCompleteResults. Therefore they are merged. Adding the same set 
of results on the above is not enough for that particular case, due to the 
clever nature of CodeCompleteOverloadResult at 
https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaCodeComplete.cpp#L4439.

It doesn't just try to tell you the overloads of that particular function but 
also tries to complete the current argument. Therefore the list simply gets 
huge by expansion of all the macros and other stuff.

But when looking into it I found out I was checking for wrong return values 
fixing that with the new diff.


Repository:
  rC Clang

https://reviews.llvm.org/D51038



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

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 164307.
kadircet added a comment.

- Fix tests.


Repository:
  rC Clang

https://reviews.llvm.org/D51038

Files:
  include/clang/Parse/Parser.h
  lib/Parse/ParseExpr.cpp
  test/CodeCompletion/function-overloads-inside-param.cpp
  test/CodeCompletion/function-overloads.cpp
  test/Index/complete-block-property-assignment.m


Index: test/Index/complete-block-property-assignment.m
===
--- test/Index/complete-block-property-assignment.m
+++ test/Index/complete-block-property-assignment.m
@@ -60,13 +60,14 @@
 // RUN: c-index-test -code-completion-at=%s:51:16 %s | FileCheck 
-check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:52:23 %s | FileCheck 
-check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:53:12 %s | FileCheck 
-check-prefix=CHECK-NO %s
-// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck 
-check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:56: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)
 // CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType int (^)(int)}{TypedText 
processEvent} (35)
 
+// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck 
-check-prefix=CHECK-NO1 %s
+// CHECK-NO1: OverloadCandidate:{ResultType void}{Text func}{LeftParen 
(}{CurrentParameter int x}{RightParen )} (1)
 @end
Index: test/CodeCompletion/function-overloads.cpp
===
--- /dev/null
+++ test/CodeCompletion/function-overloads.cpp
@@ -0,0 +1,8 @@
+void f(int i, int j = 2, int k = 5);
+void f(float x, float y...);
+
+void test() {
+  ::f(
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:7 %s -o - | 
FileCheck -check-prefix=CHECK-CC1 %s
+  // CHECK-CC1: f(<#int i#>{#, <#int j = 2#>{#, <#int k = 5#>#}#})
+  // CHECK-CC1: f(<#float x#>, <#float y, ...#>)
Index: test/CodeCompletion/function-overloads-inside-param.cpp
===
--- /dev/null
+++ test/CodeCompletion/function-overloads-inside-param.cpp
@@ -0,0 +1,8 @@
+void f(int i, int j = 2, int k = 5);
+void f(float x, float y...);
+
+void test() {
+  ::f(1
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:8 %s -o - | 
FileCheck -check-prefix=CHECK-CC1 %s
+  // CHECK-CC1: f(<#int i#>{#, <#int j = 2#>{#, <#int k = 5#>#}#})
+  // CHECK-CC1: f(<#float x#>, <#float y, ...#>)
Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -1658,11 +1658,21 @@
 
   if (OpKind == tok::l_paren || !LHS.isInvalid()) {
 if (Tok.isNot(tok::r_paren)) {
-  if (ParseExpressionList(ArgExprs, CommaLocs, [&] {
-Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs,
- PT.getOpenLocation());
-  })) {
+  auto Completer = [&] {
+if (CalledOverloadCompletion)
+  return;
+Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs,
+ PT.getOpenLocation());
+CalledOverloadCompletion = true;
+  };
+  if (ParseExpressionList(ArgExprs, CommaLocs, Completer)) {
 (void)Actions.CorrectDelayedTyposInExpr(LHS);
+// If we got an error when parsing expression list, we don't call
+// the CodeCompleteCall handler inside the parser. So call it here
+// to make sure we get overload suggestions even when we are in the
+// middle of a parameter.
+if (PP.isCodeCompletionReached())
+  Completer();
 LHS = ExprError();
   } else if (LHS.isInvalid()) {
 for (auto  : ArgExprs)
Index: include/clang/Parse/Parser.h
===
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -214,6 +214,10 @@
   /// should not be set directly.
   bool InMessageExpression;
 
+  /// Gets set to true after calling CodeCompleteCall, it is for a workaround 
to
+  /// make sure CodeComleteCall is only called at the deepest level.
+  bool CalledOverloadCompletion = false;
+
   /// The "depth" of the template parameters currently being parsed.
   unsigned TemplateParameterDepth;
 


Index: test/Index/complete-block-property-assignment.m
===
--- test/Index/complete-block-property-assignment.m

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: test/Index/complete-block-property-assignment.m:71
 
+// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck 
-check-prefix=CHECK-NO1 %s
+// CHECK-NO1: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35)

ilya-biryukov wrote:
> Any idea why behavior changed in that case?
Looked at it more thoroughly now...
So we're now showing both the signature help and the completion, right?
If that the case, LG, but can we include the rest of completion items from 
CHECK-N0? 

Maybe also add a comment that the last item is from overload set completion, 
rather than the ordinary code completion? (To avoid confusion and make it clear 
why we need an extra check there)


Repository:
  rC Clang

https://reviews.llvm.org/D51038



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

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: test/Index/complete-block-property-assignment.m:71
 
+// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck 
-check-prefix=CHECK-NO1 %s
+// CHECK-NO1: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35)

Any idea why behavior changed in that case?


Repository:
  rC Clang

https://reviews.llvm.org/D51038



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

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 163971.
kadircet added a comment.

- Update comments.


Repository:
  rC Clang

https://reviews.llvm.org/D51038

Files:
  include/clang/Parse/Parser.h
  lib/Parse/ParseExpr.cpp
  test/CodeCompletion/function-overloads-inside-param.cpp
  test/CodeCompletion/function-overloads.cpp
  test/Index/complete-block-property-assignment.m


Index: test/Index/complete-block-property-assignment.m
===
--- test/Index/complete-block-property-assignment.m
+++ test/Index/complete-block-property-assignment.m
@@ -60,13 +60,15 @@
 // RUN: c-index-test -code-completion-at=%s:51:16 %s | FileCheck 
-check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:52:23 %s | FileCheck 
-check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:53:12 %s | FileCheck 
-check-prefix=CHECK-NO %s
-// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck 
-check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:56: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)
 // CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType int (^)(int)}{TypedText 
processEvent} (35)
 
+// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck 
-check-prefix=CHECK-NO1 %s
+// CHECK-NO1: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35)
+// CHECK-NO1-NEXT: FunctionDecl:{ResultType void}{TypedText func}{LeftParen 
(}{Placeholder int x}{RightParen )} (50)
 @end
Index: test/CodeCompletion/function-overloads.cpp
===
--- /dev/null
+++ test/CodeCompletion/function-overloads.cpp
@@ -0,0 +1,8 @@
+void f(int i, int j = 2, int k = 5);
+void f(float x, float y...);
+
+void test() {
+  ::f(
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:7 %s -o - | 
FileCheck -check-prefix=CHECK-CC1 %s
+  // CHECK-CC1: f(<#int i#>{#, <#int j = 2#>{#, <#int k = 5#>#}#})
+  // CHECK-CC1: f(<#float x#>, <#float y, ...#>)
Index: test/CodeCompletion/function-overloads-inside-param.cpp
===
--- /dev/null
+++ test/CodeCompletion/function-overloads-inside-param.cpp
@@ -0,0 +1,8 @@
+void f(int i, int j = 2, int k = 5);
+void f(float x, float y...);
+
+void test() {
+  ::f(1
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:8 %s -o - | 
FileCheck -check-prefix=CHECK-CC1 %s
+  // CHECK-CC1: f(<#int i#>{#, <#int j = 2#>{#, <#int k = 5#>#}#})
+  // CHECK-CC1: f(<#float x#>, <#float y, ...#>)
Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -1658,11 +1658,21 @@
 
   if (OpKind == tok::l_paren || !LHS.isInvalid()) {
 if (Tok.isNot(tok::r_paren)) {
-  if (ParseExpressionList(ArgExprs, CommaLocs, [&] {
-Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs,
- PT.getOpenLocation());
-  })) {
+  auto Completer = [&] {
+if (CalledOverloadCompletion)
+  return;
+Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs,
+ PT.getOpenLocation());
+CalledOverloadCompletion = true;
+  };
+  if (ParseExpressionList(ArgExprs, CommaLocs, Completer)) {
 (void)Actions.CorrectDelayedTyposInExpr(LHS);
+// If we got an error when parsing expression list, we don't call
+// the CodeCompleteCall handler inside the parser. So call it here
+// to make sure we get overload suggestions even when we are in the
+// middle of a parameter.
+if (PP.isCodeCompletionReached())
+  Completer();
 LHS = ExprError();
   } else if (LHS.isInvalid()) {
 for (auto  : ArgExprs)
Index: include/clang/Parse/Parser.h
===
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -214,6 +214,10 @@
   /// should not be set directly.
   bool InMessageExpression;
 
+  /// Gets set to true after calling CodeCompleteCall, it is for a workaround 
to
+  /// make sure CodeComleteCall is only called at the deepest level.
+  bool CalledOverloadCompletion = false;
+
   /// The "depth" of the template parameters currently being parsed.
   unsigned TemplateParameterDepth;
 


Index: test/Index/complete-block-property-assignment.m

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 163970.
kadircet added a comment.

- Update tests.
- Update comment.
- Rebase.


Repository:
  rC Clang

https://reviews.llvm.org/D51038

Files:
  include/clang/Parse/Parser.h
  lib/Parse/ParseExpr.cpp
  test/CodeCompletion/function-overloads-inside-param.cpp
  test/CodeCompletion/function-overloads.cpp
  test/Index/complete-block-property-assignment.m


Index: test/Index/complete-block-property-assignment.m
===
--- test/Index/complete-block-property-assignment.m
+++ test/Index/complete-block-property-assignment.m
@@ -60,13 +60,15 @@
 // RUN: c-index-test -code-completion-at=%s:51:16 %s | FileCheck 
-check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:52:23 %s | FileCheck 
-check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:53:12 %s | FileCheck 
-check-prefix=CHECK-NO %s
-// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck 
-check-prefix=CHECK-NO %s
 // RUN: c-index-test -code-completion-at=%s:56: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)
 // CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType int (^)(int)}{TypedText 
processEvent} (35)
 
+// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck 
-check-prefix=CHECK-NO1 %s
+// CHECK-NO1: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35)
+// CHECK-NO1-NEXT: FunctionDecl:{ResultType void}{TypedText func}{LeftParen 
(}{Placeholder int x}{RightParen )} (50)
 @end
Index: test/CodeCompletion/function-overloads.cpp
===
--- /dev/null
+++ test/CodeCompletion/function-overloads.cpp
@@ -0,0 +1,8 @@
+void f(int i, int j = 2, int k = 5);
+void f(float x, float y...);
+
+void test() {
+  ::f(
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:7 %s -o - | 
FileCheck -check-prefix=CHECK-CC1 %s
+  // CHECK-CC1: f(<#int i#>{#, <#int j = 2#>{#, <#int k = 5#>#}#})
+  // CHECK-CC1: f(<#float x#>, <#float y, ...#>)
Index: test/CodeCompletion/function-overloads-inside-param.cpp
===
--- /dev/null
+++ test/CodeCompletion/function-overloads-inside-param.cpp
@@ -0,0 +1,8 @@
+void f(int i, int j = 2, int k = 5);
+void f(float x, float y...);
+
+void test() {
+  ::f(1
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:8 %s -o - | 
FileCheck -check-prefix=CHECK-CC1 %s
+  // CHECK-CC1: f(<#int i#>{#, <#int j = 2#>{#, <#int k = 5#>#}#})
+  // CHECK-CC1: f(<#float x#>, <#float y, ...#>)
Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -1658,11 +1658,21 @@
 
   if (OpKind == tok::l_paren || !LHS.isInvalid()) {
 if (Tok.isNot(tok::r_paren)) {
-  if (ParseExpressionList(ArgExprs, CommaLocs, [&] {
-Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs,
- PT.getOpenLocation());
-  })) {
+  auto Completer = [&] {
+if (CalledOverloadCompletion)
+  return;
+Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs,
+ PT.getOpenLocation());
+CalledOverloadCompletion = true;
+  };
+  if (ParseExpressionList(ArgExprs, CommaLocs, Completer)) {
 (void)Actions.CorrectDelayedTyposInExpr(LHS);
+// If we got an error when parsing expression list, we don't call
+// the CodeCompleteCall handler inside the parser. So call it here
+// to make sure we get overload suggestions even when we are in the
+// middle of a parameter.
+if (PP.isCodeCompletionReached())
+  Completer();
 LHS = ExprError();
   } else if (LHS.isInvalid()) {
 for (auto  : ArgExprs)
Index: include/clang/Parse/Parser.h
===
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -214,6 +214,10 @@
   /// should not be set directly.
   bool InMessageExpression;
 
+  /// Gets set to true after calling CodeCompleteCall, it is for a hack to make
+  /// signature help working even when it is triggered inside a token.
+  bool CalledOverloadCompletion = false;
+
   /// The "depth" of the template parameters currently being parsed.
   unsigned TemplateParameterDepth;
 


Index: test/Index/complete-block-property-assignment.m

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

One major limitation of the current workaround is that IIRC clang can actually 
call code completion callbacks, including this method, multiple times (e.g. 
when completing inside a macro definition that gets expanded later in the file 
or during parser recovery).
The workaround will work at the first call, but won't trigger at the repeated 
calls. We could find a way around that by resetting the flag to false at the 
right points (probably at every top-level function call?) WDYT?




Comment at: include/clang/Parse/Parser.h:217
 
+  /// Gets set to true after calling CodeCompleteCall, it is for a hack to make
+  /// signature help working even when it is triggered inside a token.

NIT: would go with something less harsh, like "**workaround** to make sure we 
only call CodeCompleteCall on the deepest call expression"



Comment at: lib/Parse/ParseExpr.cpp:1661
+  auto Completer = [&] {
+Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs);
+CalledOverloadCompletion = true;

Shouldn't we ignore `CodeCompleteCall` if `CalledOverloadCompletion` is set to 
true?



Comment at: lib/Parse/ParseExpr.cpp:1672
+// let the parser do it instead.
+if (!getLangOpts().ObjC1 && PP.isCodeCompletionReached() &&
+!CalledOverloadCompletion)

What's the special handling in ObjC? Can we unify with our workaround?



Comment at: lib/Parse/ParseExpr.cpp:1674
+!CalledOverloadCompletion)
+  Completer();
 LHS = ExprError();

Why don't we handle the `CalledOverloadCompletion` flag inside the 
`Completer()` itself?
We're currently special-casing the middle identifier and error case, why not 
have the same code path for `CodeCompleteCall()` calls at the start of the 
identifier?


Repository:
  rC Clang

https://reviews.llvm.org/D51038



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

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 161966.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Resolve discussions.
- Add tests.
- Fix a bug noticed by lit tests.


Repository:
  rC Clang

https://reviews.llvm.org/D51038

Files:
  include/clang/Parse/Parser.h
  lib/Parse/ParseExpr.cpp
  test/CodeCompletion/function-overloads-inside-param.cpp
  test/CodeCompletion/function-overloads.cpp


Index: test/CodeCompletion/function-overloads.cpp
===
--- /dev/null
+++ test/CodeCompletion/function-overloads.cpp
@@ -0,0 +1,8 @@
+void f(int i, int j = 2, int k = 5);
+void f(float x, float y...);
+
+void test() {
+  ::f(
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:7 %s -o - | 
FileCheck -check-prefix=CHECK-CC1 %s
+  // CHECK-CC1: f(<#int i#>{#, <#int j = 2#>{#, <#int k = 5#>#}#})
+  // CHECK-CC1: f(<#float x#>, <#float y, ...#>)
Index: test/CodeCompletion/function-overloads-inside-param.cpp
===
--- /dev/null
+++ test/CodeCompletion/function-overloads-inside-param.cpp
@@ -0,0 +1,8 @@
+void f(int i, int j = 2, int k = 5);
+void f(float x, float y...);
+
+void test() {
+  ::f(1
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:8 %s -o - | 
FileCheck -check-prefix=CHECK-CC1 %s
+  // CHECK-CC1: f(<#int i#>{#, <#int j = 2#>{#, <#int k = 5#>#}#})
+  // CHECK-CC1: f(<#float x#>, <#float y, ...#>)
Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -1657,10 +1657,21 @@
 
   if (OpKind == tok::l_paren || !LHS.isInvalid()) {
 if (Tok.isNot(tok::r_paren)) {
-  if (ParseExpressionList(ArgExprs, CommaLocs, [&] {
-Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs);
- })) {
+  auto Completer = [&] {
+Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs);
+CalledOverloadCompletion = true;
+  };
+  if (ParseExpressionList(ArgExprs, CommaLocs, Completer)) {
 (void)Actions.CorrectDelayedTyposInExpr(LHS);
+// If we got an error when parsing expression list, we don't call
+// the CodeCompleteCall handler inside the parser. So call it here
+// to make sure we get overload suggestions even when we are in the
+// middle of a parameter. There is a different handling mechanism 
in
+// ObjC for that type of completion, so don't call completion and
+// let the parser do it instead.
+if (!getLangOpts().ObjC1 && PP.isCodeCompletionReached() &&
+!CalledOverloadCompletion)
+  Completer();
 LHS = ExprError();
   } else if (LHS.isInvalid()) {
 for (auto  : ArgExprs)
Index: include/clang/Parse/Parser.h
===
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -214,6 +214,10 @@
   /// should not be set directly.
   bool InMessageExpression;
 
+  /// Gets set to true after calling CodeCompleteCall, it is for a hack to make
+  /// signature help working even when it is triggered inside a token.
+  bool CalledOverloadCompletion = false;
+
   /// The "depth" of the template parameters currently being parsed.
   unsigned TemplateParameterDepth;
 


Index: test/CodeCompletion/function-overloads.cpp
===
--- /dev/null
+++ test/CodeCompletion/function-overloads.cpp
@@ -0,0 +1,8 @@
+void f(int i, int j = 2, int k = 5);
+void f(float x, float y...);
+
+void test() {
+  ::f(
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:7 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+  // CHECK-CC1: f(<#int i#>{#, <#int j = 2#>{#, <#int k = 5#>#}#})
+  // CHECK-CC1: f(<#float x#>, <#float y, ...#>)
Index: test/CodeCompletion/function-overloads-inside-param.cpp
===
--- /dev/null
+++ test/CodeCompletion/function-overloads-inside-param.cpp
@@ -0,0 +1,8 @@
+void f(int i, int j = 2, int k = 5);
+void f(float x, float y...);
+
+void test() {
+  ::f(1
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:8 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+  // CHECK-CC1: f(<#int i#>{#, <#int j = 2#>{#, <#int k = 5#>#}#})
+  // CHECK-CC1: f(<#float x#>, <#float y, ...#>)
Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -1657,10 +1657,21 @@
 
   if (OpKind == tok::l_paren || !LHS.isInvalid()) {
 if (Tok.isNot(tok::r_paren)) {
-  if (ParseExpressionList(ArgExprs, CommaLocs, [&] {
-Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs);
- })) {
+  auto 

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Sorry, missed the dependent revision adding tests to clangd at first. Having 
another test in sema would still be great, to make sure we guard against 
regressions if someone does not have clang-tools-extra checked out.


Repository:
  rC Clang

https://reviews.llvm.org/D51038



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

[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Maybe add tests? Lit tests for code completion (`clang/test/CodeCompletion`) 
should be a good fit




Comment at: include/clang/Parse/Parser.h:2971
+  /// signature help working even when it is triggered inside a token.
+  bool CalledOverloadCompletion = false;
 };

Maybe put the field closer to the existing ones?



Comment at: lib/Parse/ParseExpr.cpp:2820
 Actions.CodeCompleteOrdinaryName(getCurScope(), Sema::PCC_Expression);
+  CalledOverloadCompletion = true;
   cutOffParsing();

Why should we set this flag here? Can it be handled by `Completer` function 
that we pass into `ParseExperssionList` instead?


Repository:
  rC Clang

https://reviews.llvm.org/D51038



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