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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
