[PATCH] D151852: [clang] NFCI: Use `FileEntryRef` in `ModuleMapCallbacks`
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf09729042d8f: [clang] NFCI: Use `FileEntryRef` in
`ModuleMapCallbacks` (authored by jansvoboda11).
Changed prior to commit:
https://reviews.llvm.org/D151852?vs=527228=527477#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D151852/new/
https://reviews.llvm.org/D151852
Files:
clang/include/clang/Lex/ModuleMap.h
clang/lib/Frontend/ModuleDependencyCollector.cpp
clang/lib/Lex/ModuleMap.cpp
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -1180,7 +1180,7 @@
// Notify callbacks that we just added a new header.
for (const auto : Callbacks)
-Cb->moduleMapAddUmbrellaHeader((),
UmbrellaHeader);
+Cb->moduleMapAddUmbrellaHeader(UmbrellaHeader);
}
void ModuleMap::setUmbrellaDirAsWritten(
Index: clang/lib/Frontend/ModuleDependencyCollector.cpp
===
--- clang/lib/Frontend/ModuleDependencyCollector.cpp
+++ clang/lib/Frontend/ModuleDependencyCollector.cpp
@@ -72,37 +72,12 @@
if (llvm::sys::path::is_absolute(HeaderPath))
Collector.addFile(HeaderPath);
}
- void moduleMapAddUmbrellaHeader(FileManager *FileMgr,
- const FileEntry *Header) override {
-StringRef HeaderFilename = Header->getName();
-moduleMapAddHeader(HeaderFilename);
-// The FileManager can find and cache the symbolic link for a framework
-// header before its real path, this means a module can have some of its
-// headers to use other paths. Although this is usually not a problem, it's
-// inconsistent, and not collecting the original path header leads to
-// umbrella clashes while rebuilding modules in the crash reproducer. For
-// example:
-//ApplicationServices.framework/Frameworks/ImageIO.framework/ImageIO.h
-// instead of:
-//ImageIO.framework/ImageIO.h
-//
-// FIXME: this shouldn't be necessary once we have FileName instances
-// around instead of FileEntry ones. For now, make sure we collect all
-// that we need for the reproducer to work correctly.
-StringRef UmbreallDirFromHeader =
-llvm::sys::path::parent_path(HeaderFilename);
-StringRef UmbrellaDir = Header->getDir()->getName();
-if (!UmbrellaDir.equals(UmbreallDirFromHeader)) {
- SmallString<128> AltHeaderFilename;
- llvm::sys::path::append(AltHeaderFilename, UmbrellaDir,
- llvm::sys::path::filename(HeaderFilename));
- if (FileMgr->getFile(AltHeaderFilename))
-moduleMapAddHeader(AltHeaderFilename);
-}
+ void moduleMapAddUmbrellaHeader(FileEntryRef Header) override {
+moduleMapAddHeader(Header.getNameAsRequested());
}
};
-}
+} // namespace
void ModuleDependencyCollector::attachToASTReader(ASTReader ) {
R.addListener(
Index: clang/include/clang/Lex/ModuleMap.h
===
--- clang/include/clang/Lex/ModuleMap.h
+++ clang/include/clang/Lex/ModuleMap.h
@@ -67,10 +67,8 @@
/// Called when an umbrella header is added during module map parsing.
///
- /// \param FileMgr FileManager instance
/// \param Header The umbrella header to collect.
- virtual void moduleMapAddUmbrellaHeader(FileManager *FileMgr,
- const FileEntry *Header) {}
+ virtual void moduleMapAddUmbrellaHeader(FileEntryRef Header) {}
};
class ModuleMap {
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -1180,7 +1180,7 @@
// Notify callbacks that we just added a new header.
for (const auto : Callbacks)
-Cb->moduleMapAddUmbrellaHeader((), UmbrellaHeader);
+Cb->moduleMapAddUmbrellaHeader(UmbrellaHeader);
}
void ModuleMap::setUmbrellaDirAsWritten(
Index: clang/lib/Frontend/ModuleDependencyCollector.cpp
===
--- clang/lib/Frontend/ModuleDependencyCollector.cpp
+++ clang/lib/Frontend/ModuleDependencyCollector.cpp
@@ -72,37 +72,12 @@
if (llvm::sys::path::is_absolute(HeaderPath))
Collector.addFile(HeaderPath);
}
- void moduleMapAddUmbrellaHeader(FileManager *FileMgr,
- const FileEntry *Header) override {
-StringRef HeaderFilename = Header->getName();
-moduleMapAddHeader(HeaderFilename);
-// The FileManager can find and cache the symbolic link for a framework
-// header before its real path, this means a module can have some of its
-// headers to use other paths. Although this is usually not a problem, it's
-// inconsistent, and not collecting the original path header leads to
-// umbrella clashes while
[PATCH] D151852: [clang] NFCI: Use `FileEntryRef` in `ModuleMapCallbacks`
benlangmuir accepted this revision.
benlangmuir added a comment.
This revision is now accepted and ready to land.
Nice simplification!
Comment at: clang/include/clang/Lex/ModuleMap.h:72
/// \param Header The umbrella header to collect.
- virtual void moduleMapAddUmbrellaHeader(FileManager *FileMgr,
- const FileEntry *Header) {}
+ virtual void moduleMapAddUmbrellaHeader(FileEntryRef Header) {}
};
Doc comment needs update
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D151852/new/
https://reviews.llvm.org/D151852
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D151852: [clang] NFCI: Use `FileEntryRef` in `ModuleMapCallbacks`
jansvoboda11 created this revision.
jansvoboda11 added reviewers: benlangmuir, bnbarham.
Herald added a subscriber: ributzka.
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 path hackery from `ModuleMapCallbacks` by adopting
`FileEntryRef`. No functional change intended.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D151852
Files:
clang/include/clang/Lex/ModuleMap.h
clang/lib/Frontend/ModuleDependencyCollector.cpp
clang/lib/Lex/ModuleMap.cpp
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -1180,7 +1180,7 @@
// Notify callbacks that we just added a new header.
for (const auto : Callbacks)
-Cb->moduleMapAddUmbrellaHeader((),
UmbrellaHeader);
+Cb->moduleMapAddUmbrellaHeader(UmbrellaHeader);
}
void ModuleMap::setUmbrellaDirAsWritten(
Index: clang/lib/Frontend/ModuleDependencyCollector.cpp
===
--- clang/lib/Frontend/ModuleDependencyCollector.cpp
+++ clang/lib/Frontend/ModuleDependencyCollector.cpp
@@ -72,37 +72,12 @@
if (llvm::sys::path::is_absolute(HeaderPath))
Collector.addFile(HeaderPath);
}
- void moduleMapAddUmbrellaHeader(FileManager *FileMgr,
- const FileEntry *Header) override {
-StringRef HeaderFilename = Header->getName();
-moduleMapAddHeader(HeaderFilename);
-// The FileManager can find and cache the symbolic link for a framework
-// header before its real path, this means a module can have some of its
-// headers to use other paths. Although this is usually not a problem, it's
-// inconsistent, and not collecting the original path header leads to
-// umbrella clashes while rebuilding modules in the crash reproducer. For
-// example:
-//ApplicationServices.framework/Frameworks/ImageIO.framework/ImageIO.h
-// instead of:
-//ImageIO.framework/ImageIO.h
-//
-// FIXME: this shouldn't be necessary once we have FileName instances
-// around instead of FileEntry ones. For now, make sure we collect all
-// that we need for the reproducer to work correctly.
-StringRef UmbreallDirFromHeader =
-llvm::sys::path::parent_path(HeaderFilename);
-StringRef UmbrellaDir = Header->getDir()->getName();
-if (!UmbrellaDir.equals(UmbreallDirFromHeader)) {
- SmallString<128> AltHeaderFilename;
- llvm::sys::path::append(AltHeaderFilename, UmbrellaDir,
- llvm::sys::path::filename(HeaderFilename));
- if (FileMgr->getFile(AltHeaderFilename))
-moduleMapAddHeader(AltHeaderFilename);
-}
+ void moduleMapAddUmbrellaHeader(FileEntryRef Header) override {
+moduleMapAddHeader(Header.getNameAsRequested());
}
};
-}
+} // namespace
void ModuleDependencyCollector::attachToASTReader(ASTReader ) {
R.addListener(
Index: clang/include/clang/Lex/ModuleMap.h
===
--- clang/include/clang/Lex/ModuleMap.h
+++ clang/include/clang/Lex/ModuleMap.h
@@ -69,8 +69,7 @@
///
/// \param FileMgr FileManager instance
/// \param Header The umbrella header to collect.
- virtual void moduleMapAddUmbrellaHeader(FileManager *FileMgr,
- const FileEntry *Header) {}
+ virtual void moduleMapAddUmbrellaHeader(FileEntryRef Header) {}
};
class ModuleMap {
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -1180,7 +1180,7 @@
// Notify callbacks that we just added a new header.
for (const auto : Callbacks)
-Cb->moduleMapAddUmbrellaHeader((), UmbrellaHeader);
+Cb->moduleMapAddUmbrellaHeader(UmbrellaHeader);
}
void ModuleMap::setUmbrellaDirAsWritten(
Index: clang/lib/Frontend/ModuleDependencyCollector.cpp
===
--- clang/lib/Frontend/ModuleDependencyCollector.cpp
+++ clang/lib/Frontend/ModuleDependencyCollector.cpp
@@ -72,37 +72,12 @@
if (llvm::sys::path::is_absolute(HeaderPath))
Collector.addFile(HeaderPath);
}
- void moduleMapAddUmbrellaHeader(FileManager *FileMgr,
- const FileEntry *Header) override {
-StringRef HeaderFilename = Header->getName();
-moduleMapAddHeader(HeaderFilename);
-// The FileManager can find and cache the symbolic link for a framework
-// header before its real path, this means a module can have some of its
-// headers to use other paths. Although this is usually not a problem, it's
-// inconsistent, and not collecting the original path header leads to
-// umbrella clashes while rebuilding modules in
