[PATCH] modularize - header dependencies - version 2

[Sean Silva]

  Does this patch make sense from a design standpoint? Aren't these kinds of implicit dependencies the things that modularize is trying to get rid of in the first place? Why would we expend effort to enshrine them, when we can just detect them and emit an error?

  I've also made a few comments about the code, but I think the above design issue is more important.


================
Comment at: modularize/Modularize.cpp:281
@@ +280,3 @@
+      std::string File(
+        std::string("\"") + FileDependents[Index] + std::string("\""));
+      NewArgs.push_back(FileDependents[Index]);
----------------
Is the quoting here actually necessary? I don't think that there is a shell-escaping step after this.

================
Comment at: modularize/Modularize.cpp:292-294
@@ +291,5 @@
+    SmallVector<const char*, 256> Argv;
+    for (CommandLineArguments::const_iterator I = CLArgs.begin(),
+          E = CLArgs.end();
+        I != E; ++I)
+      Argv.push_back(I->c_str());
----------------
clang-format

================
Comment at: modularize/Modularize.cpp:287-302
@@ +286,18 @@
+  // Helper function for finding the input file in an arguments list.
+  llvm::StringRef findInputFile(const CommandLineArguments &CLArgs) {
+    OwningPtr<OptTable> Opts(createDriverOptTable());
+    const unsigned IncludedFlagsBitmask = options::CC1Option;
+    unsigned MissingArgIndex, MissingArgCount;
+    SmallVector<const char*, 256> Argv;
+    for (CommandLineArguments::const_iterator I = CLArgs.begin(),
+          E = CLArgs.end();
+        I != E; ++I)
+      Argv.push_back(I->c_str());
+    OwningPtr<InputArgList> Args(
+      Opts->ParseArgs(Argv.data(), Argv.data() + Argv.size(),
+                      MissingArgIndex, MissingArgCount,
+                      IncludedFlagsBitmask));
+    std::vector<std::string> Inputs = Args->getAllArgValues(OPT_INPUT);
+    return Inputs.back();
+  }
+  DependencyMap &Dependencies;
----------------
Does this function actually use any of the class members? It seems like it should be a free function.


http://llvm-reviews.chandlerc.com/D1474