Skip to content

Commit

Permalink
Module: Use FileEntryRef and DirectoryEntryRef in Umbrella, Header, a…
Browse files Browse the repository at this point in the history
…nd DirectoryName, NFC

Push `FileEntryRef` and `DirectoryEntryRef` further, using it them
`Module::Umbrella`, `Module::Header::Entry`, and
`Module::DirectoryName::Entry`.

- Add `DirectoryEntryRef::operator const DirectoryEntry *` and
  `OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr`, to get the
  same "degrades to `DirectoryEntry*` behaviour `FileEntryRef` enjoys
  (this avoids a bunch of churn in various clang tools).
- Fix the `DirectoryEntryRef` constructor from `MapEntry` to take it by
  `const&`.

Note that we cannot get rid of the `...AsWritten` names leveraging the
new classes, since these need to be as written in the `ModuleMap` file
and the module directory path is preprended for the lookup in the
`FileManager`.

Differential Revision: https://reviews.llvm.org/D90497
  • Loading branch information
dexonsmith committed Dec 2, 2020
1 parent 327af6a commit 32c501d
Show file tree
Hide file tree
Showing 7 changed files with 128 additions and 30 deletions.
93 changes: 92 additions & 1 deletion clang/include/clang/Basic/DirectoryEntry.h
Expand Up @@ -51,7 +51,26 @@ class DirectoryEntryRef {
const MapEntry &getMapEntry() const { return *ME; }

DirectoryEntryRef() = delete;
DirectoryEntryRef(MapEntry &ME) : ME(&ME) {}
DirectoryEntryRef(const MapEntry &ME) : ME(&ME) {}

/// Allow DirectoryEntryRef to degrade into 'const DirectoryEntry*' to
/// facilitate incremental adoption.
///
/// The goal is to avoid code churn due to dances like the following:
/// \code
/// // Old code.
/// lvalue = rvalue;
///
/// // Temporary code from an incremental patch.
/// lvalue = &rvalue.getDirectoryEntry();
///
/// // Final code.
/// lvalue = rvalue;
/// \endcode
///
/// FIXME: Once DirectoryEntryRef is "everywhere" and DirectoryEntry::getName
/// has been deleted, delete this implicit conversion.
operator const DirectoryEntry *() const { return &getDirEntry(); }

private:
friend class FileMgr::MapEntryOptionalStorage<DirectoryEntryRef>;
Expand Down Expand Up @@ -147,4 +166,76 @@ static_assert(
} // end namespace optional_detail
} // end namespace llvm

namespace clang {

/// Wrapper around Optional<DirectoryEntryRef> that degrades to 'const
/// DirectoryEntry*', facilitating incremental patches to propagate
/// DirectoryEntryRef.
///
/// This class can be used as return value or field where it's convenient for
/// an Optional<DirectoryEntryRef> to degrade to a 'const DirectoryEntry*'. The
/// purpose is to avoid code churn due to dances like the following:
/// \code
/// // Old code.
/// lvalue = rvalue;
///
/// // Temporary code from an incremental patch.
/// Optional<DirectoryEntryRef> MaybeF = rvalue;
/// lvalue = MaybeF ? &MaybeF.getDirectoryEntry() : nullptr;
///
/// // Final code.
/// lvalue = rvalue;
/// \endcode
///
/// FIXME: Once DirectoryEntryRef is "everywhere" and DirectoryEntry::LastRef
/// and DirectoryEntry::getName have been deleted, delete this class and
/// replace instances with Optional<DirectoryEntryRef>.
class OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr
: public Optional<DirectoryEntryRef> {
public:
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr() = default;
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr(
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &&) = default;
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr(
const OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &) = default;
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &
operator=(OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &&) = default;
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &
operator=(const OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &) = default;

OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr(llvm::NoneType) {}
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr(DirectoryEntryRef Ref)
: Optional<DirectoryEntryRef>(Ref) {}
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr(Optional<DirectoryEntryRef> MaybeRef)
: Optional<DirectoryEntryRef>(MaybeRef) {}

OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &operator=(llvm::NoneType) {
Optional<DirectoryEntryRef>::operator=(None);
return *this;
}
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &operator=(DirectoryEntryRef Ref) {
Optional<DirectoryEntryRef>::operator=(Ref);
return *this;
}
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &
operator=(Optional<DirectoryEntryRef> MaybeRef) {
Optional<DirectoryEntryRef>::operator=(MaybeRef);
return *this;
}

/// Degrade to 'const DirectoryEntry *' to allow DirectoryEntry::LastRef and
/// DirectoryEntry::getName have been deleted, delete this class and replace
/// instances with Optional<DirectoryEntryRef>
operator const DirectoryEntry *() const {
return hasValue() ? &getValue().getDirEntry() : nullptr;
}
};

static_assert(std::is_trivially_copyable<
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr>::value,
"OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr should be "
"trivially copyable");

} // end namespace clang

#endif // LLVM_CLANG_BASIC_DIRECTORYENTRY_H
18 changes: 10 additions & 8 deletions clang/include/clang/Basic/Module.h
Expand Up @@ -133,7 +133,9 @@ class Module {
std::string PresumedModuleMapFile;

/// The umbrella header or directory.
llvm::PointerUnion<const FileEntry *, const DirectoryEntry *> Umbrella;
llvm::PointerUnion<const FileEntryRef::MapEntry *,
const DirectoryEntryRef::MapEntry *>
Umbrella;

/// The module signature.
ASTFileSignature Signature;
Expand Down Expand Up @@ -188,18 +190,18 @@ class Module {
/// file.
struct Header {
std::string NameAsWritten;
const FileEntry *Entry;
OptionalFileEntryRefDegradesToFileEntryPtr Entry;

explicit operator bool() { return Entry; }
explicit operator bool() { return Entry != None; }
};

/// Information about a directory name as found in the module map
/// file.
struct DirectoryName {
std::string NameAsWritten;
const DirectoryEntry *Entry;
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr Entry;

explicit operator bool() { return Entry; }
explicit operator bool() { return Entry != None; }
};

/// The headers that are part of this module.
Expand Down Expand Up @@ -544,15 +546,15 @@ class Module {
/// Retrieve the header that serves as the umbrella header for this
/// module.
Header getUmbrellaHeader() const {
if (auto *FE = Umbrella.dyn_cast<const FileEntry *>())
return Header{UmbrellaAsWritten, FE};
if (auto *ME = Umbrella.dyn_cast<const FileEntryRef::MapEntry *>())
return Header{UmbrellaAsWritten, FileEntryRef(*ME)};
return Header{};
}

/// Determine whether this module has an umbrella directory that is
/// not based on an umbrella header.
bool hasUmbrellaDir() const {
return Umbrella && Umbrella.is<const DirectoryEntry *>();
return Umbrella && Umbrella.is<const DirectoryEntryRef::MapEntry *>();
}

/// Add a top-level header associated with this module.
Expand Down
6 changes: 3 additions & 3 deletions clang/include/clang/Lex/ModuleMap.h
Expand Up @@ -14,6 +14,7 @@
#ifndef LLVM_CLANG_LEX_MODULEMAP_H
#define LLVM_CLANG_LEX_MODULEMAP_H

#include "clang/Basic/FileEntry.h"
#include "clang/Basic/IdentifierTable.h"
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/Module.h"
Expand All @@ -37,7 +38,6 @@ namespace clang {

class DiagnosticsEngine;
class DirectoryEntry;
class FileEntry;
class FileManager;
class HeaderSearch;
class SourceManager;
Expand Down Expand Up @@ -648,12 +648,12 @@ class ModuleMap {

/// Sets the umbrella header of the given module to the given
/// header.
void setUmbrellaHeader(Module *Mod, const FileEntry *UmbrellaHeader,
void setUmbrellaHeader(Module *Mod, FileEntryRef UmbrellaHeader,
Twine NameAsWritten);

/// Sets the umbrella directory of the given module to the given
/// directory.
void setUmbrellaDir(Module *Mod, const DirectoryEntry *UmbrellaDir,
void setUmbrellaDir(Module *Mod, DirectoryEntryRef UmbrellaDir,
Twine NameAsWritten);

/// Adds this header to the given module.
Expand Down
5 changes: 4 additions & 1 deletion clang/lib/Basic/Module.cpp
Expand Up @@ -247,7 +247,10 @@ Module::DirectoryName Module::getUmbrellaDir() const {
if (Header U = getUmbrellaHeader())
return {"", U.Entry->getDir()};

return {UmbrellaAsWritten, Umbrella.dyn_cast<const DirectoryEntry *>()};
if (auto *ME = Umbrella.dyn_cast<const DirectoryEntryRef::MapEntry *>())
return {UmbrellaAsWritten, DirectoryEntryRef(*ME)};

return {"", None};
}

void Module::addTopHeader(const FileEntry *File) {
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Frontend/FrontendActions.cpp
Expand Up @@ -304,7 +304,7 @@ bool GenerateHeaderModuleAction::BeginSourceFileAction(
<< Name;
continue;
}
Headers.push_back({std::string(Name), &FE->getFileEntry()});
Headers.push_back({std::string(Name), *FE});
}
HS.getModuleMap().createHeaderModule(CI.getLangOpts().CurrentModule, Headers);

Expand Down
26 changes: 13 additions & 13 deletions clang/lib/Lex/ModuleMap.cpp
Expand Up @@ -300,7 +300,7 @@ bool ModuleMap::resolveAsBuiltinHeader(
// supplied by Clang. Find that builtin header.
SmallString<128> Path;
llvm::sys::path::append(Path, BuiltinIncludeDir->getName(), Header.FileName);
auto File = SourceMgr.getFileManager().getFile(Path);
auto File = SourceMgr.getFileManager().getOptionalFileRef(Path);
if (!File)
return false;

Expand Down Expand Up @@ -1012,7 +1012,7 @@ Module *ModuleMap::inferFrameworkModule(const DirectoryEntry *FrameworkDir,
// Look for an umbrella header.
SmallString<128> UmbrellaName = StringRef(FrameworkDir->getName());
llvm::sys::path::append(UmbrellaName, "Headers", ModuleName + ".h");
auto UmbrellaHeader = FileMgr.getFile(UmbrellaName);
auto UmbrellaHeader = FileMgr.getOptionalFileRef(UmbrellaName);

// FIXME: If there's no umbrella header, we could probably scan the
// framework to load *everything*. But, it's not clear that this is a good
Expand Down Expand Up @@ -1121,21 +1121,21 @@ Module *ModuleMap::createShadowedModule(StringRef Name, bool IsFramework,
return Result;
}

void ModuleMap::setUmbrellaHeader(Module *Mod, const FileEntry *UmbrellaHeader,
void ModuleMap::setUmbrellaHeader(Module *Mod, FileEntryRef UmbrellaHeader,
Twine NameAsWritten) {
Headers[UmbrellaHeader].push_back(KnownHeader(Mod, NormalHeader));
Mod->Umbrella = UmbrellaHeader;
Mod->Umbrella = &UmbrellaHeader.getMapEntry();
Mod->UmbrellaAsWritten = NameAsWritten.str();
UmbrellaDirs[UmbrellaHeader->getDir()] = Mod;
UmbrellaDirs[UmbrellaHeader.getDir()] = Mod;

// Notify callbacks that we just added a new header.
for (const auto &Cb : Callbacks)
Cb->moduleMapAddUmbrellaHeader(&SourceMgr.getFileManager(), UmbrellaHeader);
}

void ModuleMap::setUmbrellaDir(Module *Mod, const DirectoryEntry *UmbrellaDir,
void ModuleMap::setUmbrellaDir(Module *Mod, DirectoryEntryRef UmbrellaDir,
Twine NameAsWritten) {
Mod->Umbrella = UmbrellaDir;
Mod->Umbrella = &UmbrellaDir.getMapEntry();
Mod->UmbrellaAsWritten = NameAsWritten.str();
UmbrellaDirs[UmbrellaDir] = Mod;
}
Expand Down Expand Up @@ -2416,15 +2416,15 @@ void ModuleMapParser::parseUmbrellaDirDecl(SourceLocation UmbrellaLoc) {
}

// Look for this file.
const DirectoryEntry *Dir = nullptr;
Optional<DirectoryEntryRef> Dir;
if (llvm::sys::path::is_absolute(DirName)) {
if (auto D = SourceMgr.getFileManager().getDirectory(DirName))
if (auto D = SourceMgr.getFileManager().getOptionalDirectoryRef(DirName))
Dir = *D;
} else {
SmallString<128> PathName;
PathName = Directory->getName();
llvm::sys::path::append(PathName, DirName);
if (auto D = SourceMgr.getFileManager().getDirectory(PathName))
if (auto D = SourceMgr.getFileManager().getOptionalDirectoryRef(PathName))
Dir = *D;
}

Expand All @@ -2445,7 +2445,7 @@ void ModuleMapParser::parseUmbrellaDirDecl(SourceLocation UmbrellaLoc) {
SourceMgr.getFileManager().getVirtualFileSystem();
for (llvm::vfs::recursive_directory_iterator I(FS, Dir->getName(), EC), E;
I != E && !EC; I.increment(EC)) {
if (auto FE = SourceMgr.getFileManager().getFile(I->path())) {
if (auto FE = SourceMgr.getFileManager().getOptionalFileRef(I->path())) {
Module::Header Header = {std::string(I->path()), *FE};
Headers.push_back(std::move(Header));
}
Expand All @@ -2459,15 +2459,15 @@ void ModuleMapParser::parseUmbrellaDirDecl(SourceLocation UmbrellaLoc) {
return;
}

if (Module *OwningModule = Map.UmbrellaDirs[Dir]) {
if (Module *OwningModule = Map.UmbrellaDirs[*Dir]) {
Diags.Report(UmbrellaLoc, diag::err_mmap_umbrella_clash)
<< OwningModule->getFullModuleName();
HadError = true;
return;
}

// Record this umbrella directory.
Map.setUmbrellaDir(ActiveModule, Dir, DirName);
Map.setUmbrellaDir(ActiveModule, *Dir, DirName);
}

/// Parse a module export declaration.
Expand Down
8 changes: 5 additions & 3 deletions clang/lib/Serialization/ASTReader.cpp
Expand Up @@ -1915,7 +1915,8 @@ HeaderFileInfoTrait::ReadData(internal_key_ref key, const unsigned char *d,
// FIXME: This is not always the right filename-as-written, but we're not
// going to use this information to rebuild the module, so it doesn't make
// a lot of difference.
Module::Header H = {std::string(key.Filename), *FileMgr.getFile(Filename)};
Module::Header H = {std::string(key.Filename),
*FileMgr.getOptionalFileRef(Filename)};
ModMap.addHeader(Mod, H, HeaderRole, /*Imported*/true);
HFI.isModuleHeader |= !(HeaderRole & ModuleMap::TextualHeader);
}
Expand Down Expand Up @@ -5564,7 +5565,7 @@ ASTReader::ReadSubmoduleBlock(ModuleFile &F, unsigned ClientLoadCapabilities) {
case SUBMODULE_UMBRELLA_HEADER: {
std::string Filename = std::string(Blob);
ResolveImportedPath(F, Filename);
if (auto Umbrella = PP.getFileManager().getFile(Filename)) {
if (auto Umbrella = PP.getFileManager().getOptionalFileRef(Filename)) {
if (!CurrentModule->getUmbrellaHeader())
ModMap.setUmbrellaHeader(CurrentModule, *Umbrella, Blob);
else if (CurrentModule->getUmbrellaHeader().Entry != *Umbrella) {
Expand Down Expand Up @@ -5597,7 +5598,8 @@ ASTReader::ReadSubmoduleBlock(ModuleFile &F, unsigned ClientLoadCapabilities) {
case SUBMODULE_UMBRELLA_DIR: {
std::string Dirname = std::string(Blob);
ResolveImportedPath(F, Dirname);
if (auto Umbrella = PP.getFileManager().getDirectory(Dirname)) {
if (auto Umbrella =
PP.getFileManager().getOptionalDirectoryRef(Dirname)) {
if (!CurrentModule->getUmbrellaDir())
ModMap.setUmbrellaDir(CurrentModule, *Umbrella, Blob);
else if (CurrentModule->getUmbrellaDir().Entry != *Umbrella) {
Expand Down

0 comments on commit 32c501d

Please sign in to comment.