Skip to content

Commit

Permalink
[clangd] Use FileEntryRef for canonicalizing filepaths.
Browse files Browse the repository at this point in the history 
Using FileEntry for retrieving filenames give the name used for the last access. FileEntryRef gives the first access name.

Last access name is suspected to change with unrelated changes in clang or the underlying filesystem. First access name gives more stability to the name and makes it easier to track.

Differential Revision: https://reviews.llvm.org/D148213
  • Loading branch information
@usx95
usx95 committed Apr 13, 2023
1 parent 18b1a7a commit ed365f4
Show file tree
Hide file tree
Showing 9 changed files with 38 additions and 36 deletions.
4 changes: 2 additions & 2 deletions clang-tools-extra/clangd/Diagnostics.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -715,9 +715,9 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
D.InsideMainFile = isInsideMainFile(PatchLoc, SM);
D.Range = diagnosticRange(Info, *LangOpts);
auto FID = SM.getFileID(Info.getLocation());
if (auto *FE = SM.getFileEntryForID(FID)) {
if (const auto FE = SM.getFileEntryRefForID(FID)) {
D.File = FE->getName().str();
D.AbsFile = getCanonicalPath(FE, SM);
D.AbsFile = getCanonicalPath(*FE, SM);
}
D.ID = Info.getID();
return D;
Expand Down
4 changes: 2 additions & 2 deletions clang-tools-extra/clangd/IncludeCleaner.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,8 @@ convertIncludes(const SourceManager &SM,
std::string spellHeader(ParsedAST &AST, const FileEntry *MainFile,
include_cleaner::Header Provider) {
if (Provider.kind() == include_cleaner::Header::Physical) {
if (auto CanonicalPath =
getCanonicalPath(Provider.physical(), AST.getSourceManager())) {
if (auto CanonicalPath = getCanonicalPath(Provider.physical()->getLastRef(),
AST.getSourceManager())) {
std::string SpelledHeader =
llvm::cantFail(URI::includeSpelling(URI::create(*CanonicalPath)));
if (!SpelledHeader.empty())
Expand Down
8 changes: 3 additions & 5 deletions clang-tools-extra/clangd/SourceCode.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "Protocol.h"
#include "support/Context.h"
#include "support/Logger.h"
#include "clang/Basic/FileEntry.h"
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
Expand Down Expand Up @@ -512,12 +513,9 @@ std::vector<TextEdit> replacementsToEdits(llvm::StringRef Code,
return Edits;
}

std::optional<std::string> getCanonicalPath(const FileEntry *F,
std::optional<std::string> getCanonicalPath(const FileEntryRef F,
const SourceManager &SourceMgr) {
if (!F)
return std::nullopt;

llvm::SmallString<128> FilePath = F->getName();
llvm::SmallString<128> FilePath = F.getName();
if (!llvm::sys::path::is_absolute(FilePath)) {
if (auto EC =
SourceMgr.getFileManager().getVirtualFileSystem().makeAbsolute(
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/clangd/SourceCode.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
/// This function should be used when paths needs to be used outside the
/// component that generate it, so that paths are normalized as much as
/// possible.
std::optional<std::string> getCanonicalPath(const FileEntry *F,
std::optional<std::string> getCanonicalPath(const FileEntryRef F,
const SourceManager &SourceMgr);

/// Choose the clang-format style we should apply to a certain file.
Expand Down
13 changes: 8 additions & 5 deletions clang-tools-extra/clangd/XRefs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,10 +208,10 @@ getDeclAtPosition(ParsedAST &AST, SourceLocation Pos, DeclRelationSet Relations,
std::optional<Location> makeLocation(const ASTContext &AST, SourceLocation Loc,
llvm::StringRef TUPath) {
const auto &SM = AST.getSourceManager();
const FileEntry *F = SM.getFileEntryForID(SM.getFileID(Loc));
const auto F = SM.getFileEntryRefForID(SM.getFileID(Loc));
if (!F)
return std::nullopt;
auto FilePath = getCanonicalPath(F, SM);
auto FilePath = getCanonicalPath(*F, SM);
if (!FilePath) {
log("failed to get path!");
return std::nullopt;
Expand Down Expand Up @@ -1617,8 +1617,10 @@ declToHierarchyItem(const NamedDecl &ND, llvm::StringRef TUPath) {
toHalfOpenFileRange(SM, Ctx.getLangOpts(), {BeginLoc, EndLoc});
if (!DeclRange)
return std::nullopt;
auto FilePath =
getCanonicalPath(SM.getFileEntryForID(SM.getFileID(NameLoc)), SM);
const auto FE = SM.getFileEntryRefForID(SM.getFileID(NameLoc));
if (!FE)
return std::nullopt;
auto FilePath = getCanonicalPath(*FE, SM);
if (!FilePath)
return std::nullopt; // Not useful without a uri.

Expand Down Expand Up @@ -1968,7 +1970,8 @@ static void unwrapFindType(
return unwrapFindType(FT->getReturnType(), H, Out);
if (auto *CRD = T->getAsCXXRecordDecl()) {
if (CRD->isLambda())
return unwrapFindType(CRD->getLambdaCallOperator()->getReturnType(), H, Out);
return unwrapFindType(CRD->getLambdaCallOperator()->getReturnType(), H,
Out);
// FIXME: more cases we'd prefer the return type of the call operator?
// std::function etc?
}
Expand Down
4 changes: 2 additions & 2 deletions clang-tools-extra/clangd/index/Background.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,10 +288,10 @@ llvm::Error BackgroundIndex::index(tooling::CompileCommand Cmd) {
// digests.
IndexOpts.FileFilter = [&ShardVersionsSnapshot](const SourceManager &SM,
FileID FID) {
const auto *F = SM.getFileEntryForID(FID);
const auto F = SM.getFileEntryRefForID(FID);
if (!F)
return false; // Skip invalid files.
auto AbsPath = getCanonicalPath(F, SM);
auto AbsPath = getCanonicalPath(*F, SM);
if (!AbsPath)
return false; // Skip files without absolute path.
auto Digest = digestFile(SM, FID);
Expand Down
30 changes: 15 additions & 15 deletions clang-tools-extra/clangd/index/SymbolCollector.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,11 +205,11 @@ class SymbolCollector::HeaderFileURICache {

// Returns a canonical URI for the file \p FE.
// We attempt to make the path absolute first.
const std::string &toURI(const FileEntry *FE) {
const std::string &toURI(const FileEntryRef FE) {
auto R = CacheFEToURI.try_emplace(FE);
if (R.second) {
auto CanonPath = getCanonicalPath(FE, SM);
R.first->second = &toURIInternal(CanonPath ? *CanonPath : FE->getName());
R.first->second = &toURIInternal(CanonPath ? *CanonPath : FE.getName());
}
return *R.first->second;
}
Expand All @@ -218,7 +218,7 @@ class SymbolCollector::HeaderFileURICache {
// If the file is in the FileManager, use that to canonicalize the path.
// We attempt to make the path absolute in any case.
const std::string &toURI(llvm::StringRef Path) {
if (auto File = SM.getFileManager().getFile(Path))
if (auto File = SM.getFileManager().getFileRef(Path))
return toURI(*File);
return toURIInternal(Path);
}
Expand Down Expand Up @@ -373,7 +373,7 @@ class SymbolCollector::HeaderFileURICache {
}

llvm::StringRef getIncludeHeaderUncached(FileID FID) {
const FileEntry *FE = SM.getFileEntryForID(FID);
const auto FE = SM.getFileEntryRefForID(FID);
if (!FE || FE->getName().empty())
return "";
llvm::StringRef Filename = FE->getName();
Expand All @@ -392,13 +392,13 @@ class SymbolCollector::HeaderFileURICache {
// Framework headers are spelled as <FrameworkName/Foo.h>, not
// "path/FrameworkName.framework/Headers/Foo.h".
auto &HS = PP->getHeaderSearchInfo();
if (const auto *HFI = HS.getExistingFileInfo(FE, /*WantExternal*/ false))
if (const auto *HFI = HS.getExistingFileInfo(*FE, /*WantExternal*/ false))
if (!HFI->Framework.empty())
if (auto Spelling =
getFrameworkHeaderIncludeSpelling(FE, HFI->Framework, HS))
getFrameworkHeaderIncludeSpelling(*FE, HFI->Framework, HS))
return *Spelling;

if (!tooling::isSelfContainedHeader(FE, PP->getSourceManager(),
if (!tooling::isSelfContainedHeader(*FE, PP->getSourceManager(),
PP->getHeaderSearchInfo())) {
// A .inc or .def file is often included into a real header to define
// symbols (e.g. LLVM tablegen files).
Expand All @@ -409,20 +409,20 @@ class SymbolCollector::HeaderFileURICache {
return "";
}
// Standard case: just insert the file itself.
return toURI(FE);
return toURI(*FE);
}
};

// Return the symbol location of the token at \p TokLoc.
std::optional<SymbolLocation>
SymbolCollector::getTokenLocation(SourceLocation TokLoc) {
const auto &SM = ASTCtx->getSourceManager();
auto *FE = SM.getFileEntryForID(SM.getFileID(TokLoc));
const auto FE = SM.getFileEntryRefForID(SM.getFileID(TokLoc));
if (!FE)
return std::nullopt;

SymbolLocation Result;
Result.FileURI = HeaderFileURIs->toURI(FE).c_str();
Result.FileURI = HeaderFileURIs->toURI(*FE).c_str();
auto Range = getTokenRange(TokLoc, SM, ASTCtx->getLangOpts());
Result.Start = Range.first;
Result.End = Range.second;
Expand Down Expand Up @@ -635,10 +635,10 @@ bool SymbolCollector::handleDeclOccurrence(
void SymbolCollector::handleMacros(const MainFileMacros &MacroRefsToIndex) {
assert(HeaderFileURIs && PP);
const auto &SM = PP->getSourceManager();
const auto *MainFileEntry = SM.getFileEntryForID(SM.getMainFileID());
assert(MainFileEntry);
const auto MainFileEntryRef = SM.getFileEntryRefForID(SM.getMainFileID());
assert(MainFileEntryRef);

const std::string &MainFileURI = HeaderFileURIs->toURI(MainFileEntry);
const std::string &MainFileURI = HeaderFileURIs->toURI(*MainFileEntryRef);
// Add macro references.
for (const auto &IDToRefs : MacroRefsToIndex.MacroRefs) {
for (const auto &MacroRef : IDToRefs.second) {
Expand Down Expand Up @@ -987,12 +987,12 @@ void SymbolCollector::addRef(SymbolID ID, const SymbolRef &SR) {
const auto &SM = ASTCtx->getSourceManager();
// FIXME: use the result to filter out references.
shouldIndexFile(SR.FID);
if (const auto *FE = SM.getFileEntryForID(SR.FID)) {
if (const auto FE = SM.getFileEntryRefForID(SR.FID)) {
auto Range = getTokenRange(SR.Loc, SM, ASTCtx->getLangOpts());
Ref R;
R.Location.Start = Range.first;
R.Location.End = Range.second;
R.Location.FileURI = HeaderFileURIs->toURI(FE).c_str();
R.Location.FileURI = HeaderFileURIs->toURI(*FE).c_str();
R.Kind = toRefKind(SR.Roles, SR.Spelled);
R.Container = getSymbolIDCached(SR.Container);
Refs.insert(ID, R);
Expand Down
4 changes: 2 additions & 2 deletions clang-tools-extra/clangd/indexer/IndexerMain.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ class IndexActionFactory : public tooling::FrontendActionFactory {
SymbolCollector::Options Opts;
Opts.CountReferences = true;
Opts.FileFilter = [&](const SourceManager &SM, FileID FID) {
const auto *F = SM.getFileEntryForID(FID);
const auto F = SM.getFileEntryRefForID(FID);
if (!F)
return false; // Skip invalid files.
auto AbsPath = getCanonicalPath(F, SM);
auto AbsPath = getCanonicalPath(*F, SM);
if (!AbsPath)
return false; // Skip files without absolute path.
std::lock_guard<std::mutex> Lock(FilesMu);
Expand Down
5 changes: 3 additions & 2 deletions clang-tools-extra/clangd/refactor/Tweak.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,9 @@ llvm::Expected<std::pair<Path, Edit>>
Tweak::Effect::fileEdit(const SourceManager &SM, FileID FID,
tooling::Replacements Replacements) {
Edit Ed(SM.getBufferData(FID), std::move(Replacements));
if (auto FilePath = getCanonicalPath(SM.getFileEntryForID(FID), SM))
return std::make_pair(*FilePath, std::move(Ed));
if (const auto FE = SM.getFileEntryRefForID(FID))
if (auto FilePath = getCanonicalPath(*FE, SM))
return std::make_pair(*FilePath, std::move(Ed));
return error("Failed to get absolute path for edited file: {0}",
SM.getFileEntryRefForID(FID)->getName());
}
Expand Down

0 comments on commit ed365f4

Please sign in to comment.