[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.
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 >) +// CHECK-CC3: OVERLOAD: A(<#A &>) +// CHECK-CC4: OVERLOAD: A(int, <#int#>, int) Index: cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp === --- cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp +++ cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp @@ -619,6 +619,10 @@
[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.
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 >) +// CHECK-CC3: OVERLOAD: A(<#A &>) +// CHECK-CC4: OVERLOAD: A(int, <#int#>, int) Index: lib/Sema/CodeCompleteConsumer.cpp === --- lib/Sema/CodeCompleteConsumer.cpp +++ lib/Sema/CodeCompleteConsumer.cpp @@ -619,6 +619,10 @@ OS << "<#" << C.Text << "#>"; break; +// FIXME: We can also print optional parameters of an overload. +case CodeCompletionString::CK_Optional: + break; + default: OS << C.Text; break; } } Index: lib/Parse/ParseOpenMP.cpp === --- lib/Parse/ParseOpenMP.cpp +++ lib/Parse/ParseOpenMP.cpp @@ -422,8 +422,15 @@ getCurScope(), OmpPrivParm->getType()->getCanonicalTypeInternal(), OmpPrivParm->getLocation(), Exprs, LParLoc); + CalledSignatureHelp = true; Actions.CodeCompleteExpression(getCurScope(), PreferredType); })) { + if (PP.isCodeCompletionReached() && !CalledSignatureHelp) { +Actions.ProduceConstructorSignatureHelp( +getCurScope(), OmpPrivParm->getType()->getCanonicalTypeInternal(), +OmpPr
[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.
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.
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 >) +// CHECK-CC3: OVERLOAD: A(<#A &>) +// CHECK-CC4: OVERLOAD: A(int, <#int#>, int) Index: lib/Sema/CodeCompleteConsumer.cpp === --- lib/Sema/CodeCompleteConsumer.cpp +++ lib/Sema/CodeCompleteConsumer.cpp @@ -619,6 +619,10 @@ OS << "<#" << C.Text << "#>"; break; +// FIXME: We can also print optional parameters of an overload. +case CodeCompletionString::CK_Optional: + break; + default: OS << C.Text; break; } } Index: lib/Parse/ParseOpenMP.cpp === --- lib/Parse/ParseOpenMP.cpp +++ lib/Parse/ParseOpenMP.cpp @@ -417,13 +417,20 @@ SourceLocation LParLoc = T.getOpenLocation(); if (ParseExpressionList( -Exprs, CommaLocs, [this, OmpPrivParm, LParLoc, &Exprs] { +Exprs, CommaLocs, [this, OmpPrivParm, LParLoc, &Exprs]() { QualType PreferredType = Actions.ProduceConstructorSignatureHelp( getCurScope(), OmpPrivParm->getType()->getCanonicalTypeInternal(), OmpPrivParm->getLocation(), Exprs, LParLoc); + CalledSignatureHelp = true; Actions.CodeCompleteExpression(g
[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.
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 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 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, &Exprs] { - QualType PreferredType = Actions.ProduceConstructorSignatureHelp( - getCurScope(), - OmpPrivParm->getType()->getCanonicalTypeInternal(), - OmpPrivParm->getLocation(), Exprs, LParLoc); - Actions.CodeCompleteExpression(getCurScope(), PreferredType); -})) { +auto Completer = [this, OmpPrivParm, LParLoc, &Exprs]() { + 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.
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 >) +// CHECK-CC3: OVERLOAD: A(<#A &>) +// CHECK-CC4: OVERLOAD: A(int, <#int#>, int) Index: lib/Sema/CodeCompleteConsumer.cpp === --- lib/Sema/CodeCompleteConsumer.cpp +++ lib/Sema/CodeCompleteConsumer.cpp @@ -619,6 +619,9 @@ OS << "<#" << C.Text << "#>"; break; +case CodeCompletionString::CK_Optional: + break; + default: OS << C.Text; break; } } Index: lib/Parse/ParseOpenMP.cpp === --- lib/Parse/ParseOpenMP.cpp +++ lib/Parse/ParseOpenMP.cpp @@ -416,14 +416,20 @@ CommaLocsTy CommaLocs; SourceLocation LParLoc = T.getOpenLocation(); -if (ParseExpressionList( -Exprs, CommaLocs, [this, OmpPrivParm, LParLoc, &Exprs] { - QualType PreferredType = Actions.ProduceConstructorSignatureHelp( - getCurScope(), - OmpPrivParm->getType()->getCanonicalTypeInternal(), - OmpPrivParm->getLocation(), Exprs, LParLoc); - Actions.CodeCompleteExpression(getCurScope(), PreferredType); -})) { +auto Completer = [this, OmpPrivParm, LParLoc, &Exprs]() {
[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.
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 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.
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.
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 >) +// CHECK-CC3: OVERLOAD: A(<#A &>) +// CHECK-CC4: OVERLOAD: A(int, <#int#>, int) Index: lib/Sema/CodeCompleteConsumer.cpp === --- lib/Sema/CodeCompleteConsumer.cpp +++ lib/Sema/CodeCompleteConsumer.cpp @@ -619,6 +619,9 @@ OS << "<#" << C.Text << "#>"; break; +case CodeCompletionString::CK_Optional: + break; + default: OS << C.Text; break; } } Index: lib/Parse/ParseOpenMP.cpp === --- lib/Parse/ParseOpenMP.cpp +++ lib/Parse/ParseOpenMP.cpp @@ -416,14 +416,20 @@ CommaLocsTy CommaLocs; SourceLocation LParLoc = T.getOpenLocation(); -if (ParseExpressionList( -Exprs, CommaLocs, [this, OmpPrivParm, LParLoc, &Exprs] { - QualType PreferredType = Actions.ProduceConstructorSignatureHelp( - getCurScope(), - OmpPrivParm->getType()->getCanonicalTypeInternal(), - OmpPrivParm->getLocation(), Exprs, LParLoc); - Actions.CodeCompleteExpression(getCurScope(), Preferr
[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.
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 &E : 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.
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 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 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 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.
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.
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 &E : 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 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 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.
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 &E : 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.
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 &E : 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 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.
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 &E : 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 Comp
[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.
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 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