From d9971d0b2e34a6a5ca182089d019c9f079f528af Mon Sep 17 00:00:00 2001 From: Ilya Biryukov Date: Mon, 28 Oct 2019 09:34:21 +0100 Subject: [PATCH] [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 --- clang-tools-extra/clangd/CodeComplete.cpp | 20 ++++++++--- .../clangd/unittests/CodeCompleteTests.cpp | 42 ++++++++++++++++++++++ clang/include/clang/Parse/Parser.h | 6 ++-- clang/include/clang/Sema/CodeCompleteConsumer.h | 13 +++++-- clang/include/clang/Sema/Sema.h | 3 +- clang/lib/Parse/ParseDeclCXX.cpp | 5 ++- clang/lib/Parse/ParseExprCXX.cpp | 13 +++---- clang/lib/Sema/SemaCodeComplete.cpp | 33 +++++++++-------- 8 files changed, 98 insertions(+), 37 deletions(-) diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index bc826ba..db9f9cc 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 @@ private: } std::string summarizeSnippet() const { + if (!GenerateSnippets) + return ""; auto *Snippet = onlyValue<&BundledEntry::SnippetSuffix>(); if (!Snippet) // All bundles are function calls. @@ -476,6 +479,8 @@ private: llvm::SmallVector 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 @@ public: auto RecorderOwner = std::make_unique(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 @@ public: 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() - : ""); + : "", + IsUsingDeclaration ? ", inside using declaration" : ""); }); Recorder = RecorderOwner.get(); @@ -1328,6 +1336,7 @@ public: 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 @@ private: 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 74d37df..20f8989 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 52d1590..5add58f 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -1758,13 +1758,13 @@ private: 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 f7d073f..7293784 100644 --- a/clang/include/clang/Sema/CodeCompleteConsumer.h +++ b/clang/include/clang/Sema/CodeCompleteConsumer.h @@ -339,6 +339,11 @@ public: 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 @@ private: 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 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 @@ public: 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 d5b3582..694b923 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -11216,7 +11216,8 @@ public: 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 6d4a1a4..c6ffbfc 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 a064e4b..77eed54 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 f24c3b2..e4c4264 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(); -- 2.7.4