[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    Re: [PATCH] D11403: [Modules] Add Darwin-specific compatibility module map parsing hacks
From:       Richard Smith <richard () metafoo ! co ! uk>
Date:       2015-08-05 21:45:08
Message-ID: ca4d8301e92782128d6ce67c8b841a19 () localhost ! localdomain
[Download RAW message or body]

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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[prev in list] [next in list] [prev in thread] [next in thread]