zyounan updated this revision to Diff 504947.
zyounan marked 3 inline comments as done.
zyounan added a comment.
Address the comments. Revise parameters of `getSignature` to avoid passing CCR
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145319/new/
https://reviews.llvm.org/D145319
Files:
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/CodeCompletionStrings.cpp
clang-tools-extra/clangd/CodeCompletionStrings.h
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
Index: clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
@@ -27,8 +27,16 @@
bool CompletingPattern = false) {
Signature.clear();
Snippet.clear();
- getSignature(CCS, &Signature, &Snippet, /*RequiredQualifier=*/nullptr,
- CompletingPattern);
+ // Note that `getSignature` uses CursorKind to identify if we shouldn't
+ // complete $0 for certain patterns, such as constructors. Passing
+ // CXCursor_NotImplemented to circumvent that logic, thus the behavior of
+ // this function matches that before https://reviews.llvm.org/D145319.
+ getSignature(CCS, &Signature, &Snippet,
+ CompletingPattern
+ ? CodeCompletionResult::ResultKind::RK_Pattern
+ : CodeCompletionResult::ResultKind::RK_Declaration,
+ CXCursorKind::CXCursor_NotImplemented,
+ /*RequiredQualifiers=*/nullptr);
}
std::shared_ptr<clang::GlobalCodeCompletionAllocator> Allocator;
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -3450,6 +3450,22 @@
EXPECT_THAT(Results.Completions,
Contains(AllOf(named("while_foo"),
snippetSuffix("(${1:int a}, ${2:int b})"))));
+
+ Results = completions(R"cpp(
+ struct Base {
+ Base(int a, int b) {}
+ };
+
+ struct Derived : Base {
+ Derived() : Base^
+ };
+ )cpp",
+ /*IndexSymbols=*/{}, Options);
+ // Constructors from base classes are a kind of pattern that shouldn't end
+ // with $0.
+ EXPECT_THAT(Results.Completions,
+ Contains(AllOf(named("Base"),
+ snippetSuffix("(${1:int a}, ${2:int b})"))));
}
TEST(CompletionTest, WorksWithNullType) {
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -756,7 +756,8 @@
*PP, *CompletionAllocator, *CompletionTUInfo);
std::string Signature;
std::string SnippetSuffix;
- getSignature(*CCS, &Signature, &SnippetSuffix);
+ getSignature(*CCS, &Signature, &SnippetSuffix, SymbolCompletion.Kind,
+ SymbolCompletion.CursorKind);
S.Signature = Signature;
S.CompletionSnippetSuffix = SnippetSuffix;
@@ -933,7 +934,8 @@
S.Documentation = Documentation;
std::string Signature;
std::string SnippetSuffix;
- getSignature(*CCS, &Signature, &SnippetSuffix);
+ getSignature(*CCS, &Signature, &SnippetSuffix, SymbolCompletion.Kind,
+ SymbolCompletion.CursorKind);
S.Signature = Signature;
S.CompletionSnippetSuffix = SnippetSuffix;
std::string ReturnType = getReturnType(*CCS);
Index: clang-tools-extra/clangd/CodeCompletionStrings.h
===================================================================
--- clang-tools-extra/clangd/CodeCompletionStrings.h
+++ clang-tools-extra/clangd/CodeCompletionStrings.h
@@ -46,8 +46,9 @@
/// ${0:â¦}, indicating the cursor should stay there.
void getSignature(const CodeCompletionString &CCS, std::string *Signature,
std::string *Snippet,
- std::string *RequiredQualifiers = nullptr,
- bool CompletingPattern = false);
+ CodeCompletionResult::ResultKind ResultKind,
+ CXCursorKind CursorKind,
+ std::string *RequiredQualifiers = nullptr);
/// Assembles formatted documentation for a completion result. This includes
/// documentation comments and other relevant information like annotations.
Index: clang-tools-extra/clangd/CodeCompletionStrings.cpp
===================================================================
--- clang-tools-extra/clangd/CodeCompletionStrings.cpp
+++ clang-tools-extra/clangd/CodeCompletionStrings.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "CodeCompletionStrings.h"
+#include "clang-c/Index.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/RawCommentList.h"
#include "clang/Basic/SourceManager.h"
@@ -95,12 +96,29 @@
}
void getSignature(const CodeCompletionString &CCS, std::string *Signature,
- std::string *Snippet, std::string *RequiredQualifiers,
- bool CompletingPattern) {
+ std::string *Snippet,
+ CodeCompletionResult::ResultKind ResultKind,
+ CXCursorKind CursorKind, std::string *RequiredQualifiers) {
// Placeholder with this index will be ${0:â¦} to mark final cursor position.
// Usually we do not add $0, so the cursor is placed at end of completed text.
unsigned CursorSnippetArg = std::numeric_limits<unsigned>::max();
- if (CompletingPattern) {
+ bool CompletingPattern = ResultKind == CodeCompletionResult::RK_Pattern;
+ // If the result kind of CCR is `RK_Pattern`, it doesn't always mean we're
+ // completing a chunk of statements. Constructors defined in base class, for
+ // example, are considered as a type of pattern, with the cursor type set to
+ // CXCursor_Constructor.
+ // We have to discriminate these cases manually in order to avoid providing
+ // incorrect placeholder `$0` which should have been a normal parameter.
+ bool ShouldPatchPlaceholder0 = CompletingPattern && [CursorKind] {
+ // The process of CCR construction employs `clang::getCursorKindForDecl` to
+ // obtain cursor kind for Decls, otherwise CursorKind would be set by
+ // constructor. Note that the default value is CXCursor_NotImplemented.
+ if (CursorKind == CXCursorKind::CXCursor_Constructor ||
+ CursorKind == CXCursorKind::CXCursor_Destructor)
+ return false;
+ return true;
+ }();
+ if (ShouldPatchPlaceholder0) {
// In patterns, it's best to place the cursor at the last placeholder, to
// handle cases like
// namespace ${1:name} {
@@ -185,14 +203,14 @@
*Snippet += Chunk.Text;
break;
case CodeCompletionString::CK_Optional:
- assert(Chunk.Optional);
+ assert(Chunk.Optional);
// No need to create placeholders for default arguments in Snippet.
appendOptionalChunk(*Chunk.Optional, Signature);
break;
case CodeCompletionString::CK_Placeholder:
*Signature += Chunk.Text;
++SnippetArg;
- if (SnippetArg == CursorSnippetArg) {
+ if (ShouldPatchPlaceholder0 && SnippetArg == CursorSnippetArg) {
// We'd like to make $0 a placeholder too, but vscode does not support
// this (https://github.com/microsoft/vscode/issues/152837).
*Snippet += "$0";
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -436,9 +436,8 @@
Bundled.emplace_back();
BundledEntry &S = Bundled.back();
if (C.SemaResult) {
- bool IsPattern = C.SemaResult->Kind == CodeCompletionResult::RK_Pattern;
- getSignature(*SemaCCS, &S.Signature, &S.SnippetSuffix,
- &Completion.RequiredQualifier, IsPattern);
+ 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);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
