zyounan updated this revision to Diff 504950.
zyounan added a comment.

Update doxygen comments


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
@@ -42,12 +42,15 @@
 /// If set, RequiredQualifiers is the text that must be typed before the name.
 /// e.g "Base::" when calling a base class member function that's hidden.
 ///
-/// When \p CompletingPattern is true, the last placeholder will be of the form
-/// ${0:…}, indicating the cursor should stay there.
+/// When \p ResultKind is RK_Pattern, the last placeholder will be $0,
+/// indicating the cursor should stay there.
+/// Note that for certain \p CursorKind like \p CXCursor_Constructor, $0 won't
+/// be emitted in order to avoid overlapping normal parameters.
 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

Reply via email to