Add module dependencies with -MD, -MMD

[Argyrios Kyrtzidis]

@@ -2014,13 +2074,17 @@ ASTReader::ReadControlBlock(ModuleFile &F,
        // files.
        unsigned NumInputs = Record[0];
        unsigned NumUserInputs = Record[1];
-        unsigned N = ValidateSystemInputs ||
-                             (HSOpts.ModulesValidateOncePerBuildSession &&
-                              F.Kind == MK_Module)
-                         ? NumInputs
-                         : NumUserInputs;
+        unsigned N = NumUserInputs;
+        if (ValidateSystemInputs ||
+            (Listener && Listener->needsInputFileVisitation()) ||
+            (HSOpts.ModulesValidateOncePerBuildSession && F.Kind == MK_Module))
+          N = NumInputs;
+
        for (unsigned I = 0; I < N; ++I) {
          InputFile IF = getInputFile(F, I+1, Complain);
+          if (const FileEntry *F = IF.getFile())
+            Listener->visitInputFile(F->getName(), I >= NumUserInputs);
          if (!IF.getFile() || IF.isOutOfDate())
            return OutOfDate;
        }

The code here combines validation with reporting the dependencies and things start getting problematic..

One issue is that earlier there is this check:

     if (!DisableValidation &&
         (!HSOpts.ModulesValidateOncePerBuildSession ||
          F.InputFilesValidationTimestamp <= HSOpts.BuildSessionTimestamp)) {

so no dependencies are going to get reported if we are skipping validation.

Another issue is that even if we don't need to report the system dependencies we still go through all the system headers and stat them to get an InputFile to pass to the Listener.

I'd suggest separating the validation block from the "reporting dependencies".
Also, inside ASTReader::getInputFile() there is the code to get the file path string from the record ID; factor this out into a separate function and use that to go through and report the dependencies (to avoid stat'ing).
I believe we always store absolute paths in the module files so this should work for reporting the dependencies.


+++ b/test/Modules/dependency-gen.m
@@ -0,0 +1,20 @@
+// RUN: rm -rf %t-mcp
+// RUN: mkdir -p %t-mcp
+
+// RUN: %clang_cc1 -x objective-c -dependency-file %t.d.1 -MT %s.o -I %S/Inputs -fsyntax-only -fmodules -fmodules-cache-path=%t-mcp %s
+// RUN: FileCheck %s < %t.d.1
+// CHECK: dependency-gen.m
+// CHECK: Inputs/diamond_top.h
+// CHECK: Inputs/module.map
+// CHECK-NOT: string.h
+
+
+// RUN: %clang_cc1 -x objective-c -dependency-file %t.d.2 -MT %s.o -I %S/Inputs -sys-header-deps -fsyntax-only -fmodules -fmodules-cache-path=%t-mcp %s
+// RUN: FileCheck %s -check-prefix=CHECK-SYS < %t.d.2
+// CHECK-SYS: dependency-gen.m
+// CHECK-SYS: Inputs/diamond_top.h
+// CHECK-SYS: Inputs/module.map
+// CHECK-SYS: string.h
+
+#import "diamond_top.h"
+#import "string.h"

Is this importing 'string.h' from the system ?


On Mar 4, 2014, at 9:37 PM, Ben Langmuir <blangmuir at apple.com> wrote:

> Annnnnd I finally got around to updating this patch!
> 
> The new patch is simplified to rely on the ASTWriter’s concept of what a ‘system’ dependency is.  To that end, I’ve changed module.map files to no longer be unconditionally ‘user’ files.  Previously we forced module.map to be ‘user’ input files as a cheap way to detect system header changes without ‘stat’ing all the headers.  With Dmitri’s work on a timestamp-file-based approach, we shouldn’t need that trick anymore.
> 
> Ben
> 
> <print-module-deps.patch>
> On Jan 22, 2014, at 4:40 PM, Ben Langmuir <blangmuir at apple.com> wrote:
> 
>> This patch adds module dependencies to the dependency files created by -MD/-MMD/etc.  It does so by attaching an ASTReaderListener that will call into the dependency file generator when a module input file is seen in the serialized AST.  In order to add/not add system headers appropriately, the IsSystem flag from the clang::Module is passed through the ASTReader and ModuleFile.  There was also an addition to allow chaining ASTReaderListeners (let me know if you’d prefer that to be a separate patch).
>> 
>> Ben
>> 
>> <module-deps.patch>_______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>