[PATCH] D63290: Unify DependencyFileGenerator class and DependencyCollector interface

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a reviewer: benlangmuir.
aganea added inline comments.


================
Comment at: clang/include/clang/Frontend/Utils.h:118
 
 /// Builds a depdenency file when attached to a Preprocessor (for includes) and
 /// ASTReader (for module imports), and writes it out at the end of processing
----------------
s/depdenency/dependency/


================
Comment at: clang/lib/Frontend/DependencyFile.cpp:78
                           SrcMgr::CharacteristicKind FileType) override {
     if (!File)
       DepCollector.maybeAddDependency(FileName, /*FromModule*/false,
----------------
Given that `llvm::sys::path::remove_leading_dotslash` is not called here, but it is for the function above, could you end up with two entries in `DependencyCollector::Seen` when `DependencyCollector::sawDependency` is overriden?


================
Comment at: clang/lib/Frontend/DependencyFile.cpp:165
                                        bool IsMissing) {
-  return !isSpecialFilename(Filename) &&
+  return !IsMissing && !isSpecialFilename(Filename) &&
          (needSystemDependencies() || !IsSystem);
----------------
Why `!IsMissing` wasn't considered before? Just wondering.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63290/new/

https://reviews.llvm.org/D63290