Skip to content

[clang] Make deprecations of some FileManager APIs formal #110014

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clang-tools-extra/clang-move/tool/ClangMove.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ int main(int argc, const char **argv) {
for (auto I = Files.begin(), E = Files.end(); I != E; ++I) {
OS << " {\n";
OS << " \"FilePath\": \"" << *I << "\",\n";
const auto Entry = FileMgr.getFile(*I);
const auto Entry = FileMgr.getOptionalFileRef(*I);
auto ID = SM.translateFile(*Entry);
std::string Content;
llvm::raw_string_ostream ContentStream(Content);
Expand Down
4 changes: 2 additions & 2 deletions clang-tools-extra/clangd/SourceCode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -814,8 +814,8 @@ llvm::SmallVector<llvm::StringRef> ancestorNamespaces(llvm::StringRef NS) {

// Checks whether \p FileName is a valid spelling of main file.
bool isMainFile(llvm::StringRef FileName, const SourceManager &SM) {
auto FE = SM.getFileManager().getFile(FileName);
return FE && *FE == SM.getFileEntryForID(SM.getMainFileID());
auto FE = SM.getFileManager().getOptionalFileRef(FileName);
return FE && FE == SM.getFileEntryRefForID(SM.getMainFileID());
}

} // namespace
Expand Down
4 changes: 2 additions & 2 deletions clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -397,10 +397,10 @@ TEST(ParsedASTTest, PatchesAdditionalIncludes) {
auto &FM = SM.getFileManager();
// Copy so that we can use operator[] to get the children.
IncludeStructure Includes = PatchedAST->getIncludeStructure();
auto MainFE = FM.getFile(testPath("foo.cpp"));
auto MainFE = FM.getOptionalFileRef(testPath("foo.cpp"));
ASSERT_TRUE(MainFE);
auto MainID = Includes.getID(*MainFE);
auto AuxFE = FM.getFile(testPath("sub/aux.h"));
auto AuxFE = FM.getOptionalFileRef(testPath("sub/aux.h"));
ASSERT_TRUE(AuxFE);
auto AuxID = Includes.getID(*AuxFE);
EXPECT_THAT(Includes.IncludeChildren[*MainID], Contains(*AuxID));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class FindHeadersTest : public testing::Test {
llvm::SmallVector<Hinted<Header>> findHeaders(llvm::StringRef FileName) {
return include_cleaner::findHeaders(
AST->sourceManager().translateFileLineCol(
AST->fileManager().getFile(FileName).get(),
*AST->fileManager().getOptionalFileRef(FileName),
/*Line=*/1, /*Col=*/1),
AST->sourceManager(), &PI);
}
Expand Down
84 changes: 42 additions & 42 deletions clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ TEST_F(RecordPPTest, CapturesMacroRefs) {
const auto &SM = AST.sourceManager();

SourceLocation Def = SM.getComposedLoc(
SM.translateFile(AST.fileManager().getFile("header.h").get()),
SM.translateFile(*AST.fileManager().getOptionalFileRef("header.h")),
Header.point("def"));
ASSERT_THAT(Recorded.MacroReferences, Not(IsEmpty()));
Symbol OrigX = Recorded.MacroReferences.front().Target;
Expand Down Expand Up @@ -368,29 +368,29 @@ TEST_F(PragmaIncludeTest, IWYUKeep) {
TestAST Processed = build();
auto &FM = Processed.fileManager();

EXPECT_FALSE(PI.shouldKeep(FM.getFile("normal.h").get()));
EXPECT_FALSE(PI.shouldKeep(FM.getFile("std/vector").get()));
EXPECT_FALSE(PI.shouldKeep(*FM.getOptionalFileRef("normal.h")));
EXPECT_FALSE(PI.shouldKeep(*FM.getOptionalFileRef("std/vector")));

// Keep
EXPECT_TRUE(PI.shouldKeep(FM.getFile("keep1.h").get()));
EXPECT_TRUE(PI.shouldKeep(FM.getFile("keep2.h").get()));
EXPECT_TRUE(PI.shouldKeep(FM.getFile("keep3.h").get()));
EXPECT_TRUE(PI.shouldKeep(FM.getFile("keep4.h").get()));
EXPECT_TRUE(PI.shouldKeep(FM.getFile("keep5.h").get()));
EXPECT_TRUE(PI.shouldKeep(FM.getFile("keep6.h").get()));
EXPECT_TRUE(PI.shouldKeep(FM.getFile("std/map").get()));
EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("keep1.h")));
EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("keep2.h")));
EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("keep3.h")));
EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("keep4.h")));
EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("keep5.h")));
EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("keep6.h")));
EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("std/map")));

// Exports
EXPECT_TRUE(PI.shouldKeep(FM.getFile("export1.h").get()));
EXPECT_TRUE(PI.shouldKeep(FM.getFile("export2.h").get()));
EXPECT_TRUE(PI.shouldKeep(FM.getFile("export3.h").get()));
EXPECT_TRUE(PI.shouldKeep(FM.getFile("std/set").get()));
EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("export1.h")));
EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("export2.h")));
EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("export3.h")));
EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("std/set")));
}

TEST_F(PragmaIncludeTest, AssociatedHeader) {
createEmptyFiles({"foo/main.h", "bar/main.h", "bar/other.h", "std/vector"});
auto IsKeep = [&](llvm::StringRef Name, TestAST &AST) {
return PI.shouldKeep(AST.fileManager().getFile(Name).get());
return PI.shouldKeep(*AST.fileManager().getOptionalFileRef(Name));
};

Inputs.FileName = "main.cc";
Expand Down Expand Up @@ -452,19 +452,19 @@ TEST_F(PragmaIncludeTest, IWYUPrivate) {
// IWYU pragma: private
)cpp";
TestAST Processed = build();
auto PrivateFE = Processed.fileManager().getFile("private.h");
auto PrivateFE = Processed.fileManager().getOptionalFileRef("private.h");
assert(PrivateFE);
EXPECT_TRUE(PI.isPrivate(PrivateFE.get()));
EXPECT_EQ(PI.getPublic(PrivateFE.get()), "\"public2.h\"");
EXPECT_TRUE(PI.isPrivate(*PrivateFE));
EXPECT_EQ(PI.getPublic(*PrivateFE), "\"public2.h\"");

auto PublicFE = Processed.fileManager().getFile("public.h");
auto PublicFE = Processed.fileManager().getOptionalFileRef("public.h");
assert(PublicFE);
EXPECT_EQ(PI.getPublic(PublicFE.get()), ""); // no mapping.
EXPECT_FALSE(PI.isPrivate(PublicFE.get()));
EXPECT_EQ(PI.getPublic(*PublicFE), ""); // no mapping.
EXPECT_FALSE(PI.isPrivate(*PublicFE));

auto Private2FE = Processed.fileManager().getFile("private2.h");
auto Private2FE = Processed.fileManager().getOptionalFileRef("private2.h");
assert(Private2FE);
EXPECT_TRUE(PI.isPrivate(Private2FE.get()));
EXPECT_TRUE(PI.isPrivate(*Private2FE));
}

TEST_F(PragmaIncludeTest, IWYUExport) {
Expand All @@ -486,13 +486,13 @@ TEST_F(PragmaIncludeTest, IWYUExport) {
const auto &SM = Processed.sourceManager();
auto &FM = Processed.fileManager();

EXPECT_THAT(PI.getExporters(FM.getFile("private.h").get(), FM),
EXPECT_THAT(PI.getExporters(*FM.getOptionalFileRef("private.h"), FM),
testing::UnorderedElementsAre(FileNamed("export1.h"),
FileNamed("export3.h")));

EXPECT_TRUE(PI.getExporters(FM.getFile("export1.h").get(), FM).empty());
EXPECT_TRUE(PI.getExporters(FM.getFile("export2.h").get(), FM).empty());
EXPECT_TRUE(PI.getExporters(FM.getFile("export3.h").get(), FM).empty());
EXPECT_TRUE(PI.getExporters(*FM.getOptionalFileRef("export1.h"), FM).empty());
EXPECT_TRUE(PI.getExporters(*FM.getOptionalFileRef("export2.h"), FM).empty());
EXPECT_TRUE(PI.getExporters(*FM.getOptionalFileRef("export3.h"), FM).empty());
EXPECT_TRUE(
PI.getExporters(SM.getFileEntryForID(SM.getMainFileID()), FM).empty());
}
Expand Down Expand Up @@ -548,23 +548,23 @@ TEST_F(PragmaIncludeTest, IWYUExportBlock) {
}
return Result;
};
auto Exporters = PI.getExporters(FM.getFile("private1.h").get(), FM);
auto Exporters = PI.getExporters(*FM.getOptionalFileRef("private1.h"), FM);
EXPECT_THAT(Exporters, testing::UnorderedElementsAre(FileNamed("export1.h"),
FileNamed("normal.h")))
<< GetNames(Exporters);

Exporters = PI.getExporters(FM.getFile("private2.h").get(), FM);
Exporters = PI.getExporters(*FM.getOptionalFileRef("private2.h"), FM);
EXPECT_THAT(Exporters, testing::UnorderedElementsAre(FileNamed("export1.h")))
<< GetNames(Exporters);

Exporters = PI.getExporters(FM.getFile("private3.h").get(), FM);
Exporters = PI.getExporters(*FM.getOptionalFileRef("private3.h"), FM);
EXPECT_THAT(Exporters, testing::UnorderedElementsAre(FileNamed("export1.h")))
<< GetNames(Exporters);

Exporters = PI.getExporters(FM.getFile("foo.h").get(), FM);
Exporters = PI.getExporters(*FM.getOptionalFileRef("foo.h"), FM);
EXPECT_TRUE(Exporters.empty()) << GetNames(Exporters);

Exporters = PI.getExporters(FM.getFile("bar.h").get(), FM);
Exporters = PI.getExporters(*FM.getOptionalFileRef("bar.h"), FM);
EXPECT_TRUE(Exporters.empty()) << GetNames(Exporters);
}

Expand All @@ -580,8 +580,8 @@ TEST_F(PragmaIncludeTest, SelfContained) {
Inputs.ExtraFiles["unguarded.h"] = "";
TestAST Processed = build();
auto &FM = Processed.fileManager();
EXPECT_TRUE(PI.isSelfContained(FM.getFile("guarded.h").get()));
EXPECT_FALSE(PI.isSelfContained(FM.getFile("unguarded.h").get()));
EXPECT_TRUE(PI.isSelfContained(*FM.getOptionalFileRef("guarded.h")));
EXPECT_FALSE(PI.isSelfContained(*FM.getOptionalFileRef("unguarded.h")));
}

TEST_F(PragmaIncludeTest, AlwaysKeep) {
Expand All @@ -596,8 +596,8 @@ TEST_F(PragmaIncludeTest, AlwaysKeep) {
Inputs.ExtraFiles["usual.h"] = "#pragma once";
TestAST Processed = build();
auto &FM = Processed.fileManager();
EXPECT_TRUE(PI.shouldKeep(FM.getFile("always_keep.h").get()));
EXPECT_FALSE(PI.shouldKeep(FM.getFile("usual.h").get()));
EXPECT_TRUE(PI.shouldKeep(*FM.getOptionalFileRef("always_keep.h")));
EXPECT_FALSE(PI.shouldKeep(*FM.getOptionalFileRef("usual.h")));
}

TEST_F(PragmaIncludeTest, ExportInUnnamedBuffer) {
Expand Down Expand Up @@ -653,13 +653,13 @@ TEST_F(PragmaIncludeTest, OutlivesFMAndSM) {
// Now this build gives us a new File&Source Manager.
TestAST Processed = build(/*ResetPragmaIncludes=*/false);
auto &FM = Processed.fileManager();
auto PrivateFE = FM.getFile("private.h");
auto PrivateFE = FM.getOptionalFileRef("private.h");
assert(PrivateFE);
EXPECT_EQ(PI.getPublic(PrivateFE.get()), "\"public.h\"");
EXPECT_EQ(PI.getPublic(*PrivateFE), "\"public.h\"");

auto Private2FE = FM.getFile("private2.h");
auto Private2FE = FM.getOptionalFileRef("private2.h");
assert(Private2FE);
EXPECT_THAT(PI.getExporters(Private2FE.get(), FM),
EXPECT_THAT(PI.getExporters(*Private2FE, FM),
testing::ElementsAre(llvm::cantFail(FM.getFileRef("public.h"))));
}

Expand All @@ -676,8 +676,8 @@ TEST_F(PragmaIncludeTest, CanRecordManyTimes) {

TestAST Processed = build();
auto &FM = Processed.fileManager();
auto PrivateFE = FM.getFile("private.h");
llvm::StringRef Public = PI.getPublic(PrivateFE.get());
auto PrivateFE = FM.getOptionalFileRef("private.h");
llvm::StringRef Public = PI.getPublic(*PrivateFE);
EXPECT_EQ(Public, "\"public.h\"");

// This build populates same PI during build, but this time we don't have
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class VirtualFileHelper {
I != E; ++I) {
std::unique_ptr<llvm::MemoryBuffer> Buf =
llvm::MemoryBuffer::getMemBuffer(I->Code);
const FileEntry *Entry = SM.getFileManager().getVirtualFile(
FileEntryRef Entry = SM.getFileManager().getVirtualFileRef(
I->FileName, Buf->getBufferSize(), /*ModificationTime=*/0);
SM.overrideFileContents(Entry, std::move(Buf));
}
Expand Down
2 changes: 0 additions & 2 deletions clang/include/clang/Basic/DiagnosticFrontendKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,6 @@ def err_fe_expected_clang_command : Error<
"expected a clang compiler command">;
def err_fe_remap_missing_to_file : Error<
"could not remap file '%0' to the contents of file '%1'">, DefaultFatal;
def err_fe_remap_missing_from_file : Error<
"could not remap from missing file '%0'">, DefaultFatal;
def err_fe_unable_to_load_pch : Error<
"unable to load PCH file">;
def err_fe_unable_to_load_plugin : Error<
Expand Down
8 changes: 7 additions & 1 deletion clang/include/clang/Basic/FileManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class FileManager : public RefCountedBase<FileManager> {
/// VirtualDirectoryEntries/VirtualFileEntries above.
///
llvm::StringMap<llvm::ErrorOr<DirectoryEntry &>, llvm::BumpPtrAllocator>
SeenDirEntries;
SeenDirEntries;

/// A cache that maps paths to file entries (either real or
/// virtual) we have looked up, or an error that occurred when we looked up
Expand Down Expand Up @@ -190,6 +190,8 @@ class FileManager : public RefCountedBase<FileManager> {
///
/// \param CacheFailure If true and the file does not exist, we'll cache
/// the failure to find this file.
LLVM_DEPRECATED("Functions returning DirectoryEntry are deprecated.",
"getOptionalDirectoryRef()")
llvm::ErrorOr<const DirectoryEntry *>
getDirectory(StringRef DirName, bool CacheFailure = true);

Expand All @@ -207,6 +209,8 @@ class FileManager : public RefCountedBase<FileManager> {
///
/// \param CacheFailure If true and the file does not exist, we'll cache
/// the failure to find this file.
LLVM_DEPRECATED("Functions returning FileEntry are deprecated.",
"getOptionalFileRef()")
llvm::ErrorOr<const FileEntry *>
getFile(StringRef Filename, bool OpenFile = false, bool CacheFailure = true);

Expand Down Expand Up @@ -269,6 +273,8 @@ class FileManager : public RefCountedBase<FileManager> {
FileEntryRef getVirtualFileRef(StringRef Filename, off_t Size,
time_t ModificationTime);

LLVM_DEPRECATED("Functions returning FileEntry are deprecated.",
"getVirtualFileRef()")
const FileEntry *getVirtualFile(StringRef Filename, off_t Size,
time_t ModificationTime);

Expand Down
4 changes: 2 additions & 2 deletions clang/lib/AST/ASTImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10020,8 +10020,8 @@ Expected<FileID> ASTImporter::Import(FileID FromID, bool IsBuiltin) {
ToIncludeLocOrFakeLoc = ToSM.getLocForStartOfFile(ToSM.getMainFileID());

if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
// FIXME: We probably want to use getVirtualFile(), so we don't hit the
// disk again
// FIXME: We probably want to use getVirtualFileRef(), so we don't hit
// the disk again
// FIXME: We definitely want to re-use the existing MemoryBuffer, rather
// than mmap the files several times.
auto Entry =
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/CodeGen/CodeGenAction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -586,9 +586,9 @@ const FullSourceLoc BackendConsumer::getBestLocationFromDebugLoc(
if (D.isLocationAvailable()) {
D.getLocation(Filename, Line, Column);
if (Line > 0) {
auto FE = FileMgr.getFile(Filename);
auto FE = FileMgr.getOptionalFileRef(Filename);
if (!FE)
FE = FileMgr.getFile(D.getAbsolutePath());
FE = FileMgr.getOptionalFileRef(D.getAbsolutePath());
if (FE) {
// If -gcolumn-info was not used, Column will be 0. This upsets the
// source manager, so pass 1 if Column is not set.
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,8 @@ struct LocationFileChecker {
SmallVector<std::pair<SmallString<32>, bool>> &KnownFiles)
: CI(CI), KnownFiles(KnownFiles), ExternalFileEntries() {
for (const auto &KnownFile : KnownFiles)
if (auto FileEntry = CI.getFileManager().getFile(KnownFile.first))
KnownFileEntries.insert(*FileEntry);
if (auto FE = CI.getFileManager().getOptionalFileRef(KnownFile.first))
KnownFileEntries.insert(*FE);
}

private:
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Frontend/ASTUnit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2395,7 +2395,7 @@ void ASTUnit::TranslateStoredDiagnostics(
// Rebuild the StoredDiagnostic.
if (SD.Filename.empty())
continue;
auto FE = FileMgr.getFile(SD.Filename);
auto FE = FileMgr.getOptionalFileRef(SD.Filename);
if (!FE)
continue;
SourceLocation FileLoc;
Expand Down
10 changes: 3 additions & 7 deletions clang/lib/Frontend/CompilerInstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -427,12 +427,8 @@ static void InitializeFileRemapping(DiagnosticsEngine &Diags,
}

// Create the file entry for the file that we're mapping from.
const FileEntry *FromFile =
FileMgr.getVirtualFile(RF.first, ToFile->getSize(), 0);
if (!FromFile) {
Diags.Report(diag::err_fe_remap_missing_from_file) << RF.first;
continue;
}
FileEntryRef FromFile =
FileMgr.getVirtualFileRef(RF.first, ToFile->getSize(), 0);

// Override the contents of the "from" file with the contents of
// the "to" file.
Expand Down Expand Up @@ -1926,7 +1922,7 @@ ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST(

// Check whether M refers to the file in the prebuilt module path.
if (M && M->getASTFile())
if (auto ModuleFile = FileMgr->getFile(ModuleFilename))
if (auto ModuleFile = FileMgr->getOptionalFileRef(ModuleFilename))
if (*ModuleFile == M->getASTFile())
return M;

Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Frontend/Rewrite/FrontendActions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ class RewriteIncludesAction::RewriteImportsListener : public ASTReaderListener {

void visitModuleFile(StringRef Filename,
serialization::ModuleKind Kind) override {
auto File = CI.getFileManager().getFile(Filename);
auto File = CI.getFileManager().getOptionalFileRef(Filename);
assert(File && "missing file for loaded module?");

// Only rewrite each module file once.
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/InstallAPI/Frontend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ InstallAPIContext::findAndRecordFile(const FileEntry *FE,
}

void InstallAPIContext::addKnownHeader(const HeaderFile &H) {
auto FE = FM->getFile(H.getPath());
auto FE = FM->getOptionalFileRef(H.getPath());
if (!FE)
return; // File does not exist.
KnownFiles[*FE] = H.getType();
Expand Down
8 changes: 4 additions & 4 deletions clang/lib/Lex/HeaderSearch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ std::string HeaderSearch::getPrebuiltModuleFileName(StringRef ModuleName,
".pcm");
else
llvm::sys::path::append(Result, ModuleName + ".pcm");
if (getFileMgr().getFile(Result.str()))
if (getFileMgr().getOptionalFileRef(Result))
return std::string(Result);
}

Expand All @@ -246,7 +246,7 @@ std::string HeaderSearch::getPrebuiltImplicitModuleFileName(Module *Module) {
llvm::sys::path::append(CachePath, ModuleCacheHash);
std::string FileName =
getCachedModuleFileNameImpl(ModuleName, ModuleMapPath, CachePath);
if (!FileName.empty() && getFileMgr().getFile(FileName))
if (!FileName.empty() && getFileMgr().getOptionalFileRef(FileName))
return FileName;
}
return {};
Expand Down Expand Up @@ -655,7 +655,7 @@ OptionalFileEntryRef DirectoryLookup::DoFrameworkLookup(
++NumFrameworkLookups;

// If the framework dir doesn't exist, we fail.
auto Dir = FileMgr.getDirectory(FrameworkName);
auto Dir = FileMgr.getOptionalDirectoryRef(FrameworkName);
if (!Dir)
return std::nullopt;

Expand Down Expand Up @@ -718,7 +718,7 @@ OptionalFileEntryRef DirectoryLookup::DoFrameworkLookup(
bool FoundFramework = false;
do {
// Determine whether this directory exists.
auto Dir = FileMgr.getDirectory(FrameworkPath);
auto Dir = FileMgr.getOptionalDirectoryRef(FrameworkPath);
if (!Dir)
break;

Expand Down
Loading
Loading