[clang] af3fb07 - [Frontend] Simplify PrecompiledPreamble::PCHStorage. NFC
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 21 09:10:35 PDT 2022
Author: Sam McCall
Date: 2022-04-21T18:10:13+02:00
New Revision: af3fb071545918f2de61142fa12d8782e5a37fa5
URL: https://github.com/llvm/llvm-project/commit/af3fb071545918f2de61142fa12d8782e5a37fa5
DIFF: https://github.com/llvm/llvm-project/commit/af3fb071545918f2de61142fa12d8782e5a37fa5.diff
LOG: [Frontend] Simplify PrecompiledPreamble::PCHStorage. NFC
- Remove fiddly union, preambles are heavyweight
- Remove fiddly move constructors in TempPCHFile and PCHStorage, use unique_ptr
- Remove unneccesary accessors on PCHStorage
- Remove trivial InMemoryStorage
- Move implementation details into cpp file
This is a prefactoring, followup change will change the in-memory PCHStorage to
avoid extra string copies while creating it.
Differential Revision: https://reviews.llvm.org/D124177
Added:
Modified:
clang/include/clang/Frontend/PrecompiledPreamble.h
clang/lib/Frontend/PrecompiledPreamble.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Frontend/PrecompiledPreamble.h b/clang/include/clang/Frontend/PrecompiledPreamble.h
index 628736f34091b..db9f33ae5961f 100644
--- a/clang/include/clang/Frontend/PrecompiledPreamble.h
+++ b/clang/include/clang/Frontend/PrecompiledPreamble.h
@@ -17,7 +17,6 @@
#include "clang/Lex/Preprocessor.h"
#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/ADT/StringRef.h"
-#include "llvm/Support/AlignOf.h"
#include "llvm/Support/MD5.h"
#include <cstddef>
#include <memory>
@@ -86,8 +85,9 @@ class PrecompiledPreamble {
std::shared_ptr<PCHContainerOperations> PCHContainerOps,
bool StoreInMemory, PreambleCallbacks &Callbacks);
- PrecompiledPreamble(PrecompiledPreamble &&) = default;
- PrecompiledPreamble &operator=(PrecompiledPreamble &&) = default;
+ PrecompiledPreamble(PrecompiledPreamble &&);
+ PrecompiledPreamble &operator=(PrecompiledPreamble &&);
+ ~PrecompiledPreamble();
/// PreambleBounds used to build the preamble.
PreambleBounds getBounds() const;
@@ -128,79 +128,12 @@ class PrecompiledPreamble {
llvm::MemoryBuffer *MainFileBuffer) const;
private:
- PrecompiledPreamble(PCHStorage Storage, std::vector<char> PreambleBytes,
+ PrecompiledPreamble(std::unique_ptr<PCHStorage> Storage,
+ std::vector<char> PreambleBytes,
bool PreambleEndsAtStartOfLine,
llvm::StringMap<PreambleFileHash> FilesInPreamble,
llvm::StringSet<> MissingFiles);
- /// A temp file that would be deleted on destructor call. If destructor is not
- /// called for any reason, the file will be deleted at static objects'
- /// destruction.
- /// An assertion will fire if two TempPCHFiles are created with the same name,
- /// so it's not intended to be used outside preamble-handling.
- class TempPCHFile {
- public:
- // A main method used to construct TempPCHFile.
- static llvm::ErrorOr<TempPCHFile> CreateNewPreamblePCHFile();
-
- private:
- TempPCHFile(std::string FilePath);
-
- public:
- TempPCHFile(TempPCHFile &&Other);
- TempPCHFile &operator=(TempPCHFile &&Other);
-
- TempPCHFile(const TempPCHFile &) = delete;
- ~TempPCHFile();
-
- /// A path where temporary file is stored.
- llvm::StringRef getFilePath() const;
-
- private:
- void RemoveFileIfPresent();
-
- private:
- llvm::Optional<std::string> FilePath;
- };
-
- class InMemoryPreamble {
- public:
- std::string Data;
- };
-
- class PCHStorage {
- public:
- enum class Kind { Empty, InMemory, TempFile };
-
- PCHStorage() = default;
- PCHStorage(TempPCHFile File);
- PCHStorage(InMemoryPreamble Memory);
-
- PCHStorage(const PCHStorage &) = delete;
- PCHStorage &operator=(const PCHStorage &) = delete;
-
- PCHStorage(PCHStorage &&Other);
- PCHStorage &operator=(PCHStorage &&Other);
-
- ~PCHStorage();
-
- Kind getKind() const;
-
- TempPCHFile &asFile();
- const TempPCHFile &asFile() const;
-
- InMemoryPreamble &asMemory();
- const InMemoryPreamble &asMemory() const;
-
- private:
- void destroy();
- void setEmpty();
-
- private:
- Kind StorageKind = Kind::Empty;
- llvm::AlignedCharArrayUnion<TempPCHFile, InMemoryPreamble> Storage = {};
- };
-
/// Data used to determine if a file used in the preamble has been changed.
struct PreambleFileHash {
/// All files have size set.
@@ -245,7 +178,7 @@ class PrecompiledPreamble {
IntrusiveRefCntPtr<llvm::vfs::FileSystem> &VFS);
/// Manages the memory buffer or temporary file that stores the PCH.
- PCHStorage Storage;
+ std::unique_ptr<PCHStorage> Storage;
/// Keeps track of the files that were used when computing the
/// preamble, with both their buffer size and their modification time.
///
diff --git a/clang/lib/Frontend/PrecompiledPreamble.cpp b/clang/lib/Frontend/PrecompiledPreamble.cpp
index f29cb038c2080..9f5be05a14308 100644
--- a/clang/lib/Frontend/PrecompiledPreamble.cpp
+++ b/clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -11,10 +11,8 @@
//===----------------------------------------------------------------------===//
#include "clang/Frontend/PrecompiledPreamble.h"
-#include "clang/AST/DeclObjC.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/LangStandard.h"
-#include "clang/Basic/TargetInfo.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/FrontendActions.h"
@@ -25,7 +23,6 @@
#include "clang/Lex/PreprocessorOptions.h"
#include "clang/Serialization/ASTWriter.h"
#include "llvm/ADT/SmallString.h"
-#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringSet.h"
#include "llvm/ADT/iterator_range.h"
#include "llvm/Config/llvm-config.h"
@@ -192,6 +189,49 @@ void TemporaryFiles::removeFile(StringRef File) {
llvm::sys::fs::remove(File);
}
+// A temp file that would be deleted on destructor call. If destructor is not
+// called for any reason, the file will be deleted at static objects'
+// destruction.
+// An assertion will fire if two TempPCHFiles are created with the same name,
+// so it's not intended to be used outside preamble-handling.
+class TempPCHFile {
+public:
+ // A main method used to construct TempPCHFile.
+ static std::unique_ptr<TempPCHFile> create() {
+ // FIXME: This is a hack so that we can override the preamble file during
+ // crash-recovery testing, which is the only case where the preamble files
+ // are not necessarily cleaned up.
+ if (const char *TmpFile = ::getenv("CINDEXTEST_PREAMBLE_FILE"))
+ return std::unique_ptr<TempPCHFile>(new TempPCHFile(TmpFile));
+
+ llvm::SmallString<64> File;
+ // Using a version of createTemporaryFile with a file descriptor guarantees
+ // that we would never get a race condition in a multi-threaded setting
+ // (i.e., multiple threads getting the same temporary path).
+ int FD;
+ if (auto EC =
+ llvm::sys::fs::createTemporaryFile("preamble", "pch", FD, File))
+ return nullptr;
+ // We only needed to make sure the file exists, close the file right away.
+ llvm::sys::Process::SafelyCloseFileDescriptor(FD);
+ return std::unique_ptr<TempPCHFile>(new TempPCHFile(File.str().str()));
+ }
+
+ TempPCHFile &operator=(const TempPCHFile &) = delete;
+ TempPCHFile(const TempPCHFile &) = delete;
+ ~TempPCHFile() { TemporaryFiles::getInstance().removeFile(FilePath); };
+
+ /// A path where temporary file is stored.
+ llvm::StringRef getFilePath() const { return FilePath; };
+
+private:
+ TempPCHFile(std::string FilePath) : FilePath(std::move(FilePath)) {
+ TemporaryFiles::getInstance().addFile(this->FilePath);
+ }
+
+ std::string FilePath;
+};
+
class PrecompilePreambleAction : public ASTFrontendAction {
public:
PrecompilePreambleAction(std::string *InMemStorage,
@@ -308,6 +348,55 @@ PreambleBounds clang::ComputePreambleBounds(const LangOptions &LangOpts,
return Lexer::ComputePreamble(Buffer.getBuffer(), LangOpts, MaxLines);
}
+class PrecompiledPreamble::PCHStorage {
+public:
+ static std::unique_ptr<PCHStorage> file(std::unique_ptr<TempPCHFile> File) {
+ assert(File);
+ std::unique_ptr<PCHStorage> S(new PCHStorage());
+ S->File = std::move(File);
+ return S;
+ }
+ static std::unique_ptr<PCHStorage> inMemory() {
+ std::unique_ptr<PCHStorage> S(new PCHStorage());
+ S->Memory.emplace();
+ return S;
+ }
+
+ enum class Kind { InMemory, TempFile };
+ Kind getKind() const {
+ if (Memory)
+ return Kind::InMemory;
+ if (File)
+ return Kind::TempFile;
+ llvm_unreachable("Neither Memory nor File?");
+ }
+ llvm::StringRef filePath() const {
+ assert(getKind() == Kind::TempFile);
+ return File->getFilePath();
+ }
+ llvm::StringRef memoryContents() const {
+ assert(getKind() == Kind::InMemory);
+ return *Memory;
+ }
+ std::string &memoryBufferForWrite() {
+ assert(getKind() == Kind::InMemory);
+ return *Memory;
+ }
+
+private:
+ PCHStorage() = default;
+ PCHStorage(const PCHStorage &) = delete;
+ PCHStorage &operator=(const PCHStorage &) = delete;
+
+ llvm::Optional<std::string> Memory;
+ std::unique_ptr<TempPCHFile> File;
+};
+
+PrecompiledPreamble::~PrecompiledPreamble() = default;
+PrecompiledPreamble::PrecompiledPreamble(PrecompiledPreamble &&) = default;
+PrecompiledPreamble &
+PrecompiledPreamble::operator=(PrecompiledPreamble &&) = default;
+
llvm::ErrorOr<PrecompiledPreamble> PrecompiledPreamble::Build(
const CompilerInvocation &Invocation,
const llvm::MemoryBuffer *MainFileBuffer, PreambleBounds Bounds,
@@ -322,20 +411,18 @@ llvm::ErrorOr<PrecompiledPreamble> PrecompiledPreamble::Build(
PreprocessorOptions &PreprocessorOpts =
PreambleInvocation->getPreprocessorOpts();
- llvm::Optional<TempPCHFile> TempFile;
- if (!StoreInMemory) {
+ std::unique_ptr<PCHStorage> Storage;
+ if (StoreInMemory) {
+ Storage = PCHStorage::inMemory();
+ } else {
// Create a temporary file for the precompiled preamble. In rare
// circumstances, this can fail.
- llvm::ErrorOr<PrecompiledPreamble::TempPCHFile> PreamblePCHFile =
- PrecompiledPreamble::TempPCHFile::CreateNewPreamblePCHFile();
+ std::unique_ptr<TempPCHFile> PreamblePCHFile = TempPCHFile::create();
if (!PreamblePCHFile)
return BuildPreambleError::CouldntCreateTempFile;
- TempFile = std::move(*PreamblePCHFile);
+ Storage = PCHStorage::file(std::move(PreamblePCHFile));
}
- PCHStorage Storage = StoreInMemory ? PCHStorage(InMemoryPreamble())
- : PCHStorage(std::move(*TempFile));
-
// Save the preamble text for later; we'll need to compare against it for
// subsequent reparses.
std::vector<char> PreambleBytes(MainFileBuffer->getBufferStart(),
@@ -345,9 +432,8 @@ llvm::ErrorOr<PrecompiledPreamble> PrecompiledPreamble::Build(
// Tell the compiler invocation to generate a temporary precompiled header.
FrontendOpts.ProgramAction = frontend::GeneratePCH;
- FrontendOpts.OutputFile =
- std::string(StoreInMemory ? getInMemoryPreamblePath()
- : Storage.asFile().getFilePath());
+ FrontendOpts.OutputFile = std::string(
+ StoreInMemory ? getInMemoryPreamblePath() : Storage->filePath());
PreprocessorOpts.PrecompiledPreambleBytes.first = 0;
PreprocessorOpts.PrecompiledPreambleBytes.second = false;
// Inform preprocessor to record conditional stack when building the preamble.
@@ -411,7 +497,7 @@ llvm::ErrorOr<PrecompiledPreamble> PrecompiledPreamble::Build(
std::unique_ptr<PrecompilePreambleAction> Act;
Act.reset(new PrecompilePreambleAction(
- StoreInMemory ? &Storage.asMemory().Data : nullptr, Callbacks));
+ StoreInMemory ? &Storage->memoryBufferForWrite() : nullptr, Callbacks));
if (!Act->BeginSourceFile(*Clang.get(), Clang->getFrontendOpts().Inputs[0]))
return BuildPreambleError::BeginSourceFileFailed;
@@ -475,16 +561,12 @@ PreambleBounds PrecompiledPreamble::getBounds() const {
}
std::size_t PrecompiledPreamble::getSize() const {
- switch (Storage.getKind()) {
- case PCHStorage::Kind::Empty:
- assert(false && "Calling getSize() on invalid PrecompiledPreamble. "
- "Was it std::moved?");
- return 0;
+ switch (Storage->getKind()) {
case PCHStorage::Kind::InMemory:
- return Storage.asMemory().Data.size();
+ return Storage->memoryContents().size();
case PCHStorage::Kind::TempFile: {
uint64_t Result;
- if (llvm::sys::fs::file_size(Storage.asFile().getFilePath(), Result))
+ if (llvm::sys::fs::file_size(Storage->filePath(), Result))
return 0;
assert(Result <= std::numeric_limits<std::size_t>::max() &&
@@ -621,7 +703,7 @@ void PrecompiledPreamble::OverridePreamble(
}
PrecompiledPreamble::PrecompiledPreamble(
- PCHStorage Storage, std::vector<char> PreambleBytes,
+ std::unique_ptr<PCHStorage> Storage, std::vector<char> PreambleBytes,
bool PreambleEndsAtStartOfLine,
llvm::StringMap<PreambleFileHash> FilesInPreamble,
llvm::StringSet<> MissingFiles)
@@ -629,142 +711,7 @@ PrecompiledPreamble::PrecompiledPreamble(
MissingFiles(std::move(MissingFiles)),
PreambleBytes(std::move(PreambleBytes)),
PreambleEndsAtStartOfLine(PreambleEndsAtStartOfLine) {
- assert(this->Storage.getKind() != PCHStorage::Kind::Empty);
-}
-
-llvm::ErrorOr<PrecompiledPreamble::TempPCHFile>
-PrecompiledPreamble::TempPCHFile::CreateNewPreamblePCHFile() {
- // FIXME: This is a hack so that we can override the preamble file during
- // crash-recovery testing, which is the only case where the preamble files
- // are not necessarily cleaned up.
- if (const char *TmpFile = ::getenv("CINDEXTEST_PREAMBLE_FILE"))
- return TempPCHFile(TmpFile);
-
- llvm::SmallString<64> File;
- // Using a version of createTemporaryFile with a file descriptor guarantees
- // that we would never get a race condition in a multi-threaded setting
- // (i.e., multiple threads getting the same temporary path).
- int FD;
- auto EC = llvm::sys::fs::createTemporaryFile("preamble", "pch", FD, File);
- if (EC)
- return EC;
- // We only needed to make sure the file exists, close the file right away.
- llvm::sys::Process::SafelyCloseFileDescriptor(FD);
- return TempPCHFile(std::string(std::move(File).str()));
-}
-
-PrecompiledPreamble::TempPCHFile::TempPCHFile(std::string FilePath)
- : FilePath(std::move(FilePath)) {
- TemporaryFiles::getInstance().addFile(*this->FilePath);
-}
-
-PrecompiledPreamble::TempPCHFile::TempPCHFile(TempPCHFile &&Other) {
- FilePath = std::move(Other.FilePath);
- Other.FilePath = None;
-}
-
-PrecompiledPreamble::TempPCHFile &PrecompiledPreamble::TempPCHFile::
-operator=(TempPCHFile &&Other) {
- RemoveFileIfPresent();
-
- FilePath = std::move(Other.FilePath);
- Other.FilePath = None;
- return *this;
-}
-
-PrecompiledPreamble::TempPCHFile::~TempPCHFile() { RemoveFileIfPresent(); }
-
-void PrecompiledPreamble::TempPCHFile::RemoveFileIfPresent() {
- if (FilePath) {
- TemporaryFiles::getInstance().removeFile(*FilePath);
- FilePath = None;
- }
-}
-
-llvm::StringRef PrecompiledPreamble::TempPCHFile::getFilePath() const {
- assert(FilePath && "TempPCHFile doesn't have a FilePath. Had it been moved?");
- return *FilePath;
-}
-
-PrecompiledPreamble::PCHStorage::PCHStorage(TempPCHFile File)
- : StorageKind(Kind::TempFile) {
- new (&asFile()) TempPCHFile(std::move(File));
-}
-
-PrecompiledPreamble::PCHStorage::PCHStorage(InMemoryPreamble Memory)
- : StorageKind(Kind::InMemory) {
- new (&asMemory()) InMemoryPreamble(std::move(Memory));
-}
-
-PrecompiledPreamble::PCHStorage::PCHStorage(PCHStorage &&Other) : PCHStorage() {
- *this = std::move(Other);
-}
-
-PrecompiledPreamble::PCHStorage &PrecompiledPreamble::PCHStorage::
-operator=(PCHStorage &&Other) {
- destroy();
-
- StorageKind = Other.StorageKind;
- switch (StorageKind) {
- case Kind::Empty:
- // do nothing;
- break;
- case Kind::TempFile:
- new (&asFile()) TempPCHFile(std::move(Other.asFile()));
- break;
- case Kind::InMemory:
- new (&asMemory()) InMemoryPreamble(std::move(Other.asMemory()));
- break;
- }
-
- Other.setEmpty();
- return *this;
-}
-
-PrecompiledPreamble::PCHStorage::~PCHStorage() { destroy(); }
-
-PrecompiledPreamble::PCHStorage::Kind
-PrecompiledPreamble::PCHStorage::getKind() const {
- return StorageKind;
-}
-
-PrecompiledPreamble::TempPCHFile &PrecompiledPreamble::PCHStorage::asFile() {
- assert(getKind() == Kind::TempFile);
- return *reinterpret_cast<TempPCHFile *>(&Storage);
-}
-
-const PrecompiledPreamble::TempPCHFile &
-PrecompiledPreamble::PCHStorage::asFile() const {
- return const_cast<PCHStorage *>(this)->asFile();
-}
-
-PrecompiledPreamble::InMemoryPreamble &
-PrecompiledPreamble::PCHStorage::asMemory() {
- assert(getKind() == Kind::InMemory);
- return *reinterpret_cast<InMemoryPreamble *>(&Storage);
-}
-
-const PrecompiledPreamble::InMemoryPreamble &
-PrecompiledPreamble::PCHStorage::asMemory() const {
- return const_cast<PCHStorage *>(this)->asMemory();
-}
-
-void PrecompiledPreamble::PCHStorage::destroy() {
- switch (StorageKind) {
- case Kind::Empty:
- return;
- case Kind::TempFile:
- asFile().~TempPCHFile();
- return;
- case Kind::InMemory:
- asMemory().~InMemoryPreamble();
- return;
- }
-}
-
-void PrecompiledPreamble::PCHStorage::setEmpty() {
- destroy();
- StorageKind = Kind::Empty;
+ assert(this->Storage != nullptr);
}
PrecompiledPreamble::PreambleFileHash
@@ -810,20 +757,19 @@ void PrecompiledPreamble::configurePreamble(
PreprocessorOpts.DisablePCHOrModuleValidation =
DisableValidationForModuleKind::PCH;
- setupPreambleStorage(Storage, PreprocessorOpts, VFS);
+ setupPreambleStorage(*Storage, PreprocessorOpts, VFS);
}
void PrecompiledPreamble::setupPreambleStorage(
const PCHStorage &Storage, PreprocessorOptions &PreprocessorOpts,
IntrusiveRefCntPtr<llvm::vfs::FileSystem> &VFS) {
if (Storage.getKind() == PCHStorage::Kind::TempFile) {
- const TempPCHFile &PCHFile = Storage.asFile();
- PreprocessorOpts.ImplicitPCHInclude = std::string(PCHFile.getFilePath());
+ llvm::StringRef PCHPath = Storage.filePath();
+ PreprocessorOpts.ImplicitPCHInclude = PCHPath.str();
// Make sure we can access the PCH file even if we're using a VFS
IntrusiveRefCntPtr<llvm::vfs::FileSystem> RealFS =
llvm::vfs::getRealFileSystem();
- auto PCHPath = PCHFile.getFilePath();
if (VFS == RealFS || VFS->exists(PCHPath))
return;
auto Buf = RealFS->getBufferForFile(PCHPath);
@@ -844,7 +790,7 @@ void PrecompiledPreamble::setupPreambleStorage(
StringRef PCHPath = getInMemoryPreamblePath();
PreprocessorOpts.ImplicitPCHInclude = std::string(PCHPath);
- auto Buf = llvm::MemoryBuffer::getMemBuffer(Storage.asMemory().Data);
+ auto Buf = llvm::MemoryBuffer::getMemBuffer(Storage.memoryContents());
VFS = createVFSOverlayForPreamblePCH(PCHPath, std::move(Buf), VFS);
}
}
More information about the cfe-commits
mailing list