sammccall updated this revision to Diff 194486.
sammccall added a comment.
Update comment
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60503/new/
https://reviews.llvm.org/D60503
Files:
clangd/CodeComplete.cpp
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCompleteTests.cpp
===================================================================
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -2374,19 +2374,19 @@
UnorderedElementsAre(AllOf(Qualifier(""), Named("XYZ"))));
}
-// Regression test: clang parser gets confused here and doesn't report the ns::
-// prefix - naive behavior is to insert it again.
-// However we can recognize this from the source code.
-// Test disabled until we can make it pass.
-TEST(CompletionTest, DISABLED_NamespaceDoubleInsertion) {
+// Clang parser gets confused here and doesn't report the ns:: prefix.
+// Naive behavior is to insert it again. We examine the source and recover.
+TEST(CompletionTest, NamespaceDoubleInsertion) {
clangd::CodeCompleteOptions Opts = {};
auto Results = completions(R"cpp(
+ namespace foo {
namespace ns {}
#define M(X) < X
M(ns::ABC^
+ }
)cpp",
- {cls("ns::ABCDE")}, Opts);
+ {cls("foo::ns::ABCDE")}, Opts);
EXPECT_THAT(Results.Completions,
UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE"))));
}
Index: clangd/CodeComplete.cpp
===================================================================
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -45,6 +45,7 @@
#include "clang/Frontend/FrontendActions.h"
#include "clang/Lex/PreprocessorOptions.h"
#include "clang/Sema/CodeCompleteConsumer.h"
+#include "clang/Sema/DeclSpec.h"
#include "clang/Sema/Sema.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/None.h"
@@ -539,54 +540,59 @@
// (e.g. enclosing namespace).
std::pair<std::vector<std::string>, bool>
getQueryScopes(CodeCompletionContext &CCContext, const Sema &CCSema,
+ const CompletionPrefix &HeuristicPrefix,
const CodeCompleteOptions &Opts) {
- auto GetAllAccessibleScopes = [](CodeCompletionContext &CCContext) {
- SpecifiedScope Info;
- for (auto *Context : CCContext.getVisitedContexts()) {
- if (isa<TranslationUnitDecl>(Context))
- Info.AccessibleScopes.push_back(""); // global namespace
- else if (isa<NamespaceDecl>(Context))
- Info.AccessibleScopes.push_back(printNamespaceScope(*Context));
- }
- return Info;
- };
-
- auto SS = CCContext.getCXXScopeSpecifier();
+ SpecifiedScope Scopes;
+ for (auto *Context : CCContext.getVisitedContexts()) {
+ if (isa<TranslationUnitDecl>(Context))
+ Scopes.AccessibleScopes.push_back(""); // global namespace
+ else if (isa<NamespaceDecl>(Context))
+ Scopes.AccessibleScopes.push_back(printNamespaceScope(*Context));
+ }
- // Unqualified completion (e.g. "vec^").
- if (!SS) {
- std::vector<std::string> Scopes;
+ const CXXScopeSpec *SemaSpecifier =
+ CCContext.getCXXScopeSpecifier().getValueOr(nullptr);
+ // Case 1: unqualified completion.
+ if (!SemaSpecifier) {
+ // Case 2 (exception): sema saw no qualifier, but there appears to be one!
+ // This can happen e.g. in incomplete macro expansions. Use heuristics.
+ if (!HeuristicPrefix.Qualifier.empty()) {
+ vlog("Sema said no scope specifier, but we saw {0} in the source code",
+ HeuristicPrefix.Qualifier);
+ StringRef SpelledSpecifier = HeuristicPrefix.Qualifier;
+ if (SpelledSpecifier.consume_front("::"))
+ Scopes.AccessibleScopes = {""};
+ Scopes.UnresolvedQualifier = SpelledSpecifier;
+ return {Scopes.scopesForIndexQuery(), false};
+ }
+ // The enclosing namespace must be first, it gets a quality boost.
+ std::vector<std::string> EnclosingAtFront;
std::string EnclosingScope = printNamespaceScope(*CCSema.CurContext);
- Scopes.push_back(EnclosingScope);
- for (auto &S : GetAllAccessibleScopes(CCContext).scopesForIndexQuery()) {
+ EnclosingAtFront.push_back(EnclosingScope);
+ for (auto &S : Scopes.scopesForIndexQuery()) {
if (EnclosingScope != S)
- Scopes.push_back(std::move(S));
+ EnclosingAtFront.push_back(std::move(S));
}
- // Allow AllScopes completion only for there is no explicit scope qualifier.
- return {Scopes, Opts.AllScopes};
+ // Allow AllScopes completion as there is no explicit scope qualifier.
+ return {EnclosingAtFront, Opts.AllScopes};
}
-
- // Qualified completion ("std::vec^"), we have two cases depending on whether
- // the qualifier can be resolved by Sema.
- if ((*SS)->isValid()) { // Resolved qualifier.
- return {GetAllAccessibleScopes(CCContext).scopesForIndexQuery(), false};
- }
-
- // Unresolved qualifier.
- SpecifiedScope Info = GetAllAccessibleScopes(CCContext);
- Info.AccessibleScopes.push_back(""); // Make sure global scope is included.
-
- llvm::StringRef SpelledSpecifier =
- Lexer::getSourceText(CharSourceRange::getCharRange((*SS)->getRange()),
- CCSema.SourceMgr, clang::LangOptions());
+ // Case 3: sema saw and resolved a scope qualifier.
+ if (SemaSpecifier && SemaSpecifier->isValid())
+ return {Scopes.scopesForIndexQuery(), false};
+
+ // Case 4: There was a qualifier, and Sema didn't resolve it.
+ Scopes.AccessibleScopes.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("::"))
- Info.AccessibleScopes = {""};
- Info.UnresolvedQualifier = SpelledSpecifier;
+ Scopes.AccessibleScopes = {""};
+ Scopes.UnresolvedQualifier = SpelledSpecifier;
// Sema excludes the trailing "::".
- if (!Info.UnresolvedQualifier->empty())
- *Info.UnresolvedQualifier += "::";
+ if (!Scopes.UnresolvedQualifier->empty())
+ *Scopes.UnresolvedQualifier += "::";
- return {Info.scopesForIndexQuery(), false};
+ return {Scopes.scopesForIndexQuery(), false};
}
// Should we perform index-based completion in a context of the specified kind?
@@ -984,7 +990,7 @@
const tooling::CompileCommand &Command;
const PreambleData *Preamble;
llvm::StringRef Contents;
- Position Pos;
+ size_t Offset;
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS;
};
@@ -1015,14 +1021,9 @@
// Setup code completion.
FrontendOpts.CodeCompleteOpts = Options;
FrontendOpts.CodeCompletionAt.FileName = Input.FileName;
- auto Offset = positionToOffset(Input.Contents, Input.Pos);
- if (!Offset) {
- elog("Code completion position was invalid {0}", Offset.takeError());
- return false;
- }
std::tie(FrontendOpts.CodeCompletionAt.Line,
FrontendOpts.CodeCompletionAt.Column) =
- offsetToClangLineColumn(Input.Contents, *Offset);
+ offsetToClangLineColumn(Input.Contents, Input.Offset);
std::unique_ptr<llvm::MemoryBuffer> ContentsBuffer =
llvm::MemoryBuffer::getMemBufferCopy(Input.Contents, Input.FileName);
@@ -1036,7 +1037,7 @@
// skip all includes in this case; these completions are really simple.
bool CompletingInPreamble =
ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0).Size >
- *Offset;
+ Input.Offset;
// NOTE: we must call BeginSourceFile after prepareCompilerInstance. Otherwise
// the remapped buffers do not get freed.
IgnoreDiagnostics DummyDiagsConsumer;
@@ -1107,16 +1108,9 @@
// Creates a `FuzzyFindRequest` based on the cached index request from the
// last completion, if any, and the speculated completion filter text in the
// source code.
-llvm::Optional<FuzzyFindRequest>
-speculativeFuzzyFindRequestForCompletion(FuzzyFindRequest CachedReq,
- PathRef File, llvm::StringRef Content,
- Position Pos) {
- auto Offset = positionToOffset(Content, Pos);
- if (!Offset) {
- elog("No speculative filter: bad offset {0} in {1}", Pos, File);
- return llvm::None;
- }
- CachedReq.Query = guessCompletionPrefix(Content, *Offset).Name;
+FuzzyFindRequest speculativeFuzzyFindRequestForCompletion(
+ FuzzyFindRequest CachedReq, const CompletionPrefix &HeuristicPrefix) {
+ CachedReq.Query = HeuristicPrefix.Name;
return CachedReq;
}
@@ -1159,6 +1153,7 @@
CompletionRecorder *Recorder = nullptr;
int NSema = 0, NIndex = 0, NBoth = 0; // Counters for logging.
bool Incomplete = false; // Would more be available with a higher limit?
+ CompletionPrefix HeuristicPrefix;
llvm::Optional<FuzzyMatcher> Filter; // Initialized once Sema runs.
std::vector<std::string> QueryScopes; // Initialized once Sema runs.
// Initialized once QueryScopes is initialized, if there are scopes.
@@ -1186,12 +1181,13 @@
CodeCompleteResult run(const SemaCompleteInput &SemaCCInput) && {
trace::Span Tracer("CodeCompleteFlow");
+ HeuristicPrefix =
+ guessCompletionPrefix(SemaCCInput.Contents, SemaCCInput.Offset);
if (Opts.Index && SpecFuzzyFind && SpecFuzzyFind->CachedReq.hasValue()) {
assert(!SpecFuzzyFind->Result.valid());
- if ((SpecReq = speculativeFuzzyFindRequestForCompletion(
- *SpecFuzzyFind->CachedReq, SemaCCInput.FileName,
- SemaCCInput.Contents, SemaCCInput.Pos)))
- SpecFuzzyFind->Result = startAsyncFuzzyFind(*Opts.Index, *SpecReq);
+ SpecReq = speculativeFuzzyFindRequestForCompletion(
+ *SpecFuzzyFind->CachedReq, HeuristicPrefix);
+ SpecFuzzyFind->Result = startAsyncFuzzyFind(*Opts.Index, *SpecReq);
}
// We run Sema code completion first. It builds an AST and calculates:
@@ -1285,8 +1281,8 @@
}
Filter = FuzzyMatcher(
Recorder->CCSema->getPreprocessor().getCodeCompletionFilter());
- std::tie(QueryScopes, AllScopes) =
- getQueryScopes(Recorder->CCContext, *Recorder->CCSema, Opts);
+ std::tie(QueryScopes, AllScopes) = getQueryScopes(
+ Recorder->CCContext, *Recorder->CCSema, HeuristicPrefix, Opts);
if (!QueryScopes.empty())
ScopeProximity.emplace(QueryScopes);
PreferredType =
@@ -1563,10 +1559,15 @@
const PreambleData *Preamble, llvm::StringRef Contents,
Position Pos, llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
CodeCompleteOptions Opts, SpeculativeFuzzyFind *SpecFuzzyFind) {
+ auto Offset = positionToOffset(Contents, Pos);
+ if (!Offset) {
+ elog("Code completion position was invalid {0}", Offset.takeError());
+ return CodeCompleteResult();
+ }
return CodeCompleteFlow(FileName,
Preamble ? Preamble->Includes : IncludeStructure(),
SpecFuzzyFind, Opts)
- .run({FileName, Command, Preamble, Contents, Pos, VFS});
+ .run({FileName, Command, Preamble, Contents, *Offset, VFS});
}
SignatureHelp signatureHelp(PathRef FileName,
@@ -1575,6 +1576,11 @@
llvm::StringRef Contents, Position Pos,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
const SymbolIndex *Index) {
+ auto Offset = positionToOffset(Contents, Pos);
+ if (!Offset) {
+ elog("Code completion position was invalid {0}", Offset.takeError());
+ return SignatureHelp();
+ }
SignatureHelp Result;
clang::CodeCompleteOptions Options;
Options.IncludeGlobals = false;
@@ -1584,7 +1590,8 @@
IncludeStructure PreambleInclusions; // Unused for signatureHelp
semaCodeComplete(
llvm::make_unique<SignatureHelpCollector>(Options, Index, Result),
- Options, {FileName, Command, Preamble, Contents, Pos, std::move(VFS)});
+ Options,
+ {FileName, Command, Preamble, Contents, *Offset, std::move(VFS)});
return Result;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
