[PATCH] D52308: [Sema][CodeCompletion] Fix return type of C++ destructors in code completion
jkorous abandoned this revision. jkorous added a comment. Allright, I will remove destructor from clangd completion results which solves this particular issue. Also good point about return type being used in ranking - I incorrectly assumed it's just a presentational matter. Repository: rC Clang https://reviews.llvm.org/D52308 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52308: [Sema][CodeCompletion] Fix return type of C++ destructors in code completion
sammccall added a comment.
In https://reviews.llvm.org/D52308#1240680, @jkorous wrote:
> Sorry my bad. You are right, we aren't showing destructors in clangd for
> normal classes. The case where I noticed is kind of a corner case with
> template class.
>
>
> {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
> ---
>
> {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"template T> struct foo {}; template<> struct foo {}; foo::~"}}}
> ---
>
> {"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":76}}}
>
>
>
>
> {
> "detail": "void",
> "filterText": "~foo",
> "insertText": "~foo",
> "insertTextFormat": 1,
> "kind": 2,
> "label": " ~foo()",
> "sortText": "3f2c~foo",
> "textEdit": {
> "newText": "~foo",
> "range": {
> "end": {
> "character": 76,
> "line": 0
> },
> "start": {
> "character": 76,
> "line": 0
> }
> }
> }
> },
>
Indeed, this looks like a bug to me.
Repository:
rC Clang
https://reviews.llvm.org/D52308
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52308: [Sema][CodeCompletion] Fix return type of C++ destructors in code completion
sammccall added a comment.
In https://reviews.llvm.org/D52308#1240642, @jkorous wrote:
> You might be right - I am assuming here that we want consistent behaviour
> between constructors and destructors.
>
> IIUC ctors are currently skipped in code completions (in most cases) but they
> also don't have any type associated while result of their call definitely has
> some type.
Tricky.
**MyClass()** has a type, and should probably have return type MyClass, though
it's pretty clear from the name (one could make the same argument about
destructors).
Note that the "return type matches" ranking heuristics will trigger on these
types so it's not just presentational, I think we should be including the types
for constructors.
But class Derived : MyClass { Derived : **MyClass()** {} }; doesn't have a type
(it's not an expression).
And MyClass::**MyClass()** has a type, but it's probably more likely that the
user is going for a plain **MyClass::MyClass** (e.g. in a class-scoped using
decl) which doesn't have a useful type.
> Currently we are showing destructors in code completion in clangd (with
> detail "void") - that's actually how I noticed this.
>
> I would assume that canonical type in function/method code completion is
> "it's type" not "type of result of it's call". WDYT?
I don't think so, for better or worse it's result of it's call. Or rather: type
of the expression that we guess the user's going to form, or that we suggest.
Again, this seems to give the best code completion presentation, and also best
ranking results (when the expected type of the context is known)
Repository:
rC Clang
https://reviews.llvm.org/D52308
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52308: [Sema][CodeCompletion] Fix return type of C++ destructors in code completion
jkorous added a comment.
Sorry my bad. You are right, we aren't showing destructors in clangd for normal
classes. The case where I noticed is kind of a corner case with template class.
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
---
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"template struct foo {}; template<> struct foo {}; foo::~"}}}
---
{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":76}}}
{
"detail": "void",
"filterText": "~foo",
"insertText": "~foo",
"insertTextFormat": 1,
"kind": 2,
"label": " ~foo()",
"sortText": "3f2c~foo",
"textEdit": {
"newText": "~foo",
"range": {
"end": {
"character": 76,
"line": 0
},
"start": {
"character": 76,
"line": 0
}
}
}
},
Repository:
rC Clang
https://reviews.llvm.org/D52308
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52308: [Sema][CodeCompletion] Fix return type of C++ destructors in code completion
jkorous added a comment. You might be right - I am assuming here that we want consistent behaviour between constructors and destructors. IIUC ctors are currently skipped in code completions (in most cases) but they also don't have any type associated while result of their call definitely has some type. Currently we are showing destructors in code completion in clangd (with detail "void") - that's actually how I noticed this. I would assume that canonical type in function/method code completion is "it's type" not "type of result of it's call". WDYT? Repository: rC Clang https://reviews.llvm.org/D52308 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52308: [Sema][CodeCompletion] Fix return type of C++ destructors in code completion
sammccall added a comment. When you're *calling* a destructor, I believe the expression does have type void. Are we sure this is incorrect? Calling a destructor is really unusual though. IIRC we decided to just not show them in clangd in member context (maybe this is broken or was never implemented, though). Repository: rC Clang https://reviews.llvm.org/D52308 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52308: [Sema][CodeCompletion] Fix return type of C++ destructors in code completion
jkorous created this revision.
jkorous added reviewers: arphaman, vsapsai, sammccall, ilya-biryukov.
jkorous added a project: clang.
Herald added subscribers: cfe-commits, dexonsmith, eraman.
Destructors don't have return type "void", they don't have any return type at
all.
Repository:
rC Clang
https://reviews.llvm.org/D52308
Files:
Index/code-completion.cpp
Index/complete-access-checks.cpp
Index/complete-arrow-dot.cpp
Index/complete-cxx-inline-methods.cpp
Index/complete-qualified.cpp
Index/complete-with-annotations.cpp
Sema/SemaCodeComplete.cpp
Index: Index/complete-with-annotations.cpp
===
--- Index/complete-with-annotations.cpp
+++ Index/complete-with-annotations.cpp
@@ -19,5 +19,5 @@
// CHECK: FieldDecl:{ResultType int}{TypedText member2} (35) ("another annotation", "some annotation")
// CHECK: CXXMethod:{ResultType X &}{TypedText operator=}{LeftParen (}{Placeholder const X &}{RightParen )} (79)
// CHECK: ClassDecl:{TypedText X}{Text ::} (75)
-// CHECK: CXXDestructor:{ResultType void}{TypedText ~X}{LeftParen (}{RightParen )} (79)
+// CHECK: CXXDestructor:{TypedText ~X}{LeftParen (}{RightParen )} (79)
Index: Index/complete-qualified.cpp
===
--- Index/complete-qualified.cpp
+++ Index/complete-qualified.cpp
@@ -17,4 +17,4 @@
// CHECK-CC1: FieldDecl:{ResultType C}{TypedText c} (35)
// CHECK-CC1: ClassDecl:{TypedText Foo} (35)
// CHECK-CC1: CXXMethod:{ResultType Foo &}{TypedText operator=}{LeftParen (}{Placeholder const Foo &}{RightParen )}
-// CHECK-CC1: CXXDestructor:{ResultType void}{TypedText ~Foo}{LeftParen (}{RightParen )} (80)
+// CHECK-CC1: CXXDestructor:{TypedText ~Foo}{LeftParen (}{RightParen )} (80)
Index: Index/complete-cxx-inline-methods.cpp
===
--- Index/complete-cxx-inline-methods.cpp
+++ Index/complete-cxx-inline-methods.cpp
@@ -29,7 +29,7 @@
// CHECK-NEXT: StructDecl:{TypedText Vec}{Text ::} (75)
// CHECK-NEXT: FieldDecl:{ResultType int}{TypedText x} (35)
// CHECK-NEXT: FieldDecl:{ResultType int}{TypedText y} (35)
-// CHECK-NEXT: CXXDestructor:{ResultType void}{TypedText ~Vec}{LeftParen (}{RightParen )} (79)
+// CHECK-NEXT: CXXDestructor:{TypedText ~Vec}{LeftParen (}{RightParen )} (79)
// CHECK-NEXT: Completion contexts:
// CHECK-NEXT: Dot member access
// CHECK-NEXT: Container Kind: StructDecl
Index: Index/complete-arrow-dot.cpp
===
--- Index/complete-arrow-dot.cpp
+++ Index/complete-arrow-dot.cpp
@@ -22,33 +22,33 @@
// CHECK-NOT: CXXMethod:{ResultType X &}{TypedText operator=}{LeftParen (}{Placeholder const X &}{RightParen )} (79) (requires fix-it:{{.*}}
// CHECK-NOT: CXXMethod:{ResultType X &}{TypedText operator=}{LeftParen (}{Placeholder X &&}{RightParen )} (79) (requires fix-it:{{.*}}
// CHECK-NOT: StructDecl:{TypedText X}{Text ::} (75) (requires fix-it:{{.*}}
-// CHECK-NOT: CXXDestructor:{ResultType void}{TypedText ~X}{LeftParen (}{RightParen )} (79) (requires fix-it:{{.*}}
+// CHECK-NOT: CXXDestructor:{TypedText ~X}{LeftParen (}{RightParen )} (79) (requires fix-it:{{.*}}
// CHECK: Completion contexts:
// CHECK-NEXT: Dot member access
// CHECK-WITH-CORRECTION: CXXMethod:{ResultType void}{TypedText doSomething}{LeftParen (}{RightParen )} (34) (requires fix-it:{{.*}}
// CHECK-WITH-CORRECTION-NEXT: FieldDecl:{ResultType int}{TypedText field} (35) (requires fix-it:{{.*}}
// CHECK-WITH-CORRECTION-NEXT: CXXMethod:{ResultType X &}{TypedText operator=}{LeftParen (}{Placeholder const X &}{RightParen )} (79) (requires fix-it:{{.*}}
// CHECK-WITH-CORRECTION-NEXT: CXXMethod:{ResultType X &}{TypedText operator=}{LeftParen (}{Placeholder X &&}{RightParen )} (79) (requires fix-it:{{.*}}
// CHECK-WITH-CORRECTION-NEXT: StructDecl:{TypedText X}{Text ::} (75) (requires fix-it:{{.*}}
-// CHECK-WITH-CORRECTION-NEXT: CXXDestructor:{ResultType void}{TypedText ~X}{LeftParen (}{RightParen )} (79) (requires fix-it:{{.*}}
+// CHECK-WITH-CORRECTION-NEXT: CXXDestructor:{TypedText ~X}{LeftParen (}{RightParen )} (79) (requires fix-it:{{.*}}
// CHECK-WITH-CORRECTION-NEXT: Completion contexts:
// CHECK-WITH-CORRECTION-NEXT: Dot member access
// CHECK-ARROW-TO-DOT-NOT: CXXMethod:{ResultType void}{TypedText doSomething}{LeftParen (}{RightParen )} (34) (requires fix-it:{{.*}}
// CHECK-ARROW-TO-DOT-NOT: FieldDecl:{ResultType int}{TypedText field} (35) (requires fix-it:{{.*}}
// CHECK-ARROW-TO-DOT-NOT: CXXMethod:{ResultType X &}{TypedText operator=}{LeftParen (}{Placeholder const X &}{RightParen )} (79) (requires fix-it:{{.*}}
// CHECK-ARROW-TO-DOT-NOT: CXXMethod:{ResultType X &}{TypedText operator=}{LeftParen (}{Placeholder X &&}{RightParen )} (79) (requires fix-it:{{.*}}
// CHECK-ARROW-TO-DOT-NOT: StructDecl:{TypedText X}{Text ::} (75) (requires fix-it:{{.*}}
-// CHECK-ARROW-TO-DOT-NOT:
