[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]