[clang-tools-extra] d9971d0 - [clangd] Do not insert parentheses when completing a using declaration

Ilya Biryukov via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 28 01:56:59 PDT 2019


Author: Ilya Biryukov
Date: 2019-10-28T09:45:10+01:00
New Revision: d9971d0b2e34a6a5ca182089d019c9f079f528af

URL: https://github.com/llvm/llvm-project/commit/d9971d0b2e34a6a5ca182089d019c9f079f528af
DIFF: https://github.com/llvm/llvm-project/commit/d9971d0b2e34a6a5ca182089d019c9f079f528af.diff

LOG: [clangd] Do not insert parentheses when completing a using declaration

Summary:
Would be nice to also fix this in clang, but that looks like more work
if we want to preserve signatures in informative chunks.

Fixes https://github.com/clangd/clangd/issues/118

Reviewers: kadircet

Reviewed By: kadircet

Subscribers: merge_guards_bot, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D69382

Added: 
    

Modified: 
    clang-tools-extra/clangd/CodeComplete.cpp
    clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
    clang/include/clang/Parse/Parser.h
    clang/include/clang/Sema/CodeCompleteConsumer.h
    clang/include/clang/Sema/Sema.h
    clang/lib/Parse/ParseDeclCXX.cpp
    clang/lib/Parse/ParseExprCXX.cpp
    clang/lib/Sema/SemaCodeComplete.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp
index bc826ba0adcd..db9f9cc0519b 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -253,9 +253,10 @@ struct CodeCompletionBuilder {
                         const IncludeInserter &Includes,
                         llvm::StringRef FileName,
                         CodeCompletionContext::Kind ContextKind,
-                        const CodeCompleteOptions &Opts)
+                        const CodeCompleteOptions &Opts, bool GenerateSnippets)
       : ASTCtx(ASTCtx), ExtractDocumentation(Opts.IncludeComments),
-        EnableFunctionArgSnippets(Opts.EnableFunctionArgSnippets) {
+        EnableFunctionArgSnippets(Opts.EnableFunctionArgSnippets),
+        GenerateSnippets(GenerateSnippets) {
     add(C, SemaCCS);
     if (C.SemaResult) {
       assert(ASTCtx);
@@ -419,6 +420,8 @@ struct CodeCompletionBuilder {
   }
 
   std::string summarizeSnippet() const {
+    if (!GenerateSnippets)
+      return "";
     auto *Snippet = onlyValue<&BundledEntry::SnippetSuffix>();
     if (!Snippet)
       // All bundles are function calls.
@@ -476,6 +479,8 @@ struct CodeCompletionBuilder {
   llvm::SmallVector<BundledEntry, 1> Bundled;
   bool ExtractDocumentation;
   bool EnableFunctionArgSnippets;
+  /// When false, no snippets are generated argument lists.
+  bool GenerateSnippets;
 };
 
 // Determine the symbol ID for a Sema code completion result, if possible.
@@ -1204,6 +1209,7 @@ class CodeCompleteFlow {
   // Sema takes ownership of Recorder. Recorder is valid until Sema cleanup.
   CompletionRecorder *Recorder = nullptr;
   CodeCompletionContext::Kind CCContextKind = CodeCompletionContext::CCC_Other;
+  bool IsUsingDeclaration = false;
   // Counters for logging.
   int NSema = 0, NIndex = 0, NSemaAndIndex = 0, NIdent = 0;
   bool Incomplete = false; // Would more be available with a higher limit?
@@ -1254,6 +1260,7 @@ class CodeCompleteFlow {
     auto RecorderOwner = std::make_unique<CompletionRecorder>(Opts, [&]() {
       assert(Recorder && "Recorder is not set");
       CCContextKind = Recorder->CCContext.getKind();
+      IsUsingDeclaration = Recorder->CCContext.isUsingDeclaration();
       auto Style = getFormatStyleForFile(
           SemaCCInput.FileName, SemaCCInput.Contents, SemaCCInput.VFS.get());
       // If preprocessor was run, inclusions from preprocessor callback should
@@ -1289,11 +1296,12 @@ class CodeCompleteFlow {
       SPAN_ATTACH(Tracer, "sema_completion_kind",
                   getCompletionKindString(CCContextKind));
       log("Code complete: sema context {0}, query scopes [{1}] (AnyScope={2}), "
-          "expected type {3}",
+          "expected type {3}{4}",
           getCompletionKindString(CCContextKind),
           llvm::join(QueryScopes.begin(), QueryScopes.end(), ","), AllScopes,
           PreferredType ? Recorder->CCContext.getPreferredType().getAsString()
-                        : "<none>");
+                        : "<none>",
+          IsUsingDeclaration ? ", inside using declaration" : "");
     });
 
     Recorder = RecorderOwner.get();
@@ -1328,6 +1336,7 @@ class CodeCompleteFlow {
     HeuristicPrefix = guessCompletionPrefix(Content, Offset);
     populateContextWords(Content);
     CCContextKind = CodeCompletionContext::CCC_Recovery;
+    IsUsingDeclaration = false;
     Filter = FuzzyMatcher(HeuristicPrefix.Name);
     auto Pos = offsetToPosition(Content, Offset);
     ReplacedRange.start = ReplacedRange.end = Pos;
@@ -1665,7 +1674,8 @@ class CodeCompleteFlow {
       if (!Builder)
         Builder.emplace(Recorder ? &Recorder->CCSema->getASTContext() : nullptr,
                         Item, SemaCCS, QueryScopes, *Inserter, FileName,
-                        CCContextKind, Opts);
+                        CCContextKind, Opts,
+                        /*GenerateSnippets=*/!IsUsingDeclaration);
       else
         Builder->add(Item, SemaCCS);
     }

diff  --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 74d37df795a7..20f89895279c 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -428,6 +428,48 @@ TEST(CompletionTest, Snippets) {
                      SnippetSuffix("(${1:int i}, ${2:const float f})")));
 }
 
+TEST(CompletionTest, NoSnippetsInUsings) {
+  clangd::CodeCompleteOptions Opts;
+  Opts.EnableSnippets = true;
+  auto Results = completions(
+      R"cpp(
+      namespace ns {
+        int func(int a, int b);
+      }
+
+      using ns::^;
+      )cpp",
+      /*IndexSymbols=*/{}, Opts);
+  EXPECT_THAT(Results.Completions,
+              ElementsAre(AllOf(Named("func"), Labeled("func(int a, int b)"),
+                                SnippetSuffix(""))));
+
+  // Check index completions too.
+  auto Func = func("ns::func");
+  Func.CompletionSnippetSuffix = "(${1:int a}, ${2: int b})";
+  Func.Signature = "(int a, int b)";
+  Func.ReturnType = "void";
+
+  Results = completions(R"cpp(
+      namespace ns {}
+      using ns::^;
+  )cpp",
+                        /*IndexSymbols=*/{Func}, Opts);
+  EXPECT_THAT(Results.Completions,
+              ElementsAre(AllOf(Named("func"), Labeled("func(int a, int b)"),
+                                SnippetSuffix(""))));
+
+  // Check all-scopes completions too.
+  Opts.AllScopes = true;
+  Results = completions(R"cpp(
+      using ^;
+  )cpp",
+                        /*IndexSymbols=*/{Func}, Opts);
+  EXPECT_THAT(Results.Completions,
+              Contains(AllOf(Named("func"), Labeled("ns::func(int a, int b)"),
+                             SnippetSuffix(""))));
+}
+
 TEST(CompletionTest, Kinds) {
   auto Results = completions(
       R"cpp(

diff  --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 52d159062cd7..5add58fd5936 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1758,13 +1758,13 @@ class Parser : public CodeCompletionHandler {
                                   bool EnteringContext, IdentifierInfo &II,
                                   CXXScopeSpec &SS);
 
-  bool ParseOptionalCXXScopeSpecifier(CXXScopeSpec &SS,
-                                      ParsedType ObjectType,
+  bool ParseOptionalCXXScopeSpecifier(CXXScopeSpec &SS, ParsedType ObjectType,
                                       bool EnteringContext,
                                       bool *MayBePseudoDestructor = nullptr,
                                       bool IsTypename = false,
                                       IdentifierInfo **LastII = nullptr,
-                                      bool OnlyNamespace = false);
+                                      bool OnlyNamespace = false,
+                                      bool InUsingDeclaration = false);
 
   //===--------------------------------------------------------------------===//
   // C++11 5.1.2: Lambda expressions

diff  --git a/clang/include/clang/Sema/CodeCompleteConsumer.h b/clang/include/clang/Sema/CodeCompleteConsumer.h
index f7d073f48bfb..7293784f894b 100644
--- a/clang/include/clang/Sema/CodeCompleteConsumer.h
+++ b/clang/include/clang/Sema/CodeCompleteConsumer.h
@@ -339,6 +339,11 @@ class CodeCompletionContext {
 private:
   Kind CCKind;
 
+  /// Indicates whether we are completing a name of a using declaration, e.g.
+  ///     using ^;
+  ///     using a::^;
+  bool IsUsingDeclaration;
+
   /// The type that would prefer to see at this point (e.g., the type
   /// of an initializer or function parameter).
   QualType PreferredType;
@@ -359,12 +364,13 @@ class CodeCompletionContext {
 
 public:
   /// Construct a new code-completion context of the given kind.
-  CodeCompletionContext(Kind CCKind) : CCKind(CCKind), SelIdents(None) {}
+  CodeCompletionContext(Kind CCKind)
+      : CCKind(CCKind), IsUsingDeclaration(false), SelIdents(None) {}
 
   /// Construct a new code-completion context of the given kind.
   CodeCompletionContext(Kind CCKind, QualType T,
                         ArrayRef<IdentifierInfo *> SelIdents = None)
-      : CCKind(CCKind), SelIdents(SelIdents) {
+      : CCKind(CCKind), IsUsingDeclaration(false), SelIdents(SelIdents) {
     if (CCKind == CCC_DotMemberAccess || CCKind == CCC_ArrowMemberAccess ||
         CCKind == CCC_ObjCPropertyAccess || CCKind == CCC_ObjCClassMessage ||
         CCKind == CCC_ObjCInstanceMessage)
@@ -373,6 +379,9 @@ class CodeCompletionContext {
       PreferredType = T;
   }
 
+  bool isUsingDeclaration() const { return IsUsingDeclaration; }
+  void setIsUsingDeclaration(bool V) { IsUsingDeclaration = V; }
+
   /// Retrieve the kind of code-completion context.
   Kind getKind() const { return CCKind; }
 

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index d5b3582655f2..694b923160aa 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -11216,7 +11216,8 @@ class Sema {
   void CodeCompleteAfterIf(Scope *S);
 
   void CodeCompleteQualifiedId(Scope *S, CXXScopeSpec &SS, bool EnteringContext,
-                               QualType BaseType, QualType PreferredType);
+                               bool IsUsingDeclaration, QualType BaseType,
+                               QualType PreferredType);
   void CodeCompleteUsing(Scope *S);
   void CodeCompleteUsingDirective(Scope *S);
   void CodeCompleteNamespaceDecl(Scope *S);

diff  --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 6d4a1a4a4e87..c6ffbfc968d0 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -600,7 +600,10 @@ bool Parser::ParseUsingDeclarator(DeclaratorContext Context,
   if (ParseOptionalCXXScopeSpecifier(D.SS, nullptr, /*EnteringContext=*/false,
                                      /*MayBePseudoDtor=*/nullptr,
                                      /*IsTypename=*/false,
-                                     /*LastII=*/&LastII))
+                                     /*LastII=*/&LastII,
+                                     /*OnlyNamespace=*/false,
+                                     /*InUsingDeclaration=*/true))
+
     return true;
   if (D.SS.isInvalid())
     return true;

diff  --git a/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp
index a064e4b17587..77eed5437609 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -143,13 +143,10 @@ void Parser::CheckForTemplateAndDigraph(Token &Next, ParsedType ObjectType,
 /// \param OnlyNamespace If true, only considers namespaces in lookup.
 ///
 /// \returns true if there was an error parsing a scope specifier
-bool Parser::ParseOptionalCXXScopeSpecifier(CXXScopeSpec &SS,
-                                            ParsedType ObjectType,
-                                            bool EnteringContext,
-                                            bool *MayBePseudoDestructor,
-                                            bool IsTypename,
-                                            IdentifierInfo **LastII,
-                                            bool OnlyNamespace) {
+bool Parser::ParseOptionalCXXScopeSpecifier(
+    CXXScopeSpec &SS, ParsedType ObjectType, bool EnteringContext,
+    bool *MayBePseudoDestructor, bool IsTypename, IdentifierInfo **LastII,
+    bool OnlyNamespace, bool InUsingDeclaration) {
   assert(getLangOpts().CPlusPlus &&
          "Call sites of this function should be guarded by checking for C++");
 
@@ -240,7 +237,7 @@ bool Parser::ParseOptionalCXXScopeSpecifier(CXXScopeSpec &SS,
         // Code completion for a nested-name-specifier, where the code
         // completion token follows the '::'.
         Actions.CodeCompleteQualifiedId(getCurScope(), SS, EnteringContext,
-                                        ObjectType.get(),
+                                        InUsingDeclaration, ObjectType.get(),
                                         SavedType.get(SS.getBeginLoc()));
         // Include code completion token into the range of the scope otherwise
         // when we try to annotate the scope tokens the dangling code completion

diff  --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp
index f24c3b234ff2..e4c4264d9dc2 100644
--- a/clang/lib/Sema/SemaCodeComplete.cpp
+++ b/clang/lib/Sema/SemaCodeComplete.cpp
@@ -5330,18 +5330,21 @@ void Sema::CodeCompleteAfterIf(Scope *S) {
 }
 
 void Sema::CodeCompleteQualifiedId(Scope *S, CXXScopeSpec &SS,
-                                   bool EnteringContext, QualType BaseType,
+                                   bool EnteringContext,
+                                   bool IsUsingDeclaration, QualType BaseType,
                                    QualType PreferredType) {
   if (SS.isEmpty() || !CodeCompleter)
     return;
 
+  CodeCompletionContext CC(CodeCompletionContext::CCC_Symbol, PreferredType);
+  CC.setIsUsingDeclaration(IsUsingDeclaration);
+  CC.setCXXScopeSpecifier(SS);
+
   // We want to keep the scope specifier even if it's invalid (e.g. the scope
   // "a::b::" is not corresponding to any context/namespace in the AST), since
   // it can be useful for global code completion which have information about
   // contexts/symbols that are not in the AST.
   if (SS.isInvalid()) {
-    CodeCompletionContext CC(CodeCompletionContext::CCC_Symbol, PreferredType);
-    CC.setCXXScopeSpecifier(SS);
     // As SS is invalid, we try to collect accessible contexts from the current
     // scope with a dummy lookup so that the completion consumer can try to
     // guess what the specified scope is.
@@ -5371,10 +5374,8 @@ void Sema::CodeCompleteQualifiedId(Scope *S, CXXScopeSpec &SS,
   if (!isDependentScopeSpecifier(SS) && RequireCompleteDeclContext(SS, Ctx))
     return;
 
-  ResultBuilder Results(
-      *this, CodeCompleter->getAllocator(),
-      CodeCompleter->getCodeCompletionTUInfo(),
-      CodeCompletionContext(CodeCompletionContext::CCC_Symbol, PreferredType));
+  ResultBuilder Results(*this, CodeCompleter->getAllocator(),
+                        CodeCompleter->getCodeCompletionTUInfo(), CC);
   if (!PreferredType.isNull())
     Results.setPreferredType(PreferredType);
   Results.EnterNewScope();
@@ -5403,23 +5404,21 @@ void Sema::CodeCompleteQualifiedId(Scope *S, CXXScopeSpec &SS,
                        CodeCompleter->loadExternal());
   }
 
-  auto CC = Results.getCompletionContext();
-  CC.setCXXScopeSpecifier(SS);
-
-  HandleCodeCompleteResults(this, CodeCompleter, CC, Results.data(),
-                            Results.size());
+  HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+                            Results.data(), Results.size());
 }
 
 void Sema::CodeCompleteUsing(Scope *S) {
   if (!CodeCompleter)
     return;
 
+  // This can be both a using alias or using declaration, in the former we
+  // expect a new name and a symbol in the latter case.
+  CodeCompletionContext Context(CodeCompletionContext::CCC_SymbolOrNewName);
+  Context.setIsUsingDeclaration(true);
+
   ResultBuilder Results(*this, CodeCompleter->getAllocator(),
-                        CodeCompleter->getCodeCompletionTUInfo(),
-                        // This can be both a using alias or using
-                        // declaration, in the former we expect a new name and a
-                        // symbol in the latter case.
-                        CodeCompletionContext::CCC_SymbolOrNewName,
+                        CodeCompleter->getCodeCompletionTUInfo(), Context,
                         &ResultBuilder::IsNestedNameSpecifier);
   Results.EnterNewScope();
 


        


More information about the cfe-commits mailing list