[PATCH] Teach CC1 to dump module dependencies to a directory

Argyrios Kyrtzidis kyrtzidis at apple.com
Fri Jun 13 16:37:39 PDT 2014


Hi Justin,

Thank you for your work on this.

Some high level comments first; the way ModuleDependencyCollector is getting hooked up, results in:

- If no module needs building (because module cache has it) no module dependency is getting written out
- It doesn’t hook in the top level module, e.g. if I have "@import Darwin;” then no module dependency is written out even if the module cache is empty.


Some nitpicks:

@@ -105,6 +105,9 @@ class CompilerInstance : public ModuleLoader {
   /// \brief The ASTReader, if one exists.
   IntrusiveRefCntPtr<ASTReader> ModuleManager;
 
+  /// \brief The module dependency collector for crashdumps
+  std::shared_ptr<ModuleDependencyCollector> ModuleDepCollector;
+
   /// \brief The dependency file generator.
   std::unique_ptr<DependencyFileGenerator> TheDependencyFileGenerator;

Is there a policy to avoid IntrusiveRefCntPtr ?


+/// Collects the dependencies for imported modules into a directory.  Users
+/// should attach to the AST reader whenever a module is loaded.
+class ModuleDependencyCollector {
+  StringRef DestDir;

Use std::string here.


+  void reportError() { HasErrors = true; }

Name is a bit misleading, how about "setHasErrors()” ?


+llvm::error_code ModuleDependencyListener::copyToRoot(StringRef Src) {
+  using namespace llvm::sys;
+  SmallString<256> Dest = Collector.getDest();
+  path::append(Dest, path::relative_path(Src));

Do we need to turn Src into an absolute path first, in case it’s relative to the current directory ?


+  if (llvm::error_code EC = fs::copy_file(Src, Dest.str()))
+    return EC;

Do you have a separate patch that introduces copy_file ?




> On Jun 9, 2014, at 8:07 PM, Justin Bogner <mail at justinbogner.com> wrote:
> 
> This is lead up to making crash reports more useful when modules are
> being used. Since module imports are left unchanged by the preprocessor,
> these reports aren't really useful for reproducing problems today.
> 
> The attached patches add a -cc1 flag to dump module dependencies to a
> directory, and then to create a VFS mapping file so that these headers
> can be used with -ivfsoverlay. Later, I plan on hooking the flag up in
> generateCompilationDiagnostics.
> 
> No tests included. I'll spin some up before I commit, but I thought I
> might as well send this to be reviewed in the mean time.
> 
> <0001-Frontend-Add-a-CC1-flag-to-dump-module-dependencies-.patch><0002-Frontend-Include-a-VFS-map-when-dumping-module-depen.patch>_______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits





More information about the cfe-commits mailing list