zyounan created this revision.
zyounan added reviewers: nridge, tom-anders, sammccall.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
zyounan added a comment.
zyounan updated this revision to Diff 540687.
zyounan published this revision for review.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.
Changing the code completion string to `RK_Pattern` makes the completion
clearer, in a way that showing candidates with signatures in the completion
items if `FunctionCanBeCall` while a mere name otherwise.
F28256820: image.png <https://reviews.llvm.org/F28256820>
F28256841: image.png <https://reviews.llvm.org/F28256841>
(Comparing to that we always show signatures even if no parameters are produced
when hit the item.)
F28256859: image.png <https://reviews.llvm.org/F28256859> -> F28256863:
image.png <https://reviews.llvm.org/F28256863>
---
P.S. Despite the fact that I changed many associated tests, I still hope people
don't rely on the former behavior so that this patch could be marked as NFC. :D
I discovered that patch when trying to figure out why clangd doesn't complete
function parameters for members in global scope while it produces parameters
when I'm inside a call expression. Regard to the heuristic itself, it's
currently a bit inconvenient that I have to switch headers and sources back and
forth and do copy-pastes when I'm writing member function definitions outside
of a class. I think we could be more lenient here e.g., don't set the flag on
if we're in the global scope. What do you guys think? (I'm working on another
follow-up patch to address this, although it is not strightforward to tell
which scope the building Declarator is in inside Sema. )
zyounan added a comment.
Format
This tries to avoid extra calculations in D137040
<https://reviews.llvm.org/D137040>.
In that patch the extra completion strings were dropped at the client
site, after function parameters became available, to implement the
heuristic. However, we can make the process a bit more terse. In the
context where a call isn't required, we could teach the Sema to emit
the completion string without additional parameters by changing the Decl
to a Pattern with the function name.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D155370
Files:
clang-tools-extra/clangd/CodeComplete.cpp
clang/lib/Sema/SemaCodeComplete.cpp
clang/test/CodeCompletion/member-access.cpp
clang/test/Index/complete-qualified.cpp
clang/unittests/Sema/CodeCompleteTest.cpp
Index: clang/unittests/Sema/CodeCompleteTest.cpp
===================================================================
--- clang/unittests/Sema/CodeCompleteTest.cpp
+++ clang/unittests/Sema/CodeCompleteTest.cpp
@@ -59,14 +59,12 @@
unsigned NumResults) override {
for (unsigned I = 0; I < NumResults; ++I) {
auto R = Results[I];
- if (R.Kind == CodeCompletionResult::RK_Declaration) {
- if (const auto *FD = llvm::dyn_cast<FunctionDecl>(R.getDeclaration())) {
- CompletedFunctionDecl D;
- D.Name = FD->getNameAsString();
- D.CanBeCall = R.FunctionCanBeCall;
- D.IsStatic = FD->isStatic();
- CompletedFuncDecls.emplace_back(std::move(D));
- }
+ if (const auto *FD = llvm::dyn_cast<FunctionDecl>(R.getDeclaration())) {
+ CompletedFunctionDecl D;
+ D.Name = FD->getNameAsString();
+ D.CanBeCall = R.FunctionCanBeCall;
+ D.IsStatic = FD->isStatic();
+ CompletedFuncDecls.emplace_back(std::move(D));
}
}
}
Index: clang/test/Index/complete-qualified.cpp
===================================================================
--- clang/test/Index/complete-qualified.cpp
+++ clang/test/Index/complete-qualified.cpp
@@ -16,5 +16,6 @@
// RUN: c-index-test -code-completion-at=%s:14:8 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
// CHECK-CC1: FieldDecl:{ResultType C<Foo, class Bar>}{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: CXXMethod:{TypedText operator=} (50)
+// CHECK-CC1: CXXMethod:{TypedText operator=} (50)
+// CHECK-CC1: CXXMethod:{TypedText ~Foo} (50)
Index: clang/test/CodeCompletion/member-access.cpp
===================================================================
--- clang/test/CodeCompletion/member-access.cpp
+++ clang/test/CodeCompletion/member-access.cpp
@@ -171,7 +171,7 @@
template<typename T>
void dependentColonColonCompletion() {
Template<T>::staticFn();
-// CHECK-CC7: function : [#void#]function()
+// CHECK-CC7: Pattern : function
// CHECK-CC7: Nested : Nested
// CHECK-CC7: o1 : [#BaseTemplate<int>#]o1
// CHECK-CC7: o2 : [#BaseTemplate<T>#]o2
Index: clang/lib/Sema/SemaCodeComplete.cpp
===================================================================
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -1211,6 +1211,13 @@
R.InBaseClass = true;
}
+static void AddTypedNameChunk(ASTContext &Context, const PrintingPolicy &Policy,
+ const NamedDecl *ND,
+ CodeCompletionBuilder &Result);
+
+static PrintingPolicy getCompletionPrintingPolicy(const ASTContext &Context,
+ const Preprocessor &PP);
+
enum class OverloadCompare { BothViable, Dominates, Dominated };
// Will Candidate ever be called on the object, when overloaded with Incumbent?
// Returns Dominates if Candidate is always called, Dominated if Incumbent is
@@ -1400,10 +1407,29 @@
return nullptr;
}();
- R.FunctionCanBeCall =
+ bool FunctionCanBeCall =
CurrentClassScope &&
(CurrentClassScope == Method->getParent() ||
CurrentClassScope->isDerivedFrom(Method->getParent()));
+
+ // Instead of doing calculation for parameters later whose results would
+ // be dropped eventually, we only emit the function name.
+ if (!FunctionCanBeCall) {
+ CodeCompletionBuilder Builder(getAllocator(),
+ getCodeCompletionTUInfo());
+ auto &ASTContext = SemaRef.getASTContext();
+ AddTypedNameChunk(
+ ASTContext,
+ getCompletionPrintingPolicy(ASTContext, SemaRef.getPreprocessor()),
+ Method, Builder);
+ Result R(Builder.TakeString(), /*Priority=*/CCP_Declaration,
+ /*CursorKind=*/CXCursor_CXXMethod,
+ /*Availability=*/CXAvailability_Available,
+ /*D=*/Method);
+ R.FunctionCanBeCall = false;
+ AddResult(std::move(R));
+ return;
+ }
}
}
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -438,8 +438,6 @@
if (C.SemaResult) {
getSignature(*SemaCCS, &S.Signature, &S.SnippetSuffix, C.SemaResult->Kind,
C.SemaResult->CursorKind, &Completion.RequiredQualifier);
- if (!C.SemaResult->FunctionCanBeCall)
- S.SnippetSuffix.clear();
S.ReturnType = getReturnType(*SemaCCS);
} else if (C.IndexResult) {
S.Signature = std::string(C.IndexResult->Signature);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
