[PATCH] Allow a header to be part of multiple modules
Daniel Jasper
djasper at google.com
Mon Oct 21 01:51:22 PDT 2013
Addressed review comments.
================
Comment at: include/clang/Lex/ModuleMap.h:213
@@ +212,3 @@
+ /// \param User Used to disambiguate if a header is present in multiple
+ /// modules.
+ ///
----------------
Manuel Klimek wrote:
> Perhaps:
> Specifies the module the header is intended to be used from. Used to ...
Done.
================
Comment at: lib/Lex/HeaderSearch.cpp:508
@@ -505,1 +507,3 @@
{
+ if (!HSOpts->ModuleMapFiles.empty()) {
+ for (llvm::SetVector<std::string>::iterator
----------------
Manuel Klimek wrote:
> I think an explanation of why we pre-load all modules files might help here...
Done.
================
Comment at: lib/Lex/ModuleMap.cpp:189-190
@@ +188,4 @@
+
+ // If uses need to be specified explicitly, do not report modules that
+ // are not used by 'User'.
+ if (User && LangOpts.ModulesDeclUse &&
----------------
Manuel Klimek wrote:
> How about: If uses need to be specified explicitly, we are only allowed to return modules that are explicitly used by the requesting module.
Done, thanks!
================
Comment at: lib/Lex/ModuleMap.cpp:212
@@ -188,9 +211,3 @@
// Check again.
- Known = Headers.find(File);
- if (Known != Headers.end()) {
- // If a header is not available, don't report that it maps to anything.
- if (!Known->second.isAvailable())
- return KnownHeader();
-
- return Known->second;
- }
+ if (Headers.find(File) != Headers.end())
+ return findModuleForHeader(File, User);
----------------
Manuel Klimek wrote:
> Do we need to special-case the 'exlucded' role?
Done. I wonder why this didn't need to be checked before..
http://llvm-reviews.chandlerc.com/D1951
More information about the cfe-commits
mailing list