[clang][modules] Use file name as requested

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;
 

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() {

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;
   }
 

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 *>;

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;

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);
   }
 }

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;
 }
 

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;

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;

clang/lib/Serialization/ASTReader.cpp

@@ -4156,7 +4156,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);

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"
+        }
+      ]
+    }
+  ]
+}

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>