From: Tom Praschan <13141438+tom-anders@users.noreply.github.com> Date: Sun, 29 Jan 2023 16:02:56 +0000 (+0100) Subject: [clangd] Fix getQueryScopes for using-directive with inline namespace X-Git-Tag: upstream/17.0.6~18056 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=ce87b031437071f011026bb850a2fb2e5f9a72b4;p=platform%2Fupstream%2Fllvm.git [clangd] Fix getQueryScopes for using-directive with inline namespace For example, in the following code ``` using namespace std::string_literals; int main() { strin^ // Completes `string` instead of `std::string` } ``` The using declaration would make completion drop the std namespace, even though it shouldn't. printNamespaceScope() skips inline namespaces, so to fix this use printQualifiedName() instead See https://github.com/clangd/clangd/issues/1451 Differential Revision: https://reviews.llvm.org/D140915 --- diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 708e6f1..04494f9 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -313,7 +313,7 @@ struct ScoredBundleGreater { struct CodeCompletionBuilder { CodeCompletionBuilder(ASTContext *ASTCtx, const CompletionCandidate &C, CodeCompletionString *SemaCCS, - llvm::ArrayRef QueryScopes, + llvm::ArrayRef AccessibleScopes, const IncludeInserter &Includes, llvm::StringRef FileName, CodeCompletionContext::Kind ContextKind, @@ -367,7 +367,7 @@ struct CodeCompletionBuilder { // avoids unneeded qualifiers in cases like with `using ns::X`. if (Completion.RequiredQualifier.empty() && !C.SemaResult) { llvm::StringRef ShortestQualifier = C.IndexResult->Scope; - for (llvm::StringRef Scope : QueryScopes) { + for (llvm::StringRef Scope : AccessibleScopes) { llvm::StringRef Qualifier = C.IndexResult->Scope; if (Qualifier.consume_front(Scope) && Qualifier.size() < ShortestQualifier.size()) @@ -646,40 +646,70 @@ struct SpecifiedScope { // // Examples of unqualified completion: // - // "vec^" => {""} - // "using namespace std; vec^" => {"", "std::"} - // "using namespace std; namespace ns { vec^ }" => {"ns::", "std::", ""} + // "vec^" => {""} + // "using namespace std; vec^" => {"", "std::"} + // "namespace ns {inline namespace ni { struct Foo {}}} + // using namespace ns::ni; Fo^ " => {"", "ns::ni::"} + // "using namespace std; namespace ns { vec^ }" => {"ns::", "std::", ""} // // "" for global namespace, "ns::" for normal namespace. std::vector AccessibleScopes; + // This is an overestimate of AccessibleScopes, e.g. it ignores inline + // namespaces, to fetch more relevant symbols from index. + std::vector QueryScopes; // The full scope qualifier as typed by the user (without the leading "::"). // Set if the qualifier is not fully resolved by Sema. std::optional UnresolvedQualifier; - // Construct scopes being queried in indexes. The results are deduplicated. - // This method format the scopes to match the index request representation. - std::vector scopesForIndexQuery() { + std::optional EnclosingNamespace; + + bool AllowAllScopes = false; + + // Scopes that are accessible from current context. Used for dropping + // unnecessary namespecifiers. + std::vector scopesForQualification() { std::set Results; for (llvm::StringRef AS : AccessibleScopes) Results.insert( (AS + (UnresolvedQualifier ? *UnresolvedQualifier : "")).str()); return {Results.begin(), Results.end()}; } + + // Construct scopes being queried in indexes. The results are deduplicated. + // This method formats the scopes to match the index request representation. + std::vector scopesForIndexQuery() { + // The enclosing namespace must be first, it gets a quality boost. + std::vector EnclosingAtFront; + if (EnclosingNamespace.has_value()) + EnclosingAtFront.push_back(*EnclosingNamespace); + std::set Deduplicated; + for (llvm::StringRef S : QueryScopes) + if (S != EnclosingNamespace) + Deduplicated.insert((S + UnresolvedQualifier.value_or("")).str()); + + EnclosingAtFront.reserve(EnclosingAtFront.size() + Deduplicated.size()); + llvm::copy(Deduplicated, std::back_inserter(EnclosingAtFront)); + + return EnclosingAtFront; + } }; // Get all scopes that will be queried in indexes and whether symbols from // any scope is allowed. The first scope in the list is the preferred scope // (e.g. enclosing namespace). -std::pair, bool> -getQueryScopes(CodeCompletionContext &CCContext, const Sema &CCSema, - const CompletionPrefix &HeuristicPrefix, - const CodeCompleteOptions &Opts) { +SpecifiedScope getQueryScopes(CodeCompletionContext &CCContext, + const Sema &CCSema, + const CompletionPrefix &HeuristicPrefix, + const CodeCompleteOptions &Opts) { SpecifiedScope Scopes; for (auto *Context : CCContext.getVisitedContexts()) { - if (isa(Context)) - Scopes.AccessibleScopes.push_back(""); // global namespace - else if (isa(Context)) - Scopes.AccessibleScopes.push_back(printNamespaceScope(*Context)); + if (isa(Context)) { + Scopes.QueryScopes.push_back(""); + Scopes.AccessibleScopes.push_back(""); + } else if (const auto *ND = dyn_cast(Context)) { + Scopes.QueryScopes.push_back(printNamespaceScope(*Context)); + Scopes.AccessibleScopes.push_back(printQualifiedName(*ND) + "::"); + } } const CXXScopeSpec *SemaSpecifier = @@ -692,39 +722,40 @@ getQueryScopes(CodeCompletionContext &CCContext, const Sema &CCSema, vlog("Sema said no scope specifier, but we saw {0} in the source code", HeuristicPrefix.Qualifier); StringRef SpelledSpecifier = HeuristicPrefix.Qualifier; - if (SpelledSpecifier.consume_front("::")) + if (SpelledSpecifier.consume_front("::")) { Scopes.AccessibleScopes = {""}; + Scopes.QueryScopes = {""}; + } Scopes.UnresolvedQualifier = std::string(SpelledSpecifier); - return {Scopes.scopesForIndexQuery(), false}; - } - // The enclosing namespace must be first, it gets a quality boost. - std::vector EnclosingAtFront; - std::string EnclosingScope = printNamespaceScope(*CCSema.CurContext); - EnclosingAtFront.push_back(EnclosingScope); - for (auto &S : Scopes.scopesForIndexQuery()) { - if (EnclosingScope != S) - EnclosingAtFront.push_back(std::move(S)); + return Scopes; } + /// FIXME: When the enclosing namespace contains an inline namespace, + /// it's dropped here. This leads to a behavior similar to + /// https://github.com/clangd/clangd/issues/1451 + Scopes.EnclosingNamespace = printNamespaceScope(*CCSema.CurContext); // Allow AllScopes completion as there is no explicit scope qualifier. - return {EnclosingAtFront, Opts.AllScopes}; + Scopes.AllowAllScopes = Opts.AllScopes; + return Scopes; } // Case 3: sema saw and resolved a scope qualifier. if (SemaSpecifier && SemaSpecifier->isValid()) - return {Scopes.scopesForIndexQuery(), false}; + return Scopes; // Case 4: There was a qualifier, and Sema didn't resolve it. - Scopes.AccessibleScopes.push_back(""); // Make sure global scope is included. + Scopes.QueryScopes.push_back(""); // Make sure global scope is included. llvm::StringRef SpelledSpecifier = Lexer::getSourceText( CharSourceRange::getCharRange(SemaSpecifier->getRange()), CCSema.SourceMgr, clang::LangOptions()); - if (SpelledSpecifier.consume_front("::")) - Scopes.AccessibleScopes = {""}; + if (SpelledSpecifier.consume_front("::")) + Scopes.QueryScopes = {""}; Scopes.UnresolvedQualifier = std::string(SpelledSpecifier); // Sema excludes the trailing "::". if (!Scopes.UnresolvedQualifier->empty()) *Scopes.UnresolvedQualifier += "::"; - return {Scopes.scopesForIndexQuery(), false}; + Scopes.AccessibleScopes = Scopes.QueryScopes; + + return Scopes; } // Should we perform index-based completion in a context of the specified kind? @@ -1464,6 +1495,7 @@ class CodeCompleteFlow { std::optional Filter; // Initialized once Sema runs. Range ReplacedRange; std::vector QueryScopes; // Initialized once Sema runs. + std::vector AccessibleScopes; // Initialized once Sema runs. // Initialized once QueryScopes is initialized, if there are scopes. std::optional ScopeProximity; std::optional PreferredType; // Initialized once Sema runs. @@ -1623,21 +1655,22 @@ public: // - accessible scopes are determined heuristically. // - all-scopes query if no qualifier was typed (and it's allowed). SpecifiedScope Scopes; - Scopes.AccessibleScopes = visibleNamespaces( + Scopes.QueryScopes = visibleNamespaces( Content.take_front(Offset), format::getFormattingLangOpts(Style)); - for (std::string &S : Scopes.AccessibleScopes) + for (std::string &S : Scopes.QueryScopes) if (!S.empty()) S.append("::"); // visibleNamespaces doesn't include trailing ::. if (HeuristicPrefix.Qualifier.empty()) AllScopes = Opts.AllScopes; else if (HeuristicPrefix.Qualifier.startswith("::")) { - Scopes.AccessibleScopes = {""}; + Scopes.QueryScopes = {""}; Scopes.UnresolvedQualifier = std::string(HeuristicPrefix.Qualifier.drop_front(2)); } else Scopes.UnresolvedQualifier = std::string(HeuristicPrefix.Qualifier); // First scope is the (modified) enclosing scope. QueryScopes = Scopes.scopesForIndexQuery(); + AccessibleScopes = QueryScopes; ScopeProximity.emplace(QueryScopes); SymbolSlab IndexResults = Opts.Index ? queryIndex() : SymbolSlab(); @@ -1689,8 +1722,12 @@ private: } Filter = FuzzyMatcher( Recorder->CCSema->getPreprocessor().getCodeCompletionFilter()); - std::tie(QueryScopes, AllScopes) = getQueryScopes( + auto SpecifiedScopes = getQueryScopes( Recorder->CCContext, *Recorder->CCSema, HeuristicPrefix, Opts); + + QueryScopes = SpecifiedScopes.scopesForIndexQuery(); + AccessibleScopes = SpecifiedScopes.scopesForQualification(); + AllScopes = SpecifiedScopes.AllowAllScopes; if (!QueryScopes.empty()) ScopeProximity.emplace(QueryScopes); PreferredType = @@ -1963,7 +2000,7 @@ private: : nullptr; if (!Builder) Builder.emplace(Recorder ? &Recorder->CCSema->getASTContext() : nullptr, - Item, SemaCCS, QueryScopes, *Inserter, FileName, + Item, SemaCCS, AccessibleScopes, *Inserter, FileName, CCContextKind, Opts, IsUsingDeclaration, NextTokenKind); else Builder->add(Item, SemaCCS); diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 1973518..56e156d 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -465,6 +465,18 @@ TEST(CompletionTest, Qualifiers) { Not(Contains(AllOf(qualifier(""), named("foo"))))); } +// https://github.com/clangd/clangd/issues/1451 +TEST(CompletionTest, QualificationWithInlineNamespace) { + auto Results = completions(R"cpp( + namespace a { inline namespace b {} } + using namespace a::b; + void f() { Foo^ } + )cpp", + {cls("a::Foo")}); + EXPECT_THAT(Results.Completions, + UnorderedElementsAre(AllOf(qualifier("a::"), named("Foo")))); +} + TEST(CompletionTest, InjectedTypename) { // These are suppressed when accessed as a member... EXPECT_THAT(completions("struct X{}; void foo(){ X().^ }").Completions,