[clang] 1afb313 - [clang][modules] Use file name as requested (#68957)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 20 09:23:23 PDT 2023
Author: Jan Svoboda
Date: 2023-10-20T09:23:19-07:00
New Revision: 1afb313b26851bdb050061f6d786c83ec1f569a3
URL: https://github.com/llvm/llvm-project/commit/1afb313b26851bdb050061f6d786c83ec1f569a3
DIFF: https://github.com/llvm/llvm-project/commit/1afb313b26851bdb050061f6d786c83ec1f569a3.diff
LOG: [clang][modules] Use file name as requested (#68957)
This prevents redefinition errors due to having multiple paths for the
same module map. (rdar://24116019)
Originally implemented and tested downstream by @bcardosolopes, I just
made use of `FileEntryRef::getNameAsRequested()`.
Added:
clang/test/Modules/Inputs/all-product-headers.yaml
clang/test/Modules/modulemap-collision.m
Modified:
clang/include/clang/ARCMigrate/FileRemapper.h
clang/include/clang/Basic/FileEntry.h
clang/include/clang/Basic/Module.h
clang/include/clang/Frontend/VerifyDiagnosticConsumer.h
clang/include/clang/Lex/ModuleMap.h
clang/lib/ARCMigrate/FileRemapper.cpp
clang/lib/Basic/Module.cpp
clang/lib/Lex/HeaderSearch.cpp
clang/lib/Lex/ModuleMap.cpp
clang/lib/Serialization/ASTReader.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/ARCMigrate/FileRemapper.h b/clang/include/clang/ARCMigrate/FileRemapper.h
index ceb3e67b5035272..afcee363516a211 100644
--- a/clang/include/clang/ARCMigrate/FileRemapper.h
+++ b/clang/include/clang/ARCMigrate/FileRemapper.h
@@ -12,10 +12,10 @@
#include "clang/Basic/FileEntry.h"
#include "clang/Basic/LLVM.h"
#include "llvm/ADT/DenseMap.h"
-#include "llvm/ADT/PointerUnion.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringRef.h"
#include <memory>
+#include <variant>
namespace llvm {
class MemoryBuffer;
@@ -33,7 +33,7 @@ class FileRemapper {
// FIXME: Reuse the same FileManager for multiple ASTContexts.
std::unique_ptr<FileManager> FileMgr;
- typedef llvm::PointerUnion<FileEntryRef, llvm::MemoryBuffer *> Target;
+ using Target = std::variant<FileEntryRef, llvm::MemoryBuffer *>;
using MappingsTy = llvm::DenseMap<FileEntryRef, Target>;
MappingsTy FromToMappings;
diff --git a/clang/include/clang/Basic/FileEntry.h b/clang/include/clang/Basic/FileEntry.h
index 23237a9326f84b6..bc6546373548841 100644
--- a/clang/include/clang/Basic/FileEntry.h
+++ b/clang/include/clang/Basic/FileEntry.h
@@ -235,37 +235,6 @@ static_assert(std::is_trivially_copyable<OptionalFileEntryRef>::value,
namespace llvm {
-template <> struct PointerLikeTypeTraits<clang::FileEntryRef> {
- static inline void *getAsVoidPointer(clang::FileEntryRef File) {
- return const_cast<clang::FileEntryRef::MapEntry *>(&File.getMapEntry());
- }
-
- static inline clang::FileEntryRef getFromVoidPointer(void *Ptr) {
- return clang::FileEntryRef(
- *reinterpret_cast<const clang::FileEntryRef::MapEntry *>(Ptr));
- }
-
- static constexpr int NumLowBitsAvailable = PointerLikeTypeTraits<
- const clang::FileEntryRef::MapEntry *>::NumLowBitsAvailable;
-};
-
-template <> struct PointerLikeTypeTraits<clang::OptionalFileEntryRef> {
- static inline void *getAsVoidPointer(clang::OptionalFileEntryRef File) {
- if (!File)
- return nullptr;
- return PointerLikeTypeTraits<clang::FileEntryRef>::getAsVoidPointer(*File);
- }
-
- static inline clang::OptionalFileEntryRef getFromVoidPointer(void *Ptr) {
- if (!Ptr)
- return std::nullopt;
- return PointerLikeTypeTraits<clang::FileEntryRef>::getFromVoidPointer(Ptr);
- }
-
- static constexpr int NumLowBitsAvailable =
- PointerLikeTypeTraits<clang::FileEntryRef>::NumLowBitsAvailable;
-};
-
/// Specialisation of DenseMapInfo for FileEntryRef.
template <> struct DenseMapInfo<clang::FileEntryRef> {
static inline clang::FileEntryRef getEmptyKey() {
diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h
index 676fd372493a3aa..6a7423938bdb8fa 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -35,6 +35,7 @@
#include <optional>
#include <string>
#include <utility>
+#include <variant>
#include <vector>
namespace llvm {
@@ -162,7 +163,7 @@ class alignas(8) Module {
std::string PresumedModuleMapFile;
/// The umbrella header or directory.
- llvm::PointerUnion<FileEntryRef, DirectoryEntryRef> Umbrella;
+ std::variant<std::monostate, FileEntryRef, DirectoryEntryRef> Umbrella;
/// The module signature.
ASTFileSignature Signature;
@@ -665,18 +666,17 @@ class alignas(8) Module {
/// Retrieve the umbrella directory as written.
std::optional<DirectoryName> getUmbrellaDirAsWritten() const {
- if (Umbrella && Umbrella.is<DirectoryEntryRef>())
+ if (const auto *Dir = std::get_if<DirectoryEntryRef>(&Umbrella))
return DirectoryName{UmbrellaAsWritten,
- UmbrellaRelativeToRootModuleDirectory,
- Umbrella.get<DirectoryEntryRef>()};
+ UmbrellaRelativeToRootModuleDirectory, *Dir};
return std::nullopt;
}
/// Retrieve the umbrella header as written.
std::optional<Header> getUmbrellaHeaderAsWritten() const {
- if (Umbrella && Umbrella.is<FileEntryRef>())
+ if (const auto *Hdr = std::get_if<FileEntryRef>(&Umbrella))
return Header{UmbrellaAsWritten, UmbrellaRelativeToRootModuleDirectory,
- Umbrella.get<FileEntryRef>()};
+ *Hdr};
return std::nullopt;
}
diff --git a/clang/include/clang/Frontend/VerifyDiagnosticConsumer.h b/clang/include/clang/Frontend/VerifyDiagnosticConsumer.h
index e0f3570e5b2bc6e..500a7e11ab9ac2a 100644
--- a/clang/include/clang/Frontend/VerifyDiagnosticConsumer.h
+++ b/clang/include/clang/Frontend/VerifyDiagnosticConsumer.h
@@ -278,14 +278,15 @@ class VerifyDiagnosticConsumer: public DiagnosticConsumer,
// These facilities are used for validation in debug builds.
class UnparsedFileStatus {
- llvm::PointerIntPair<OptionalFileEntryRef, 1, bool> Data;
+ OptionalFileEntryRef File;
+ bool FoundDirectives;
public:
UnparsedFileStatus(OptionalFileEntryRef File, bool FoundDirectives)
- : Data(File, FoundDirectives) {}
+ : File(File), FoundDirectives(FoundDirectives) {}
- OptionalFileEntryRef getFile() const { return Data.getPointer(); }
- bool foundDirectives() const { return Data.getInt(); }
+ OptionalFileEntryRef getFile() const { return File; }
+ bool foundDirectives() const { return FoundDirectives; }
};
using ParsedFilesMap = llvm::DenseMap<FileID, const FileEntry *>;
diff --git a/clang/include/clang/Lex/ModuleMap.h b/clang/include/clang/Lex/ModuleMap.h
index fc49742ad4af2c1..054d4211b9c6d41 100644
--- a/clang/include/clang/Lex/ModuleMap.h
+++ b/clang/include/clang/Lex/ModuleMap.h
@@ -20,8 +20,8 @@
#include "clang/Basic/SourceLocation.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/PointerIntPair.h"
-#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
@@ -194,7 +194,7 @@ class ModuleMap {
}
};
- using AdditionalModMapsSet = llvm::SmallPtrSet<FileEntryRef, 1>;
+ using AdditionalModMapsSet = llvm::DenseSet<FileEntryRef>;
private:
friend class ModuleMapParser;
diff --git a/clang/lib/ARCMigrate/FileRemapper.cpp b/clang/lib/ARCMigrate/FileRemapper.cpp
index bd8317e0a24515f..7abc862ceecc237 100644
--- a/clang/lib/ARCMigrate/FileRemapper.cpp
+++ b/clang/lib/ARCMigrate/FileRemapper.cpp
@@ -134,9 +134,8 @@ bool FileRemapper::flushToFile(StringRef outputPath, DiagnosticsEngine &Diag) {
infoOut << origPath << '\n';
infoOut << (uint64_t)origFE.getModificationTime() << '\n';
- if (I->second.is<FileEntryRef>()) {
- auto FE = I->second.get<FileEntryRef>();
- SmallString<200> newPath = StringRef(FE.getName());
+ if (const auto *FE = std::get_if<FileEntryRef>(&I->second)) {
+ SmallString<200> newPath = StringRef(FE->getName());
fs::make_absolute(newPath);
infoOut << newPath << '\n';
} else {
@@ -150,7 +149,7 @@ bool FileRemapper::flushToFile(StringRef outputPath, DiagnosticsEngine &Diag) {
return report("Could not create file: " + tempPath.str(), Diag);
llvm::raw_fd_ostream newOut(fd, /*shouldClose=*/true);
- llvm::MemoryBuffer *mem = I->second.get<llvm::MemoryBuffer *>();
+ llvm::MemoryBuffer *mem = std::get<llvm::MemoryBuffer *>(I->second);
newOut.write(mem->getBufferStart(), mem->getBufferSize());
newOut.close();
@@ -173,7 +172,7 @@ bool FileRemapper::overwriteOriginal(DiagnosticsEngine &Diag,
for (MappingsTy::iterator
I = FromToMappings.begin(), E = FromToMappings.end(); I != E; ++I) {
FileEntryRef origFE = I->first;
- assert(I->second.is<llvm::MemoryBuffer *>());
+ assert(std::holds_alternative<llvm::MemoryBuffer *>(I->second));
if (!fs::exists(origFE.getName()))
return report(StringRef("File does not exist: ") + origFE.getName(),
Diag);
@@ -183,7 +182,7 @@ bool FileRemapper::overwriteOriginal(DiagnosticsEngine &Diag,
if (EC)
return report(EC.message(), Diag);
- llvm::MemoryBuffer *mem = I->second.get<llvm::MemoryBuffer *>();
+ llvm::MemoryBuffer *mem = std::get<llvm::MemoryBuffer *>(I->second);
Out.write(mem->getBufferStart(), mem->getBufferSize());
Out.close();
}
@@ -197,25 +196,23 @@ void FileRemapper::forEachMapping(
llvm::function_ref<void(StringRef, const llvm::MemoryBufferRef &)>
CaptureBuffer) const {
for (auto &Mapping : FromToMappings) {
- if (Mapping.second.is<FileEntryRef>()) {
- auto FE = Mapping.second.get<FileEntryRef>();
- CaptureFile(Mapping.first.getName(), FE.getName());
+ if (const auto *FE = std::get_if<FileEntryRef>(&Mapping.second)) {
+ CaptureFile(Mapping.first.getName(), FE->getName());
continue;
}
CaptureBuffer(
Mapping.first.getName(),
- Mapping.second.get<llvm::MemoryBuffer *>()->getMemBufferRef());
+ std::get<llvm::MemoryBuffer *>(Mapping.second)->getMemBufferRef());
}
}
void FileRemapper::applyMappings(PreprocessorOptions &PPOpts) const {
for (MappingsTy::const_iterator
I = FromToMappings.begin(), E = FromToMappings.end(); I != E; ++I) {
- if (I->second.is<FileEntryRef>()) {
- auto FE = I->second.get<FileEntryRef>();
- PPOpts.addRemappedFile(I->first.getName(), FE.getName());
+ if (const auto *FE = std::get_if<FileEntryRef>(&I->second)) {
+ PPOpts.addRemappedFile(I->first.getName(), FE->getName());
} else {
- llvm::MemoryBuffer *mem = I->second.get<llvm::MemoryBuffer *>();
+ llvm::MemoryBuffer *mem = std::get<llvm::MemoryBuffer *>(I->second);
PPOpts.addRemappedFile(I->first.getName(), mem);
}
}
@@ -230,18 +227,20 @@ void FileRemapper::remap(StringRef filePath,
remap(*File, std::move(memBuf));
}
-void FileRemapper::remap(FileEntryRef file,
- std::unique_ptr<llvm::MemoryBuffer> memBuf) {
- Target &targ = FromToMappings[file];
- resetTarget(targ);
- targ = memBuf.release();
+void FileRemapper::remap(FileEntryRef File,
+ std::unique_ptr<llvm::MemoryBuffer> MemBuf) {
+ auto [It, New] = FromToMappings.insert({File, nullptr});
+ if (!New)
+ resetTarget(It->second);
+ It->second = MemBuf.release();
}
-void FileRemapper::remap(FileEntryRef file, FileEntryRef newfile) {
- Target &targ = FromToMappings[file];
- resetTarget(targ);
- targ = newfile;
- ToFromMappings.insert({newfile, file});
+void FileRemapper::remap(FileEntryRef File, FileEntryRef NewFile) {
+ auto [It, New] = FromToMappings.insert({File, nullptr});
+ if (!New)
+ resetTarget(It->second);
+ It->second = NewFile;
+ ToFromMappings.insert({NewFile, File});
}
OptionalFileEntryRef FileRemapper::getOriginalFile(StringRef filePath) {
@@ -259,13 +258,11 @@ OptionalFileEntryRef FileRemapper::getOriginalFile(StringRef filePath) {
}
void FileRemapper::resetTarget(Target &targ) {
- if (!targ)
- return;
-
- if (llvm::MemoryBuffer *oldmem = targ.dyn_cast<llvm::MemoryBuffer *>()) {
+ if (std::holds_alternative<llvm::MemoryBuffer *>(targ)) {
+ llvm::MemoryBuffer *oldmem = std::get<llvm::MemoryBuffer *>(targ);
delete oldmem;
} else {
- FileEntryRef toFE = targ.get<FileEntryRef>();
+ FileEntryRef toFE = std::get<FileEntryRef>(targ);
ToFromMappings.erase(toFE);
}
}
diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index 7879a11179b6d4b..cc2e5be98d32d34 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -265,10 +265,10 @@ bool Module::fullModuleNameIs(ArrayRef<StringRef> nameParts) const {
}
OptionalDirectoryEntryRef Module::getEffectiveUmbrellaDir() const {
- if (Umbrella && Umbrella.is<FileEntryRef>())
- return Umbrella.get<FileEntryRef>().getDir();
- if (Umbrella && Umbrella.is<DirectoryEntryRef>())
- return Umbrella.get<DirectoryEntryRef>();
+ if (const auto *Hdr = std::get_if<FileEntryRef>(&Umbrella))
+ return Hdr->getDir();
+ if (const auto *Dir = std::get_if<DirectoryEntryRef>(&Umbrella))
+ return *Dir;
return std::nullopt;
}
diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 4c8b64a374b47d5..cf1c0cc5284f316 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -1623,7 +1623,7 @@ bool HeaderSearch::findUsableModuleForHeader(
ModuleMap::KnownHeader *SuggestedModule, bool IsSystemHeaderDir) {
if (needModuleLookup(RequestingModule, SuggestedModule)) {
// If there is a module that corresponds to this header, suggest it.
- hasModuleMap(File.getName(), Root, IsSystemHeaderDir);
+ hasModuleMap(File.getNameAsRequested(), Root, IsSystemHeaderDir);
return suggestModule(*this, File, RequestingModule, SuggestedModule);
}
return true;
diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index f65a5f145c04395..7fd92bff8048429 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -2426,7 +2426,8 @@ void ModuleMapParser::parseHeaderDecl(MMToken::TokenKind LeadingToken,
Header.Kind = Map.headerRoleToKind(Role);
// Check whether we already have an umbrella.
- if (Header.IsUmbrella && ActiveModule->Umbrella) {
+ if (Header.IsUmbrella &&
+ !std::holds_alternative<std::monostate>(ActiveModule->Umbrella)) {
Diags.Report(Header.FileNameLoc, diag::err_mmap_umbrella_clash)
<< ActiveModule->getFullModuleName();
HadError = true;
@@ -2523,7 +2524,7 @@ void ModuleMapParser::parseUmbrellaDirDecl(SourceLocation UmbrellaLoc) {
SourceLocation DirNameLoc = consumeToken();
// Check whether we already have an umbrella.
- if (ActiveModule->Umbrella) {
+ if (!std::holds_alternative<std::monostate>(ActiveModule->Umbrella)) {
Diags.Report(DirNameLoc, diag::err_mmap_umbrella_clash)
<< ActiveModule->getFullModuleName();
HadError = true;
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index cce403d7c6c44d0..9061efb6511db51 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -4175,7 +4175,7 @@ ASTReader::ReadModuleMapFileBlock(RecordData &Record, ModuleFile &F,
return OutOfDate;
}
- llvm::SmallPtrSet<FileEntryRef, 1> AdditionalStoredMaps;
+ ModuleMap::AdditionalModMapsSet AdditionalStoredMaps;
for (unsigned I = 0, N = Record[Idx++]; I < N; ++I) {
// FIXME: we should use input files rather than storing names.
std::string Filename = ReadPath(F, Record, Idx);
diff --git a/clang/test/Modules/Inputs/all-product-headers.yaml b/clang/test/Modules/Inputs/all-product-headers.yaml
new file mode 100644
index 000000000000000..53d683f2ad2ecc0
--- /dev/null
+++ b/clang/test/Modules/Inputs/all-product-headers.yaml
@@ -0,0 +1,33 @@
+{
+ 'version': 0,
+ 'case-sensitive': 'false',
+ 'roots': [
+ {
+ 'type': 'directory',
+ 'name': "DUMMY_DIR/build/A.framework/PrivateHeaders"
+ 'contents': [
+ {
+ 'type': 'file',
+ 'name': "A.h",
+ 'external-contents': "DUMMY_DIR/sources/A.h"
+ }
+ ]
+ },
+ {
+ 'type': 'directory',
+ 'name': "DUMMY_DIR/build/A.framework/Modules"
+ 'contents': [
+ {
+ 'type': 'file',
+ 'name': "module.modulemap",
+ 'external-contents': "DUMMY_DIR/build/module.modulemap"
+ },
+ {
+ 'type': 'file',
+ 'name': "module.private.modulemap",
+ 'external-contents': "DUMMY_DIR/build/module.private.modulemap"
+ }
+ ]
+ }
+ ]
+}
diff --git a/clang/test/Modules/modulemap-collision.m b/clang/test/Modules/modulemap-collision.m
new file mode 100644
index 000000000000000..5ada45da3dae191
--- /dev/null
+++ b/clang/test/Modules/modulemap-collision.m
@@ -0,0 +1,15 @@
+// REQUIRES: shell
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/sources %t/build
+// RUN: echo "// A.h" > %t/sources/A.h
+// RUN: echo "framework module A {}" > %t/sources/module.modulemap
+// RUN: echo "framework module A.Private { umbrella header \"A.h\" }" > %t/sources/module.private.modulemap
+// RUN: cp %t/sources/module.modulemap %t/build/module.modulemap
+// RUN: cp %t/sources/module.private.modulemap %t/build/module.private.modulemap
+
+// RUN: sed -e "s:DUMMY_DIR:%t:g" %S/Inputs/all-product-headers.yaml > %t/build/all-product-headers.yaml
+// RUN: %clang_cc1 -fsyntax-only -ivfsoverlay %t/build/all-product-headers.yaml -F%t/build -fmodules -fimplicit-module-maps -Wno-private-module -fmodules-cache-path=%t/cache -x objective-c %s -verify
+
+// expected-no-diagnostics
+#import <A/A.h>
More information about the cfe-commits
mailing list