[PATCH] D151854: [clang] Use `FileEntryRef` in modular header search (part 1/2)

[Jan Svoboda via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb092f417db21: [clang] Use `FileEntryRef` in modular header 
search (part 1/2) (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151854/new/

https://reviews.llvm.org/D151854

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/ModuleMap.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -8799,7 +8799,8 @@
 
   ASTUnit  = *cxtu::getASTUnit(TU);
   HeaderSearch  = Unit.getPreprocessor().getHeaderSearchInfo();
-  ModuleMap::KnownHeader Header = HS.findModuleForHeader(FE);
+  // TODO: Make CXFile a FileEntryRef.
+  ModuleMap::KnownHeader Header = HS.findModuleForHeader(FE->getLastRef());
 
   return Header.getModule();
 }
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -409,29 +409,27 @@
   return Known;
 }
 
-ModuleMap::KnownHeader
-ModuleMap::findHeaderInUmbrellaDirs(const FileEntry *File,
-SmallVectorImpl ) {
+ModuleMap::KnownHeader ModuleMap::findHeaderInUmbrellaDirs(
+FileEntryRef File, SmallVectorImpl ) {
   if (UmbrellaDirs.empty())
 return {};
 
-  const DirectoryEntry *Dir = File->getDir();
-  assert(Dir && "file in no directory");
+  OptionalDirectoryEntryRef Dir = File.getDir();
 
   // Note: as an egregious but useful hack we use the real path here, because
   // frameworks moving from top-level frameworks to embedded frameworks tend
   // to be symlinked from the top-level location to the embedded location,
   // and we need to resolve lookups as if we had found the embedded location.
-  StringRef DirName = SourceMgr.getFileManager().getCanonicalName(Dir);
+  StringRef DirName = SourceMgr.getFileManager().getCanonicalName(*Dir);
 
   // Keep walking up the directory hierarchy, looking for a directory with
   // an umbrella header.
   do {
-auto KnownDir = UmbrellaDirs.find(Dir);
+auto KnownDir = UmbrellaDirs.find(*Dir);
 if (KnownDir != UmbrellaDirs.end())
   return KnownHeader(KnownDir->second, NormalHeader);
 
-IntermediateDirs.push_back(Dir);
+IntermediateDirs.push_back(*Dir);
 
 // Retrieve our parent path.
 DirName = llvm::sys::path::parent_path(DirName);
@@ -439,10 +437,7 @@
   break;
 
 // Resolve the parent path to a directory entry.
-if (auto DirEntry = SourceMgr.getFileManager().getDirectory(DirName))
-  Dir = *DirEntry;
-else
-  Dir = nullptr;
+Dir = SourceMgr.getFileManager().getOptionalDirectoryRef(DirName);
   } while (Dir);
   return {};
 }
@@ -582,7 +577,7 @@
   return false;
 }
 
-ModuleMap::KnownHeader ModuleMap::findModuleForHeader(const FileEntry *File,
+ModuleMap::KnownHeader ModuleMap::findModuleForHeader(FileEntryRef File,
   bool AllowTextual,
   bool AllowExcluded) {
   auto MakeResult = [&](ModuleMap::KnownHeader R) -> ModuleMap::KnownHeader {
@@ -612,10 +607,10 @@
 }
 
 ModuleMap::KnownHeader
-ModuleMap::findOrCreateModuleForHeaderInUmbrellaDir(const FileEntry *File) {
+ModuleMap::findOrCreateModuleForHeaderInUmbrellaDir(FileEntryRef File) {
   assert(!Headers.count(File) && "already have a module for this header");
 
-  SmallVector SkippedDirs;
+  SmallVector SkippedDirs;
   KnownHeader H = findHeaderInUmbrellaDirs(File, SkippedDirs);
   if (H) {
 Module *Result = H.getModule();
@@ -635,11 +630,11 @@
   // the actual header is located.
   bool Explicit = UmbrellaModule->InferExplicitSubmodules;
 
-  for (const DirectoryEntry *SkippedDir : llvm::reverse(SkippedDirs)) {
+  for (DirectoryEntryRef SkippedDir : llvm::reverse(SkippedDirs)) {
 // Find or create the module that corresponds to this directory name.
 SmallString<32> NameBuf;
 StringRef Name = sanitizeFilenameAsIdentifier(
-llvm::sys::path::stem(SkippedDir->getName()), NameBuf);
+llvm::sys::path::stem(SkippedDir.getName()), NameBuf);
 Result = findOrCreateModule(Name, Result, /*IsFramework=*/false,
 Explicit).first;
 InferredModuleAllowedBy[Result] = UmbrellaModuleMap;
@@ -657,7 +652,7 @@
   // Infer a submodule with the same name as this header file.
   SmallString<32> NameBuf;
   StringRef Name = sanitizeFilenameAsIdentifier(
- llvm::sys::path::stem(File->getName()), NameBuf);
+ llvm::sys::path::stem(File.getName()), NameBuf);
   Result = 

[PATCH] D151854: [clang] Use `FileEntryRef` in modular header search (part 1/2)

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 527490.
jansvoboda11 added a comment.

Remove optionality from one argument


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151854/new/

https://reviews.llvm.org/D151854

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/ModuleMap.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -8799,7 +8799,8 @@
 
   ASTUnit  = *cxtu::getASTUnit(TU);
   HeaderSearch  = Unit.getPreprocessor().getHeaderSearchInfo();
-  ModuleMap::KnownHeader Header = HS.findModuleForHeader(FE);
+  // TODO: Make CXFile a FileEntryRef.
+  ModuleMap::KnownHeader Header = HS.findModuleForHeader(FE->getLastRef());
 
   return Header.getModule();
 }
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -409,29 +409,27 @@
   return Known;
 }
 
-ModuleMap::KnownHeader
-ModuleMap::findHeaderInUmbrellaDirs(const FileEntry *File,
-SmallVectorImpl ) {
+ModuleMap::KnownHeader ModuleMap::findHeaderInUmbrellaDirs(
+FileEntryRef File, SmallVectorImpl ) {
   if (UmbrellaDirs.empty())
 return {};
 
-  const DirectoryEntry *Dir = File->getDir();
-  assert(Dir && "file in no directory");
+  OptionalDirectoryEntryRef Dir = File.getDir();
 
   // Note: as an egregious but useful hack we use the real path here, because
   // frameworks moving from top-level frameworks to embedded frameworks tend
   // to be symlinked from the top-level location to the embedded location,
   // and we need to resolve lookups as if we had found the embedded location.
-  StringRef DirName = SourceMgr.getFileManager().getCanonicalName(Dir);
+  StringRef DirName = SourceMgr.getFileManager().getCanonicalName(*Dir);
 
   // Keep walking up the directory hierarchy, looking for a directory with
   // an umbrella header.
   do {
-auto KnownDir = UmbrellaDirs.find(Dir);
+auto KnownDir = UmbrellaDirs.find(*Dir);
 if (KnownDir != UmbrellaDirs.end())
   return KnownHeader(KnownDir->second, NormalHeader);
 
-IntermediateDirs.push_back(Dir);
+IntermediateDirs.push_back(*Dir);
 
 // Retrieve our parent path.
 DirName = llvm::sys::path::parent_path(DirName);
@@ -439,10 +437,7 @@
   break;
 
 // Resolve the parent path to a directory entry.
-if (auto DirEntry = SourceMgr.getFileManager().getDirectory(DirName))
-  Dir = *DirEntry;
-else
-  Dir = nullptr;
+Dir = SourceMgr.getFileManager().getOptionalDirectoryRef(DirName);
   } while (Dir);
   return {};
 }
@@ -582,7 +577,7 @@
   return false;
 }
 
-ModuleMap::KnownHeader ModuleMap::findModuleForHeader(const FileEntry *File,
+ModuleMap::KnownHeader ModuleMap::findModuleForHeader(FileEntryRef File,
   bool AllowTextual,
   bool AllowExcluded) {
   auto MakeResult = [&](ModuleMap::KnownHeader R) -> ModuleMap::KnownHeader {
@@ -612,10 +607,10 @@
 }
 
 ModuleMap::KnownHeader
-ModuleMap::findOrCreateModuleForHeaderInUmbrellaDir(const FileEntry *File) {
+ModuleMap::findOrCreateModuleForHeaderInUmbrellaDir(FileEntryRef File) {
   assert(!Headers.count(File) && "already have a module for this header");
 
-  SmallVector SkippedDirs;
+  SmallVector SkippedDirs;
   KnownHeader H = findHeaderInUmbrellaDirs(File, SkippedDirs);
   if (H) {
 Module *Result = H.getModule();
@@ -635,11 +630,11 @@
   // the actual header is located.
   bool Explicit = UmbrellaModule->InferExplicitSubmodules;
 
-  for (const DirectoryEntry *SkippedDir : llvm::reverse(SkippedDirs)) {
+  for (DirectoryEntryRef SkippedDir : llvm::reverse(SkippedDirs)) {
 // Find or create the module that corresponds to this directory name.
 SmallString<32> NameBuf;
 StringRef Name = sanitizeFilenameAsIdentifier(
-llvm::sys::path::stem(SkippedDir->getName()), NameBuf);
+llvm::sys::path::stem(SkippedDir.getName()), NameBuf);
 Result = findOrCreateModule(Name, Result, /*IsFramework=*/false,
 Explicit).first;
 InferredModuleAllowedBy[Result] = UmbrellaModuleMap;
@@ -657,7 +652,7 @@
   // Infer a submodule with the same name as this header file.
   SmallString<32> NameBuf;
   StringRef Name = sanitizeFilenameAsIdentifier(
- llvm::sys::path::stem(File->getName()), NameBuf);
+ llvm::sys::path::stem(File.getName()), NameBuf);
   Result = findOrCreateModule(Name, Result, /*IsFramework=*/false,
   Explicit).first;
   InferredModuleAllowedBy[Result] = 

[PATCH] D151854: [clang] Use `FileEntryRef` in modular header search (part 1/2)

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: clang/include/clang/Lex/HeaderSearch.h:763
   /// find this file due to requirements from \p RequestingModule.
-  bool findUsableModuleForHeader(const FileEntry *File,
+  bool findUsableModuleForHeader(OptionalFileEntryRef File,
  const DirectoryEntry *Root,

jansvoboda11 wrote:
> benlangmuir wrote:
> > This should probably be non-Optional. I can't find any calls to this API 
> > that can pass null, they all pass something that is already being 
> > dereferenced.
> You're right, thanks. Fixed in the committed version.
*In the new revision, rather.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151854/new/

https://reviews.llvm.org/D151854

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151854: [clang] Use `FileEntryRef` in modular header search (part 1/2)

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: clang/include/clang/Lex/HeaderSearch.h:763
   /// find this file due to requirements from \p RequestingModule.
-  bool findUsableModuleForHeader(const FileEntry *File,
+  bool findUsableModuleForHeader(OptionalFileEntryRef File,
  const DirectoryEntry *Root,

benlangmuir wrote:
> This should probably be non-Optional. I can't find any calls to this API that 
> can pass null, they all pass something that is already being dereferenced.
You're right, thanks. Fixed in the committed version.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151854/new/

https://reviews.llvm.org/D151854

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151854: [clang] Use `FileEntryRef` in modular header search (part 1/2)

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir added a comment.






Comment at: clang/include/clang/Lex/HeaderSearch.h:763
   /// find this file due to requirements from \p RequestingModule.
-  bool findUsableModuleForHeader(const FileEntry *File,
+  bool findUsableModuleForHeader(OptionalFileEntryRef File,
  const DirectoryEntry *Root,

This should probably be non-Optional. I can't find any calls to this API that 
can pass null, they all pass something that is already being dereferenced.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151854/new/

https://reviews.llvm.org/D151854

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151854: [clang] Use `FileEntryRef` in modular header search (part 1/2)

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: benlangmuir, bnbarham.
Herald added subscribers: ributzka, arphaman.
Herald added a project: All.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch removes some deprecated uses of `{File,Directory}Entry::getName()`. 
No functional change indended.

Depends on D151853 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151854

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/ModuleMap.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -8799,7 +8799,8 @@
 
   ASTUnit  = *cxtu::getASTUnit(TU);
   HeaderSearch  = Unit.getPreprocessor().getHeaderSearchInfo();
-  ModuleMap::KnownHeader Header = HS.findModuleForHeader(FE);
+  // TODO: Make CXFile a FileEntryRef.
+  ModuleMap::KnownHeader Header = HS.findModuleForHeader(FE->getLastRef());
 
   return Header.getModule();
 }
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -409,29 +409,27 @@
   return Known;
 }
 
-ModuleMap::KnownHeader
-ModuleMap::findHeaderInUmbrellaDirs(const FileEntry *File,
-SmallVectorImpl ) {
+ModuleMap::KnownHeader ModuleMap::findHeaderInUmbrellaDirs(
+FileEntryRef File, SmallVectorImpl ) {
   if (UmbrellaDirs.empty())
 return {};
 
-  const DirectoryEntry *Dir = File->getDir();
-  assert(Dir && "file in no directory");
+  OptionalDirectoryEntryRef Dir = File.getDir();
 
   // Note: as an egregious but useful hack we use the real path here, because
   // frameworks moving from top-level frameworks to embedded frameworks tend
   // to be symlinked from the top-level location to the embedded location,
   // and we need to resolve lookups as if we had found the embedded location.
-  StringRef DirName = SourceMgr.getFileManager().getCanonicalName(Dir);
+  StringRef DirName = SourceMgr.getFileManager().getCanonicalName(*Dir);
 
   // Keep walking up the directory hierarchy, looking for a directory with
   // an umbrella header.
   do {
-auto KnownDir = UmbrellaDirs.find(Dir);
+auto KnownDir = UmbrellaDirs.find(*Dir);
 if (KnownDir != UmbrellaDirs.end())
   return KnownHeader(KnownDir->second, NormalHeader);
 
-IntermediateDirs.push_back(Dir);
+IntermediateDirs.push_back(*Dir);
 
 // Retrieve our parent path.
 DirName = llvm::sys::path::parent_path(DirName);
@@ -439,10 +437,7 @@
   break;
 
 // Resolve the parent path to a directory entry.
-if (auto DirEntry = SourceMgr.getFileManager().getDirectory(DirName))
-  Dir = *DirEntry;
-else
-  Dir = nullptr;
+Dir = SourceMgr.getFileManager().getOptionalDirectoryRef(DirName);
   } while (Dir);
   return {};
 }
@@ -582,7 +577,7 @@
   return false;
 }
 
-ModuleMap::KnownHeader ModuleMap::findModuleForHeader(const FileEntry *File,
+ModuleMap::KnownHeader ModuleMap::findModuleForHeader(FileEntryRef File,
   bool AllowTextual,
   bool AllowExcluded) {
   auto MakeResult = [&](ModuleMap::KnownHeader R) -> ModuleMap::KnownHeader {
@@ -612,10 +607,10 @@
 }
 
 ModuleMap::KnownHeader
-ModuleMap::findOrCreateModuleForHeaderInUmbrellaDir(const FileEntry *File) {
+ModuleMap::findOrCreateModuleForHeaderInUmbrellaDir(FileEntryRef File) {
   assert(!Headers.count(File) && "already have a module for this header");
 
-  SmallVector SkippedDirs;
+  SmallVector SkippedDirs;
   KnownHeader H = findHeaderInUmbrellaDirs(File, SkippedDirs);
   if (H) {
 Module *Result = H.getModule();
@@ -635,11 +630,11 @@
   // the actual header is located.
   bool Explicit = UmbrellaModule->InferExplicitSubmodules;
 
-  for (const DirectoryEntry *SkippedDir : llvm::reverse(SkippedDirs)) {
+  for (DirectoryEntryRef SkippedDir : llvm::reverse(SkippedDirs)) {
 // Find or create the module that corresponds to this directory name.
 SmallString<32> NameBuf;
 StringRef Name = sanitizeFilenameAsIdentifier(
-llvm::sys::path::stem(SkippedDir->getName()), NameBuf);
+llvm::sys::path::stem(SkippedDir.getName()), NameBuf);
 Result = findOrCreateModule(Name, Result, /*IsFramework=*/false,
 Explicit).first;
 InferredModuleAllowedBy[Result] = UmbrellaModuleMap;
@@ -657,7 +652,7 @@
   // Infer a submodule with the same name as this header file.
   SmallString<32> NameBuf;
   StringRef Name = sanitizeFilenameAsIdentifier(
-