[PATCH] D120465: [clang][deps] Generate necessary "-fmodule-map-file=" arguments, disable implicit module maps
Duncan P. N. Exon Smith via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 1 16:48:07 PST 2022
dexonsmith added inline comments.
Herald added a project: All.
================
Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:112
void dependencies::detail::collectPCMAndModuleMapPaths(
llvm::ArrayRef<ModuleID> Modules,
----------------
Should this be renamed?
================
Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:125-126
PCMPaths.push_back(LookupPCMPath(MID).str());
- if (!M.ClangModuleMapFile.empty())
- ModMapPaths.push_back(M.ClangModuleMapFile);
}
----------------
Can you clarify why this is safe to remove, even though sometimes we do need to add module map files?
================
Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:269-273
+ // However, some module maps loaded implicitly during the dependency scan can
+ // describe anti-dependencies. That happens when the current module is marked
+ // as '[no_undeclared_includes]', doesn't 'use' module from such module map,
+ // but tries to import it anyway. We need to tell the explicit build about
+ // such module map for it to have the same semantics as the implicit build.
----------------
Is there another long-term solution to this that could be pointed at with a FIXME? E.g., could the module map be encoded redundantly here? If so, what else would need to change to make that okay?
================
Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:275-278
+ // We don't have a good way to determine which module map described the
+ // anti-dependency (let alone what's the corresponding top-level module
+ // map). We simply specify all the module maps in the order they were loaded
+ // during the implicit build during scan.
----------------
Is there a FIXME to leave behind for tracking the anti-dependencies?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120465/new/
https://reviews.llvm.org/D120465
More information about the cfe-commits
mailing list