rmaz created this revision.
Herald added a subscriber: arphaman.
Herald added a project: All.
rmaz requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
We are seeing issues with path serialization of the
`SUBMODULE_TOPHEADER` field when the inputs are links. While working
on a fix I noticed this field does not appear to be used anywhere
outside of libclang. This diff removes the field entirely and
updates the libclang API to always return empty top headers.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D142028
Files:
clang/include/clang/Basic/Module.h
clang/include/clang/Serialization/ASTBitCodes.h
clang/lib/Basic/Module.cpp
clang/lib/Frontend/FrontendAction.cpp
clang/lib/Lex/ModuleMap.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/Index/annotate-module.m
clang/test/Modules/relative-submodule-topheader.m
clang/tools/libclang/CIndex.cpp
Index: clang/tools/libclang/CIndex.cpp
===================================================================
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -8777,31 +8777,15 @@
CXModule CXMod) {
if (isNotUsableTU(TU)) {
LOG_BAD_TU(TU);
- return 0;
}
- if (!CXMod)
- return 0;
- Module *Mod = static_cast<Module *>(CXMod);
- FileManager &FileMgr = cxtu::getASTUnit(TU)->getFileManager();
- ArrayRef<const FileEntry *> TopHeaders = Mod->getTopHeaders(FileMgr);
- return TopHeaders.size();
+ return 0;
}
CXFile clang_Module_getTopLevelHeader(CXTranslationUnit TU, CXModule CXMod,
unsigned Index) {
if (isNotUsableTU(TU)) {
LOG_BAD_TU(TU);
- return nullptr;
}
- if (!CXMod)
- return nullptr;
- Module *Mod = static_cast<Module *>(CXMod);
- FileManager &FileMgr = cxtu::getASTUnit(TU)->getFileManager();
-
- ArrayRef<const FileEntry *> TopHeaders = Mod->getTopHeaders(FileMgr);
- if (Index < TopHeaders.size())
- return const_cast<FileEntry *>(TopHeaders[Index]);
-
return nullptr;
}
Index: clang/test/Modules/relative-submodule-topheader.m
===================================================================
--- clang/test/Modules/relative-submodule-topheader.m
+++ clang/test/Modules/relative-submodule-topheader.m
@@ -3,8 +3,5 @@
// RUN: llvm-bcanalyzer --dump --disable-histogram %t/mod.pcm | FileCheck %s
// CHECK: <SUBMODULE_HEADER abbrevid=6/> blob data = 'vector.h'
-// CHECK: <SUBMODULE_TOPHEADER abbrevid=7/> blob data = 'vector.h'
// CHECK: <SUBMODULE_HEADER abbrevid=6/> blob data = 'type_traits.h'
-// CHECK: <SUBMODULE_TOPHEADER abbrevid=7/> blob data = 'type_traits.h'
// CHECK: <SUBMODULE_HEADER abbrevid=6/> blob data = 'hash_map.h'
-// CHECK: <SUBMODULE_TOPHEADER abbrevid=7/> blob data = 'hash_map.h'
Index: clang/test/Index/annotate-module.m
===================================================================
--- clang/test/Index/annotate-module.m
+++ clang/test/Index/annotate-module.m
@@ -46,6 +46,4 @@
// RUN: c-index-test -cursor-at=%s:3:11 %s -fmodules-cache-path=%t.cache -fmodules -F %S/../Modules/Inputs \
// RUN: | FileCheck %s -check-prefix=CHECK-CURSOR
-// CHECK-CURSOR: 3:1 ModuleImport=DependsOnModule:3:1 (Definition) Extent=[3:1 - 3:24] Spelling=DependsOnModule ([3:9 - 3:24]) ModuleName=DependsOnModule ({{.*}}DependsOnModule-{{[^.]*}}.pcm) system=0 Headers(2):
-// CHECK-CURSOR-NEXT: {{.*}}other.h
-// CHECK-CURSOR-NEXT: {{.*}}DependsOnModule.h
+// CHECK-CURSOR: 3:1 ModuleImport=DependsOnModule:3:1 (Definition) Extent=[3:1 - 3:24] Spelling=DependsOnModule ([3:9 - 3:24]) ModuleName=DependsOnModule ({{.*}}DependsOnModule-{{[^.]*}}.pcm) system=0 Headers(0):
Index: clang/lib/Serialization/ASTWriter.cpp
===================================================================
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -890,7 +890,6 @@
RECORD(SUBMODULE_DEFINITION);
RECORD(SUBMODULE_UMBRELLA_HEADER);
RECORD(SUBMODULE_HEADER);
- RECORD(SUBMODULE_TOPHEADER);
RECORD(SUBMODULE_UMBRELLA_DIR);
RECORD(SUBMODULE_IMPORTS);
RECORD(SUBMODULE_AFFECTING_MODULES);
@@ -2741,11 +2740,6 @@
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Name
unsigned HeaderAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
- Abbrev = std::make_shared<BitCodeAbbrev>();
- Abbrev->Add(BitCodeAbbrevOp(SUBMODULE_TOPHEADER));
- Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Name
- unsigned TopHeaderAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
-
Abbrev = std::make_shared<BitCodeAbbrev>();
Abbrev->Add(BitCodeAbbrevOp(SUBMODULE_UMBRELLA_DIR));
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Name
@@ -2873,17 +2867,6 @@
Stream.EmitRecordWithBlob(HL.Abbrev, Record, H.NameAsWritten);
}
- // Emit the top headers.
- {
- auto TopHeaders = Mod->getTopHeaders(PP->getFileManager());
- RecordData::value_type Record[] = {SUBMODULE_TOPHEADER};
- for (auto *H : TopHeaders) {
- SmallString<128> HeaderName(H->getName());
- PreparePathForOutput(HeaderName);
- Stream.EmitRecordWithBlob(TopHeaderAbbrev, Record, HeaderName);
- }
- }
-
// Emit the imports.
if (!Mod->Imports.empty()) {
RecordData Record;
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -5688,13 +5688,6 @@
// them here.
break;
- case SUBMODULE_TOPHEADER: {
- std::string HeaderName(Blob);
- ResolveImportedPath(F, HeaderName);
- CurrentModule->addTopHeaderFilename(HeaderName);
- break;
- }
-
case SUBMODULE_UMBRELLA_DIR: {
// See comments in SUBMODULE_UMBRELLA_HEADER
std::string Dirname = std::string(Blob);
Index: clang/lib/Lex/ModuleMap.cpp
===================================================================
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -660,7 +660,6 @@
Explicit).first;
InferredModuleAllowedBy[Result] = UmbrellaModuleMap;
Result->IsInferred = true;
- Result->addTopHeader(File);
// If inferred submodules export everything they import, add a
// wildcard to the set of exports.
Index: clang/lib/Frontend/FrontendAction.cpp
===================================================================
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -321,7 +321,7 @@
}
/// Collect the set of header includes needed to construct the given
-/// module and update the TopHeaders file set of the module.
+/// module.
///
/// \param Module The module we're collecting includes from.
///
@@ -352,7 +352,6 @@
// Add includes for each of these headers.
for (auto HK : {Module::HK_Normal, Module::HK_Private}) {
for (Module::Header &H : Module->Headers[HK]) {
- Module->addTopHeader(H.Entry);
// Use the path as specified in the module map file. We'll look for this
// file relative to the module build directory (the directory containing
// the module map file) so this will find the same file that we found
@@ -361,10 +360,8 @@
Module->IsExternC);
}
}
- // Note that Module->PrivateHeaders will not be a TopHeader.
if (Module::Header UmbrellaHeader = Module->getUmbrellaHeader()) {
- Module->addTopHeader(UmbrellaHeader.Entry);
if (Module->Parent)
// Include the umbrella header for submodules.
addHeaderInclude(UmbrellaHeader.PathRelativeToRootModuleDirectory,
@@ -420,7 +417,6 @@
llvm::sort(Headers, llvm::less_first());
for (auto &H : Headers) {
// Include this header as part of the umbrella directory.
- Module->addTopHeader(H.second);
addHeaderInclude(H.first, Includes, LangOpts, Module->IsExternC);
}
}
Index: clang/lib/Basic/Module.cpp
===================================================================
--- clang/lib/Basic/Module.cpp
+++ clang/lib/Basic/Module.cpp
@@ -249,24 +249,6 @@
Umbrella.dyn_cast<const DirectoryEntry *>()};
}
-void Module::addTopHeader(const FileEntry *File) {
- assert(File);
- TopHeaders.insert(File);
-}
-
-ArrayRef<const FileEntry *> Module::getTopHeaders(FileManager &FileMgr) {
- if (!TopHeaderNames.empty()) {
- for (std::vector<std::string>::iterator
- I = TopHeaderNames.begin(), E = TopHeaderNames.end(); I != E; ++I) {
- if (auto FE = FileMgr.getFile(*I))
- TopHeaders.insert(*FE);
- }
- TopHeaderNames.clear();
- }
-
- return llvm::ArrayRef(TopHeaders.begin(), TopHeaders.end());
-}
-
bool Module::directlyUses(const Module *Requested) {
auto *Top = getTopLevelModule();
Index: clang/include/clang/Serialization/ASTBitCodes.h
===================================================================
--- clang/include/clang/Serialization/ASTBitCodes.h
+++ clang/include/clang/Serialization/ASTBitCodes.h
@@ -784,57 +784,54 @@
/// Specifies a header that falls into this (sub)module.
SUBMODULE_HEADER = 3,
- /// Specifies a top-level header that falls into this (sub)module.
- SUBMODULE_TOPHEADER = 4,
-
/// Specifies an umbrella directory.
- SUBMODULE_UMBRELLA_DIR = 5,
+ SUBMODULE_UMBRELLA_DIR = 4,
/// Specifies the submodules that are imported by this
/// submodule.
- SUBMODULE_IMPORTS = 6,
+ SUBMODULE_IMPORTS = 5,
/// Specifies the submodules that are re-exported from this
/// submodule.
- SUBMODULE_EXPORTS = 7,
+ SUBMODULE_EXPORTS = 6,
/// Specifies a required feature.
- SUBMODULE_REQUIRES = 8,
+ SUBMODULE_REQUIRES = 7,
/// Specifies a header that has been explicitly excluded
/// from this submodule.
- SUBMODULE_EXCLUDED_HEADER = 9,
+ SUBMODULE_EXCLUDED_HEADER = 8,
/// Specifies a library or framework to link against.
- SUBMODULE_LINK_LIBRARY = 10,
+ SUBMODULE_LINK_LIBRARY = 9,
/// Specifies a configuration macro for this module.
- SUBMODULE_CONFIG_MACRO = 11,
+ SUBMODULE_CONFIG_MACRO = 10,
/// Specifies a conflict with another module.
- SUBMODULE_CONFLICT = 12,
+ SUBMODULE_CONFLICT = 11,
/// Specifies a header that is private to this submodule.
- SUBMODULE_PRIVATE_HEADER = 13,
+ SUBMODULE_PRIVATE_HEADER = 12,
/// Specifies a header that is part of the module but must be
/// textually included.
- SUBMODULE_TEXTUAL_HEADER = 14,
+ SUBMODULE_TEXTUAL_HEADER = 13,
/// Specifies a header that is private to this submodule but
/// must be textually included.
- SUBMODULE_PRIVATE_TEXTUAL_HEADER = 15,
+ SUBMODULE_PRIVATE_TEXTUAL_HEADER = 14,
/// Specifies some declarations with initializers that must be
/// emitted to initialize the module.
- SUBMODULE_INITIALIZERS = 16,
+ SUBMODULE_INITIALIZERS = 15,
/// Specifies the name of the module that will eventually
/// re-export the entities in this module.
- SUBMODULE_EXPORT_AS = 17,
+ SUBMODULE_EXPORT_AS = 16,
/// Specifies affecting modules that were not imported.
- SUBMODULE_AFFECTING_MODULES = 18,
+ SUBMODULE_AFFECTING_MODULES = 17,
};
/// Record types used within a comments block.
Index: clang/include/clang/Basic/Module.h
===================================================================
--- clang/include/clang/Basic/Module.h
+++ clang/include/clang/Basic/Module.h
@@ -187,12 +187,6 @@
/// corresponding serialized AST file, or null otherwise.
OptionalFileEntryRef ASTFile;
- /// The top-level headers associated with this module.
- llvm::SmallSetVector<const FileEntry *, 2> TopHeaders;
-
- /// top-level header filenames that aren't resolved to FileEntries yet.
- std::vector<std::string> TopHeaderNames;
-
/// Cache of modules visible to lookup in this module.
mutable llvm::DenseSet<const Module*> VisibleModulesCache;
@@ -634,17 +628,6 @@
return Umbrella && Umbrella.is<const DirectoryEntry *>();
}
- /// Add a top-level header associated with this module.
- void addTopHeader(const FileEntry *File);
-
- /// Add a top-level header filename associated with this module.
- void addTopHeaderFilename(StringRef Filename) {
- TopHeaderNames.push_back(std::string(Filename));
- }
-
- /// The top-level headers associated with this module.
- ArrayRef<const FileEntry *> getTopHeaders(FileManager &FileMgr);
-
/// Determine whether this module has declared its intention to
/// directly use another module.
bool directlyUses(const Module *Requested);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
