Skip to content

Commit

Permalink
[clangd] Incorporate transitive #includes into code complete proximit…
Browse files Browse the repository at this point in the history 
…y scoring.

Summary:
We now compute a distance from the main file to the symbol header, which
is a weighted count of:
 - some number of #include traversals from source file --> included file
 - some number of FS traversals from file --> parent directory
 - some number of FS traversals from parent directory --> child file/dir
This calculation is performed in the appropriate URI scheme.

This means we'll get some proximity boost from header files in main-file
contexts, even when these are in different directory trees.

This extended file proximity model is not yet incorporated in the index
interface/implementation.

Reviewers: ioeric

Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, cfe-commits

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

llvm-svn: 336177
  • Loading branch information
@sam-mccall
sam-mccall committed Jul 3, 2018
1 parent a0a52bf commit 3f0243f
Show file tree
Hide file tree
Showing 17 changed files with 653 additions and 259 deletions.
1 change: 1 addition & 0 deletions clang-tools-extra/clangd/CMakeLists.txt
View file
Expand Up @@ -19,6 +19,7 @@ add_clang_library(clangDaemon
Diagnostics.cpp
DraftStore.cpp
FindSymbols.cpp
FileDistance.cpp
FuzzyMatch.cpp
GlobalCompilationDatabase.cpp
Headers.cpp
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/clangd/ClangdServer.cpp
View file
Expand Up @@ -167,7 +167,7 @@ void ClangdServer::codeComplete(PathRef File, Position Pos,
// both the old and the new version in case only one of them matches.
CodeCompleteResult Result = clangd::codeComplete(
File, IP->Command, PreambleData ? &PreambleData->Preamble : nullptr,
PreambleData ? PreambleData->Inclusions : std::vector<Inclusion>(),
PreambleData ? PreambleData->Includes : IncludeStructure(),
IP->Contents, Pos, FS, PCHs, CodeCompleteOpts);
CB(std::move(Result));
};
Expand Down
35 changes: 14 additions & 21 deletions clang-tools-extra/clangd/ClangdUnit.cpp
View file
Expand Up @@ -88,7 +88,7 @@ class CppFilePreambleCallbacks : public PreambleCallbacks {
CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback)
: File(File), ParsedCallback(ParsedCallback) {}

std::vector<Inclusion> takeInclusions() { return std::move(Inclusions); }
IncludeStructure takeIncludes() { return std::move(Includes); }

void AfterExecute(CompilerInstance &CI) override {
if (!ParsedCallback)
Expand All @@ -103,15 +103,13 @@ class CppFilePreambleCallbacks : public PreambleCallbacks {

std::unique_ptr<PPCallbacks> createPPCallbacks() override {
assert(SourceMgr && "SourceMgr must be set at this point");
return collectInclusionsInMainFileCallback(
*SourceMgr,
[this](Inclusion Inc) { Inclusions.push_back(std::move(Inc)); });
return collectIncludeStructureCallback(*SourceMgr, &Includes);
}

private:
PathRef File;
PreambleParsedCallback ParsedCallback;
std::vector<Inclusion> Inclusions;
IncludeStructure Includes;
SourceManager *SourceMgr = nullptr;
};

Expand Down Expand Up @@ -153,15 +151,11 @@ ParsedAST::Build(std::unique_ptr<clang::CompilerInvocation> CI,
return llvm::None;
}

std::vector<Inclusion> Inclusions;
// Copy over the includes from the preamble, then combine with the
// non-preamble includes below.
if (Preamble)
Inclusions = Preamble->Inclusions;

Clang->getPreprocessor().addPPCallbacks(collectInclusionsInMainFileCallback(
Clang->getSourceManager(),
[&Inclusions](Inclusion Inc) { Inclusions.push_back(std::move(Inc)); }));
auto Includes = Preamble ? Preamble->Includes : IncludeStructure{};
Clang->getPreprocessor().addPPCallbacks(
collectIncludeStructureCallback(Clang->getSourceManager(), &Includes));

if (!Action->Execute())
log("Execute() failed when building AST for " + MainInput.getFile());
Expand All @@ -179,7 +173,7 @@ ParsedAST::Build(std::unique_ptr<clang::CompilerInvocation> CI,
Diags.insert(Diags.begin(), Preamble->Diags.begin(), Preamble->Diags.end());
return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action),
std::move(ParsedDecls), std::move(Diags),
std::move(Inclusions));
std::move(Includes));
}

ParsedAST::ParsedAST(ParsedAST &&Other) = default;
Expand Down Expand Up @@ -246,25 +240,24 @@ std::size_t ParsedAST::getUsedBytes() const {
return Total;
}

const std::vector<Inclusion> &ParsedAST::getInclusions() const {
return Inclusions;
const IncludeStructure &ParsedAST::getIncludeStructure() const {
return Includes;
}

PreambleData::PreambleData(PrecompiledPreamble Preamble,
std::vector<Diag> Diags,
std::vector<Inclusion> Inclusions)
std::vector<Diag> Diags, IncludeStructure Includes)
: Preamble(std::move(Preamble)), Diags(std::move(Diags)),
Inclusions(std::move(Inclusions)) {}
Includes(std::move(Includes)) {}

ParsedAST::ParsedAST(std::shared_ptr<const PreambleData> Preamble,
std::unique_ptr<CompilerInstance> Clang,
std::unique_ptr<FrontendAction> Action,
std::vector<Decl *> LocalTopLevelDecls,
std::vector<Diag> Diags, std::vector<Inclusion> Inclusions)
std::vector<Diag> Diags, IncludeStructure Includes)
: Preamble(std::move(Preamble)), Clang(std::move(Clang)),
Action(std::move(Action)), Diags(std::move(Diags)),
LocalTopLevelDecls(std::move(LocalTopLevelDecls)),
Inclusions(std::move(Inclusions)) {
Includes(std::move(Includes)) {
assert(this->Clang);
assert(this->Action);
}
Expand Down Expand Up @@ -350,7 +343,7 @@ std::shared_ptr<const PreambleData> clangd::buildPreamble(
" for file " + Twine(FileName));
return std::make_shared<PreambleData>(
std::move(*BuiltPreamble), PreambleDiagnostics.take(),
SerializedDeclsCollector.takeInclusions());
SerializedDeclsCollector.takeIncludes());
} else {
log("Could not build a preamble for file " + Twine(FileName));
return nullptr;
Expand Down
10 changes: 5 additions & 5 deletions clang-tools-extra/clangd/ClangdUnit.h
View file
Expand Up @@ -45,14 +45,14 @@ namespace clangd {
// Stores Preamble and associated data.
struct PreambleData {
PreambleData(PrecompiledPreamble Preamble, std::vector<Diag> Diags,
std::vector<Inclusion> Inclusions);
IncludeStructure Includes);

tooling::CompileCommand CompileCommand;
PrecompiledPreamble Preamble;
std::vector<Diag> Diags;
// Processes like code completions and go-to-definitions will need #include
// information, and their compile action skips preamble range.
std::vector<Inclusion> Inclusions;
IncludeStructure Includes;
};

/// Information required to run clang, e.g. to parse AST or do code completion.
Expand Down Expand Up @@ -99,14 +99,14 @@ class ParsedAST {
/// Returns the esitmated size of the AST and the accessory structures, in
/// bytes. Does not include the size of the preamble.
std::size_t getUsedBytes() const;
const std::vector<Inclusion> &getInclusions() const;
const IncludeStructure &getIncludeStructure() const;

private:
ParsedAST(std::shared_ptr<const PreambleData> Preamble,
std::unique_ptr<CompilerInstance> Clang,
std::unique_ptr<FrontendAction> Action,
std::vector<Decl *> LocalTopLevelDecls, std::vector<Diag> Diags,
std::vector<Inclusion> Inclusions);
IncludeStructure Includes);

// In-memory preambles must outlive the AST, it is important that this member
// goes before Clang and Action.
Expand All @@ -124,7 +124,7 @@ class ParsedAST {
// Top-level decls inside the current file. Not that this does not include
// top-level decls from the preamble.
std::vector<Decl *> LocalTopLevelDecls;
std::vector<Inclusion> Inclusions;
IncludeStructure Includes;
};

using PreambleParsedCallback = std::function<void(
Expand Down
120 changes: 67 additions & 53 deletions clang-tools-extra/clangd/CodeComplete.cpp
View file
Expand Up @@ -22,6 +22,7 @@
#include "AST.h"
#include "CodeCompletionStrings.h"
#include "Compiler.h"
#include "FileDistance.h"
#include "FuzzyMatch.h"
#include "Headers.h"
#include "Logger.h"
Expand Down Expand Up @@ -763,20 +764,18 @@ struct SemaCompleteInput {
PathRef FileName;
const tooling::CompileCommand &Command;
PrecompiledPreamble const *Preamble;
const std::vector<Inclusion> &PreambleInclusions;
StringRef Contents;
Position Pos;
IntrusiveRefCntPtr<vfs::FileSystem> VFS;
std::shared_ptr<PCHContainerOperations> PCHs;
};

// Invokes Sema code completion on a file.
// If \p Includes is set, it will be initialized after a compiler instance has
// been set up.
// If \p Includes is set, it will be updated based on the compiler invocation.
bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
const clang::CodeCompleteOptions &Options,
const SemaCompleteInput &Input,
std::unique_ptr<IncludeInserter> *Includes = nullptr) {
IncludeStructure *Includes = nullptr) {
trace::Span Tracer("Sema completion");
std::vector<const char *> ArgStrs;
for (const auto &S : Input.Command.CommandLine)
Expand Down Expand Up @@ -837,29 +836,9 @@ bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
Input.FileName);
return false;
}
if (Includes) {
// Initialize Includes if provided.

// FIXME(ioeric): needs more consistent style support in clangd server.
auto Style = format::getStyle(format::DefaultFormatStyle, Input.FileName,
format::DefaultFallbackStyle, Input.Contents,
Input.VFS.get());
if (!Style) {
log("ERROR: failed to get FormatStyle for file " + Input.FileName +
". Fall back to use LLVM style. Error: " +
llvm::toString(Style.takeError()));
Style = format::getLLVMStyle();
}
*Includes = llvm::make_unique<IncludeInserter>(
Input.FileName, Input.Contents, *Style, Input.Command.Directory,
Clang->getPreprocessor().getHeaderSearchInfo());
for (const auto &Inc : Input.PreambleInclusions)
Includes->get()->addExisting(Inc);
Clang->getPreprocessor().addPPCallbacks(collectInclusionsInMainFileCallback(
Clang->getSourceManager(), [Includes](Inclusion Inc) {
Includes->get()->addExisting(std::move(Inc));
}));
}
if (Includes)
Clang->getPreprocessor().addPPCallbacks(
collectIncludeStructureCallback(Clang->getSourceManager(), Includes));
if (!Action.Execute()) {
log("Execute() failed when running codeComplete for " + Input.FileName);
return false;
Expand Down Expand Up @@ -949,24 +928,23 @@ clang::CodeCompleteOptions CodeCompleteOptions::getClangCompleteOpts() const {
// - TopN determines the results with the best score.
class CodeCompleteFlow {
PathRef FileName;
IncludeStructure Includes; // Complete once the compiler runs.
const CodeCompleteOptions &Opts;
// Sema takes ownership of Recorder. Recorder is valid until Sema cleanup.
CompletionRecorder *Recorder = nullptr;
int NSema = 0, NIndex = 0, NBoth = 0; // Counters for logging.
bool Incomplete = false; // Would more be available with a higher limit?
llvm::Optional<FuzzyMatcher> Filter; // Initialized once Sema runs.
std::unique_ptr<IncludeInserter> Includes; // Initialized once compiler runs.
FileProximityMatcher FileProximityMatch;
// Include-insertion and proximity scoring rely on the include structure.
// This is available after Sema has run.
llvm::Optional<IncludeInserter> Inserter; // Available during runWithSema.
llvm::Optional<URIDistance> FileProximity; // Initialized once Sema runs.

public:
// A CodeCompleteFlow object is only useful for calling run() exactly once.
CodeCompleteFlow(PathRef FileName, const CodeCompleteOptions &Opts)
: FileName(FileName), Opts(Opts),
// FIXME: also use path of the main header corresponding to FileName to
// calculate the file proximity, which would capture include/ and src/
// project setup where headers and implementations are not in the same
// directory.
FileProximityMatch(ArrayRef<StringRef>({FileName})) {}
CodeCompleteFlow(PathRef FileName, const IncludeStructure &Includes,
const CodeCompleteOptions &Opts)
: FileName(FileName), Includes(Includes), Opts(Opts) {}

CodeCompleteResult run(const SemaCompleteInput &SemaCCInput) && {
trace::Span Tracer("CodeCompleteFlow");
Expand All @@ -977,11 +955,45 @@ class CodeCompleteFlow {
CodeCompleteResult Output;
auto RecorderOwner = llvm::make_unique<CompletionRecorder>(Opts, [&]() {
assert(Recorder && "Recorder is not set");
assert(Includes && "Includes is not set");
// FIXME(ioeric): needs more consistent style support in clangd server.
auto Style =
format::getStyle("file", SemaCCInput.FileName, "LLVM",
SemaCCInput.Contents, SemaCCInput.VFS.get());
if (!Style) {
log("Failed to get FormatStyle for file" + SemaCCInput.FileName + ": " +
llvm::toString(Style.takeError()) + ". Fallback is LLVM style.");
Style = format::getLLVMStyle();
}
// If preprocessor was run, inclusions from preprocessor callback should
// already be added to Inclusions.
// already be added to Includes.
Inserter.emplace(
SemaCCInput.FileName, SemaCCInput.Contents, *Style,
SemaCCInput.Command.Directory,
Recorder->CCSema->getPreprocessor().getHeaderSearchInfo());
for (const auto &Inc : Includes.MainFileIncludes)
Inserter->addExisting(Inc);

// Most of the cost of file proximity is in initializing the FileDistance
// structures based on the observed includes, once per query. Conceptually
// that happens here (though the per-URI-scheme initialization is lazy).
// The per-result proximity scoring is (amortized) very cheap.
FileDistanceOptions ProxOpts{}; // Use defaults.
const auto &SM = Recorder->CCSema->getSourceManager();
llvm::StringMap<SourceParams> ProxSources;
for (auto &Entry : Includes.includeDepth(
SM.getFileEntryForID(SM.getMainFileID())->getName())) {
auto &Source = ProxSources[Entry.getKey()];
Source.Cost = Entry.getValue() * ProxOpts.IncludeCost;
// Symbols near our transitive includes are good, but only consider
// things in the same directory or below it. Otherwise there can be
// many false positives.
if (Entry.getValue() > 0)
Source.MaxUpTraversals = 1;
}
FileProximity.emplace(ProxSources, ProxOpts);

Output = runWithSema();
Includes.reset(); // Make sure this doesn't out-live Clang.
Inserter.reset(); // Make sure this doesn't out-live Clang.
SPAN_ATTACH(Tracer, "sema_completion_kind",
getCompletionKindString(Recorder->CCContext.getKind()));
});
Expand Down Expand Up @@ -1044,6 +1056,7 @@ class CodeCompleteFlow {
Req.RestrictForCodeCompletion = true;
Req.Scopes = getQueryScopes(Recorder->CCContext,
Recorder->CCSema->getSourceManager());
// FIXME: we should send multiple weighted paths here.
Req.ProximityPaths.push_back(FileName);
log(llvm::formatv("Code complete: fuzzyFind(\"{0}\", scopes=[{1}])",
Req.Query,
Expand Down Expand Up @@ -1124,7 +1137,7 @@ class CodeCompleteFlow {
SymbolQualitySignals Quality;
SymbolRelevanceSignals Relevance;
Relevance.Query = SymbolRelevanceSignals::CodeComplete;
Relevance.FileProximityMatch = &FileProximityMatch;
Relevance.FileProximityMatch = FileProximity.getPointer();
auto &First = Bundle.front();
if (auto FuzzyScore = fuzzyScore(First))
Relevance.NameMatch = *FuzzyScore;
Expand Down Expand Up @@ -1174,23 +1187,24 @@ class CodeCompleteFlow {
: nullptr;
if (!Builder)
Builder.emplace(Recorder->CCSema->getASTContext(), Item, SemaCCS,
*Includes, FileName, Opts);
*Inserter, FileName, Opts);
else
Builder->add(Item, SemaCCS);
}
return Builder->build();
}
};

CodeCompleteResult codeComplete(
PathRef FileName, const tooling::CompileCommand &Command,
PrecompiledPreamble const *Preamble,
const std::vector<Inclusion> &PreambleInclusions, StringRef Contents,
Position Pos, IntrusiveRefCntPtr<vfs::FileSystem> VFS,
std::shared_ptr<PCHContainerOperations> PCHs, CodeCompleteOptions Opts) {
return CodeCompleteFlow(FileName, Opts)
.run({FileName, Command, Preamble, PreambleInclusions, Contents, Pos, VFS,
PCHs});
CodeCompleteResult codeComplete(PathRef FileName,
const tooling::CompileCommand &Command,
PrecompiledPreamble const *Preamble,
const IncludeStructure &PreambleInclusions,
StringRef Contents, Position Pos,
IntrusiveRefCntPtr<vfs::FileSystem> VFS,
std::shared_ptr<PCHContainerOperations> PCHs,
CodeCompleteOptions Opts) {
return CodeCompleteFlow(FileName, PreambleInclusions, Opts)
.run({FileName, Command, Preamble, Contents, Pos, VFS, PCHs});
}

SignatureHelp signatureHelp(PathRef FileName,
Expand All @@ -1205,11 +1219,11 @@ SignatureHelp signatureHelp(PathRef FileName,
Options.IncludeMacros = false;
Options.IncludeCodePatterns = false;
Options.IncludeBriefComments = false;
std::vector<Inclusion> PreambleInclusions = {}; // Unused for signatureHelp
IncludeStructure PreambleInclusions; // Unused for signatureHelp
semaCodeComplete(llvm::make_unique<SignatureHelpCollector>(Options, Result),
Options,
{FileName, Command, Preamble, PreambleInclusions, Contents,
Pos, std::move(VFS), std::move(PCHs)});
{FileName, Command, Preamble, Contents, Pos, std::move(VFS),
std::move(PCHs)});
return Result;
}

Expand Down
14 changes: 8 additions & 6 deletions clang-tools-extra/clangd/CodeComplete.h
View file
Expand Up @@ -144,12 +144,14 @@ struct CodeCompleteResult {
raw_ostream &operator<<(raw_ostream &, const CodeCompleteResult &);

/// Get code completions at a specified \p Pos in \p FileName.
CodeCompleteResult codeComplete(
PathRef FileName, const tooling::CompileCommand &Command,
PrecompiledPreamble const *Preamble,
const std::vector<Inclusion> &PreambleInclusions, StringRef Contents,
Position Pos, IntrusiveRefCntPtr<vfs::FileSystem> VFS,
std::shared_ptr<PCHContainerOperations> PCHs, CodeCompleteOptions Opts);
CodeCompleteResult codeComplete(PathRef FileName,
const tooling::CompileCommand &Command,
PrecompiledPreamble const *Preamble,
const IncludeStructure &PreambleInclusions,
StringRef Contents, Position Pos,
IntrusiveRefCntPtr<vfs::FileSystem> VFS,
std::shared_ptr<PCHContainerOperations> PCHs,
CodeCompleteOptions Opts);

/// Get signature help at a specified \p Pos in \p FileName.
SignatureHelp signatureHelp(PathRef FileName,
Expand Down

0 comments on commit 3f0243f

Please sign in to comment.