<div dir="ltr">Please add a testcase!</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Oct 31, 2013 at 3:24 PM, Dmitri Gribenko <span dir="ltr"><<a href="mailto:gribozavr@gmail.com" target="_blank">gribozavr@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: gribozavr<br>
Date: Thu Oct 31 17:24:10 2013<br>
New Revision: 193815<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=193815&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=193815&view=rev</a><br>
Log:<br>
Clang modules: collect exports recursively<br>
<br>
This change makes Module::buildVisibleModulesCache() collect exported modules<br>
recursively.<br>
<br>
While computing a set of exports, getExportedModules() iterates over the set of<br>
imported modules and filters it.  But it does not consider the set of exports<br>
of those modules -- it is the responsibility of the caller to do this.<br>
<br>
Here is a certain instance of this issue.  Module::isModuleVisible says that<br>
CoreFoundation.CFArray submodule is not visible from Cocoa.  Why?<br>
<br>
- Cocoa imports Foundation.<br>
- Foundation has an export restriction: "export *".<br>
- Foundation imports CoreFoundation.  (Just the top-level module.)<br>
- CoreFoundation exports CoreFoundation.CFArray.<br>
<br>
To decide which modules are visible from Cocoa, we collect all exported modules<br>
from immediate imports in Cocoa:<br>
<br>
> visibleModulesFro(Cocoa) = exported(Foundation) + exported(CoreData) + exported(AppKit)<br>
<br>
To find out which modules are exported, we filter imports according to<br>
restrictions:<br>
<br>
> exported(Foundation) = filterByModuleMapRestrictions(imports(Foundation))<br>
<br>
Because Foundation imports CoreFoundation (not CoreFoundation.CFArray), the<br>
CFArray submodule is considered not exported from Foundation, and is not<br>
visible from Cocoa (according to Module::isModuleVisible).<br>
<br>
Modified:<br>
    cfe/trunk/include/clang/Basic/Module.h<br>
    cfe/trunk/lib/Basic/Module.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Basic/Module.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Module.h?rev=193815&r1=193814&r2=193815&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Module.h?rev=193815&r1=193814&r2=193815&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/include/clang/Basic/Module.h (original)<br>
+++ cfe/trunk/include/clang/Basic/Module.h Thu Oct 31 17:24:10 2013<br>
@@ -404,6 +404,9 @@ public:<br>
   submodule_const_iterator submodule_end() const { return SubModules.end(); }<br>
<br>
   /// \brief Returns the exported modules based on the wildcard restrictions.<br>
+  ///<br>
+  /// This returns a subset of immediately imported modules (the ones that are<br>
+  /// exported), not the complete set of exported modules.<br>
   void getExportedModules(SmallVectorImpl<Module *> &Exported) const;<br>
<br>
   static StringRef getModuleInputBufferName() {<br>
<br>
Modified: cfe/trunk/lib/Basic/Module.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Module.cpp?rev=193815&r1=193814&r2=193815&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Module.cpp?rev=193815&r1=193814&r2=193815&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/lib/Basic/Module.cpp (original)<br>
+++ cfe/trunk/lib/Basic/Module.cpp Thu Oct 31 17:24:10 2013<br>
@@ -252,15 +252,23 @@ void Module::buildVisibleModulesCache()<br>
   // This module is visible to itself.<br>
   VisibleModulesCache.insert(this);<br>
<br>
-  llvm::SmallVector<Module*, 4> Exported;<br>
-  for (unsigned I = 0, N = Imports.size(); I != N; ++I) {<br>
-    // Every imported module is visible.<br>
-    VisibleModulesCache.insert(Imports[I]);<br>
+  // Every imported module is visible.<br>
+  // Every module exported by an imported module is visible.<br>
+  llvm::SmallPtrSet<Module *, 4> Visited;<br>
+  llvm::SmallVector<Module *, 4> Exports;<br>
+  SmallVector<Module *, 4> Stack(Imports.begin(), Imports.end());<br>
+  while (!Stack.empty()) {<br>
+    Module *CurrModule = Stack.pop_back_val();<br>
+    VisibleModulesCache.insert(CurrModule);<br>
<br>
-    // Every module exported by an imported module is visible.<br>
-    Imports[I]->getExportedModules(Exported);<br>
-    VisibleModulesCache.insert(Exported.begin(), Exported.end());<br>
-    Exported.clear();<br>
+    CurrModule->getExportedModules(Exports);<br>
+    for (SmallVectorImpl<Module *>::iterator I = Exports.begin(),<br>
+                                             E = Exports.end();<br>
+         I != E; ++I) {<br>
+      Module *Exported = *I;<br>
+      if (Visited.insert(Exported))<br>
+        Stack.push_back(Exported);<br>
+    }<br>
   }<br>
 }<br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>