[PATCH] D11403: [Modules] Add Darwin-specific compatibility module map parsing hacks

[Richard Smith]

rsmith added inline comments.

================
Comment at: lib/Lex/ModuleMap.cpp:1891-1908
@@ +1890,20 @@
+
+  if (ActiveModule->UsesRequiresExcludedHack) {
+    // Mark this header 'textual' (see doc comment for
+    // Module::UsesRequiresExcludedHack). Although iterating over the directory
+    // is relatively expensive, in practice this only applies to the uncommonly
+    // used Tcl module on Darwin platforms.
+    std::error_code EC;
+    for (llvm::sys::fs::recursive_directory_iterator I(Dir->getName(), EC), E;
+         I != E && !EC; I.increment(EC)) {
+      if (const FileEntry *FE = SourceMgr.getFileManager().getFile(I->path())) {
+        // FIXME: Taking the name from the FileEntry is unstable and can give
+        // different results depending on how we've previously named that file
+        // in this build.
+        Module::Header Header = {FE->getName(), FE};
+        Map.addHeader(ActiveModule, Header, ModuleMap::TextualHeader);
+      }
+    }
+    return;
+  }
+
----------------
benlangmuir wrote:
> rsmith wrote:
> > Is there a better way of handling this? If the parent directory isn't itself an umbrella directory of some module, maybe you could just ignore the umbrella directory declaration for this module entirely?
> This only affects Tcl.Private, and Tcl has an umbrella header so I don't think that will work.  The only other way I can think of making this work is to have a notion of a *directory* that is exempt from its containing umbrella, but I'm not sure that's a generally good feature and it seems like a lot more work.  Let me know if you have any suggestions though.
Ugh, OK. In that case:

 * maybe take the file name from `I->path()` rather than `FE->getName()` (we want to imagine the files were named within the umbrella directory rather than some other way)
 * sort the paths before you add them so that the serialized pcm doesn't depend on file system traversal order

Also, you'll be paying this file system traversal cost whenever the relevant module map is parsed, not only when the Tcl module is used -- and if it's the /usr/include module map, you'll walk this umbrella directory on every build. Just wanted to confirm you're OK with that.


Repository:
  rL LLVM

http://reviews.llvm.org/D11403