[PATCH] D63290: Unify DependencyFileGenerator class and DependencyCollector interface
This revision was automatically updated to reflect the committed changes.
Closed by commit rL363840: Unify DependencyFileGenerator class and
DependencyCollector interface (NFCI) (authored by arphaman, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D63290?vs=205438=205628#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63290/new/
https://reviews.llvm.org/D63290
Files:
cfe/trunk/include/clang/Frontend/CompilerInstance.h
cfe/trunk/include/clang/Frontend/Utils.h
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/Frontend/DependencyFile.cpp
Index: cfe/trunk/include/clang/Frontend/Utils.h
===
--- cfe/trunk/include/clang/Frontend/Utils.h
+++ cfe/trunk/include/clang/Frontend/Utils.h
@@ -15,6 +15,7 @@
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/LLVM.h"
+#include "clang/Frontend/DependencyOutputOptions.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/ADT/StringMap.h"
@@ -46,7 +47,6 @@
class ASTReader;
class CompilerInstance;
class CompilerInvocation;
-class DependencyOutputOptions;
class DiagnosticsEngine;
class ExternalSemaSource;
class FrontendOptions;
@@ -77,8 +77,7 @@
/// An interface for collecting the dependencies of a compilation. Users should
/// use \c attachToPreprocessor and \c attachToASTReader to get all of the
/// dependencies.
-/// FIXME: Migrate DependencyFileGen and DependencyGraphGen to use this
-/// interface.
+/// FIXME: Migrate DependencyGraphGen to use this interface.
class DependencyCollector {
public:
virtual ~DependencyCollector();
@@ -95,7 +94,7 @@
bool IsSystem, bool IsModuleFile, bool IsMissing);
/// Called when the end of the main file is reached.
- virtual void finishedMainFile() {}
+ virtual void finishedMainFile(DiagnosticsEngine ) {}
/// Return true if system files should be passed to sawDependency().
virtual bool needSystemDependencies() { return false; }
@@ -106,25 +105,45 @@
void maybeAddDependency(StringRef Filename, bool FromModule, bool IsSystem,
bool IsModuleFile, bool IsMissing);
+protected:
+ /// Return true if the filename was added to the list of dependencies, false
+ /// otherwise.
+ bool addDependency(StringRef Filename);
+
private:
llvm::StringSet<> Seen;
std::vector Dependencies;
};
-/// Builds a depdenency file when attached to a Preprocessor (for includes) and
+/// Builds a dependency file when attached to a Preprocessor (for includes) and
/// ASTReader (for module imports), and writes it out at the end of processing
/// a source file. Users should attach to the ast reader whenever a module is
/// loaded.
-class DependencyFileGenerator {
- void *Impl; // Opaque implementation
+class DependencyFileGenerator : public DependencyCollector {
+public:
+ DependencyFileGenerator(const DependencyOutputOptions );
- DependencyFileGenerator(void *Impl);
+ void attachToPreprocessor(Preprocessor ) override;
-public:
- static DependencyFileGenerator *CreateAndAttachToPreprocessor(
-Preprocessor , const DependencyOutputOptions );
+ void finishedMainFile(DiagnosticsEngine ) override;
+
+ bool needSystemDependencies() final override { return IncludeSystemHeaders; }
+
+ bool sawDependency(StringRef Filename, bool FromModule, bool IsSystem,
+ bool IsModuleFile, bool IsMissing) final override;
+
+private:
+ void outputDependencyFile(DiagnosticsEngine );
- void AttachToASTReader(ASTReader );
+ std::string OutputFile;
+ std::vector Targets;
+ bool IncludeSystemHeaders;
+ bool PhonyTarget;
+ bool AddMissingHeaderDeps;
+ bool SeenMissingHeader;
+ bool IncludeModuleFiles;
+ DependencyOutputFormat OutputFormat;
+ unsigned InputFileIndex;
};
/// Collects the dependencies for imported modules into a directory. Users
Index: cfe/trunk/include/clang/Frontend/CompilerInstance.h
===
--- cfe/trunk/include/clang/Frontend/CompilerInstance.h
+++ cfe/trunk/include/clang/Frontend/CompilerInstance.h
@@ -124,9 +124,6 @@
/// The module provider.
std::shared_ptr ThePCHContainerOperations;
- /// The dependency file generator.
- std::unique_ptr TheDependencyFileGenerator;
-
std::vector> DependencyCollectors;
/// The set of top-level modules that has already been loaded,
@@ -661,7 +658,6 @@
InMemoryModuleCache , ASTContext ,
const PCHContainerReader ,
ArrayRef> Extensions,
- DependencyFileGenerator *DependencyFile,
ArrayRef> DependencyCollectors,
void *DeserializationListener, bool OwnDeserializationListener,
bool Preamble, bool UseGlobalModuleIndex);
Index: cfe/trunk/lib/Frontend/DependencyFile.cpp
[PATCH] D63290: Unify DependencyFileGenerator class and DependencyCollector interface
aganea accepted this revision. aganea added a comment. This revision is now accepted and ready to land. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63290/new/ https://reviews.llvm.org/D63290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D63290: Unify DependencyFileGenerator class and DependencyCollector interface
arphaman updated this revision to Diff 205438.
arphaman marked 3 inline comments as done.
arphaman added a comment.
Address review comments.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63290/new/
https://reviews.llvm.org/D63290
Files:
clang/include/clang/Frontend/CompilerInstance.h
clang/include/clang/Frontend/Utils.h
clang/lib/Frontend/CompilerInstance.cpp
clang/lib/Frontend/DependencyFile.cpp
Index: clang/lib/Frontend/DependencyFile.cpp
===
--- clang/lib/Frontend/DependencyFile.cpp
+++ clang/lib/Frontend/DependencyFile.cpp
@@ -32,8 +32,10 @@
struct DepCollectorPPCallbacks : public PPCallbacks {
DependencyCollector
SourceManager
- DepCollectorPPCallbacks(DependencyCollector , SourceManager )
- : DepCollector(L), SM(SM) { }
+ DiagnosticsEngine
+ DepCollectorPPCallbacks(DependencyCollector , SourceManager ,
+ DiagnosticsEngine )
+ : DepCollector(L), SM(SM), Diags(Diags) {}
void FileChanged(SourceLocation Loc, FileChangeReason Reason,
SrcMgr::CharacteristicKind FileType,
@@ -57,6 +59,16 @@
/*IsModuleFile*/false, /*IsMissing*/false);
}
+ void FileSkipped(const FileEntry , const Token ,
+ SrcMgr::CharacteristicKind FileType) override {
+StringRef Filename =
+llvm::sys::path::remove_leading_dotslash(SkippedFile.getName());
+DepCollector.maybeAddDependency(Filename, /*FromModule=*/false,
+/*IsSystem=*/isSystem(FileType),
+/*IsModuleFile=*/false,
+/*IsMissing=*/false);
+ }
+
void InclusionDirective(SourceLocation HashLoc, const Token ,
StringRef FileName, bool IsAngled,
CharSourceRange FilenameRange, const FileEntry *File,
@@ -70,9 +82,20 @@
// Files that actually exist are handled by FileChanged.
}
- void EndOfMainFile() override {
-DepCollector.finishedMainFile();
+ void HasInclude(SourceLocation Loc, StringRef SpelledFilename, bool IsAngled,
+ const FileEntry *File,
+ SrcMgr::CharacteristicKind FileType) override {
+if (!File)
+ return;
+StringRef Filename =
+llvm::sys::path::remove_leading_dotslash(File->getName());
+DepCollector.maybeAddDependency(Filename, /*FromModule=*/false,
+/*IsSystem=*/isSystem(FileType),
+/*IsModuleFile=*/false,
+/*IsMissing=*/false);
}
+
+ void EndOfMainFile() override { DepCollector.finishedMainFile(Diags); }
};
struct DepCollectorMMCallbacks : public ModuleMapCallbacks {
@@ -117,9 +140,16 @@
void DependencyCollector::maybeAddDependency(StringRef Filename, bool FromModule,
bool IsSystem, bool IsModuleFile,
bool IsMissing) {
- if (Seen.insert(Filename).second &&
- sawDependency(Filename, FromModule, IsSystem, IsModuleFile, IsMissing))
+ if (sawDependency(Filename, FromModule, IsSystem, IsModuleFile, IsMissing))
+addDependency(Filename);
+}
+
+bool DependencyCollector::addDependency(StringRef Filename) {
+ if (Seen.insert(Filename).second) {
Dependencies.push_back(Filename);
+return true;
+ }
+ return false;
}
static bool isSpecialFilename(StringRef Filename) {
@@ -138,8 +168,8 @@
DependencyCollector::~DependencyCollector() { }
void DependencyCollector::attachToPreprocessor(Preprocessor ) {
- PP.addPPCallbacks(
- llvm::make_unique(*this, PP.getSourceManager()));
+ PP.addPPCallbacks(llvm::make_unique(
+ *this, PP.getSourceManager(), PP.getDiagnostics()));
PP.getHeaderSearchInfo().getModuleMap().addModuleMapCallbacks(
llvm::make_unique(*this));
}
@@ -147,206 +177,57 @@
R.addListener(llvm::make_unique(*this));
}
-namespace {
-/// Private implementation for DependencyFileGenerator
-class DFGImpl : public PPCallbacks {
- std::vector Files;
- llvm::StringSet<> FilesSet;
- const Preprocessor *PP;
- std::string OutputFile;
- std::vector Targets;
- bool IncludeSystemHeaders;
- bool PhonyTarget;
- bool AddMissingHeaderDeps;
- bool SeenMissingHeader;
- bool IncludeModuleFiles;
- DependencyOutputFormat OutputFormat;
- unsigned InputFileIndex;
-
-private:
- bool FileMatchesDepCriteria(const char *Filename,
- SrcMgr::CharacteristicKind FileType);
- void OutputDependencyFile();
-
-public:
- DFGImpl(const Preprocessor *_PP, const DependencyOutputOptions )
-: PP(_PP), OutputFile(Opts.OutputFile), Targets(Opts.Targets),
+DependencyFileGenerator::DependencyFileGenerator(
+const DependencyOutputOptions )
+: OutputFile(Opts.OutputFile), Targets(Opts.Targets),
IncludeSystemHeaders(Opts.IncludeSystemHeaders),
[PATCH] D63290: Unify DependencyFileGenerator class and DependencyCollector interface
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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D63290: Unify DependencyFileGenerator class and DependencyCollector interface
arphaman updated this revision to Diff 204611.
arphaman added a comment.
Fix the missing word in the comment.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63290/new/
https://reviews.llvm.org/D63290
Files:
clang/include/clang/Frontend/CompilerInstance.h
clang/include/clang/Frontend/Utils.h
clang/lib/Frontend/CompilerInstance.cpp
clang/lib/Frontend/DependencyFile.cpp
Index: clang/lib/Frontend/DependencyFile.cpp
===
--- clang/lib/Frontend/DependencyFile.cpp
+++ clang/lib/Frontend/DependencyFile.cpp
@@ -32,8 +32,10 @@
struct DepCollectorPPCallbacks : public PPCallbacks {
DependencyCollector
SourceManager
- DepCollectorPPCallbacks(DependencyCollector , SourceManager )
- : DepCollector(L), SM(SM) { }
+ DiagnosticsEngine
+ DepCollectorPPCallbacks(DependencyCollector , SourceManager ,
+ DiagnosticsEngine )
+ : DepCollector(L), SM(SM), Diags(Diags) {}
void FileChanged(SourceLocation Loc, FileChangeReason Reason,
SrcMgr::CharacteristicKind FileType,
@@ -57,6 +59,16 @@
/*IsModuleFile*/false, /*IsMissing*/false);
}
+ void FileSkipped(const FileEntry , const Token ,
+ SrcMgr::CharacteristicKind FileType) override {
+StringRef Filename =
+llvm::sys::path::remove_leading_dotslash(SkippedFile.getName());
+DepCollector.maybeAddDependency(Filename, /*FromModule=*/false,
+/*IsSystem=*/isSystem(FileType),
+/*IsModuleFile=*/false,
+/*IsMissing=*/false);
+ }
+
void InclusionDirective(SourceLocation HashLoc, const Token ,
StringRef FileName, bool IsAngled,
CharSourceRange FilenameRange, const FileEntry *File,
@@ -70,9 +82,20 @@
// Files that actually exist are handled by FileChanged.
}
- void EndOfMainFile() override {
-DepCollector.finishedMainFile();
+ void HasInclude(SourceLocation Loc, StringRef SpelledFilename, bool IsAngled,
+ const FileEntry *File,
+ SrcMgr::CharacteristicKind FileType) override {
+if (!File)
+ return;
+StringRef Filename =
+llvm::sys::path::remove_leading_dotslash(File->getName());
+DepCollector.maybeAddDependency(Filename, /*FromModule=*/false,
+/*IsSystem=*/isSystem(FileType),
+/*IsModuleFile=*/false,
+/*IsMissing=*/false);
}
+
+ void EndOfMainFile() override { DepCollector.finishedMainFile(Diags); }
};
struct DepCollectorMMCallbacks : public ModuleMapCallbacks {
@@ -117,9 +140,16 @@
void DependencyCollector::maybeAddDependency(StringRef Filename, bool FromModule,
bool IsSystem, bool IsModuleFile,
bool IsMissing) {
- if (Seen.insert(Filename).second &&
- sawDependency(Filename, FromModule, IsSystem, IsModuleFile, IsMissing))
+ if (sawDependency(Filename, FromModule, IsSystem, IsModuleFile, IsMissing))
+addDependency(Filename);
+}
+
+bool DependencyCollector::addDependency(StringRef Filename) {
+ if (Seen.insert(Filename).second) {
Dependencies.push_back(Filename);
+return true;
+ }
+ return false;
}
static bool isSpecialFilename(StringRef Filename) {
@@ -132,14 +162,14 @@
bool DependencyCollector::sawDependency(StringRef Filename, bool FromModule,
bool IsSystem, bool IsModuleFile,
bool IsMissing) {
- return !isSpecialFilename(Filename) &&
+ return !IsMissing && !isSpecialFilename(Filename) &&
(needSystemDependencies() || !IsSystem);
}
DependencyCollector::~DependencyCollector() { }
void DependencyCollector::attachToPreprocessor(Preprocessor ) {
- PP.addPPCallbacks(
- llvm::make_unique(*this, PP.getSourceManager()));
+ PP.addPPCallbacks(llvm::make_unique(
+ *this, PP.getSourceManager(), PP.getDiagnostics()));
PP.getHeaderSearchInfo().getModuleMap().addModuleMapCallbacks(
llvm::make_unique(*this));
}
@@ -147,206 +177,57 @@
R.addListener(llvm::make_unique(*this));
}
-namespace {
-/// Private implementation for DependencyFileGenerator
-class DFGImpl : public PPCallbacks {
- std::vector Files;
- llvm::StringSet<> FilesSet;
- const Preprocessor *PP;
- std::string OutputFile;
- std::vector Targets;
- bool IncludeSystemHeaders;
- bool PhonyTarget;
- bool AddMissingHeaderDeps;
- bool SeenMissingHeader;
- bool IncludeModuleFiles;
- DependencyOutputFormat OutputFormat;
- unsigned InputFileIndex;
-
-private:
- bool FileMatchesDepCriteria(const char *Filename,
- SrcMgr::CharacteristicKind FileType);
- void OutputDependencyFile();
-
-public:
-
[PATCH] D63290: Unify DependencyFileGenerator class and DependencyCollector interface
arphaman created this revision.
arphaman added reviewers: Bigcheese, vsapsai, bruno, aganea.
Herald added subscribers: dexonsmith, jkorous.
Herald added a project: clang.
This NFCI patch makes DependencyFileGenerator a DependencyCollector as it was
intended when DependencyCollector was introduced. The missing PP overrides are
added to the DependencyCollector as well.
This change will allow `clang-scan-deps` to access the produced dependencies
without writing them out to `.d` files to disk, so that it will be able collate
them and report them to the user.
Repository:
rC Clang
https://reviews.llvm.org/D63290
Files:
clang/include/clang/Frontend/CompilerInstance.h
clang/include/clang/Frontend/Utils.h
clang/lib/Frontend/CompilerInstance.cpp
clang/lib/Frontend/DependencyFile.cpp
Index: clang/lib/Frontend/DependencyFile.cpp
===
--- clang/lib/Frontend/DependencyFile.cpp
+++ clang/lib/Frontend/DependencyFile.cpp
@@ -32,8 +32,10 @@
struct DepCollectorPPCallbacks : public PPCallbacks {
DependencyCollector
SourceManager
- DepCollectorPPCallbacks(DependencyCollector , SourceManager )
- : DepCollector(L), SM(SM) { }
+ DiagnosticsEngine
+ DepCollectorPPCallbacks(DependencyCollector , SourceManager ,
+ DiagnosticsEngine )
+ : DepCollector(L), SM(SM), Diags(Diags) {}
void FileChanged(SourceLocation Loc, FileChangeReason Reason,
SrcMgr::CharacteristicKind FileType,
@@ -57,6 +59,16 @@
/*IsModuleFile*/false, /*IsMissing*/false);
}
+ void FileSkipped(const FileEntry , const Token ,
+ SrcMgr::CharacteristicKind FileType) override {
+StringRef Filename =
+llvm::sys::path::remove_leading_dotslash(SkippedFile.getName());
+DepCollector.maybeAddDependency(Filename, /*FromModule=*/false,
+/*IsSystem=*/isSystem(FileType),
+/*IsModuleFile=*/false,
+/*IsMissing=*/false);
+ }
+
void InclusionDirective(SourceLocation HashLoc, const Token ,
StringRef FileName, bool IsAngled,
CharSourceRange FilenameRange, const FileEntry *File,
@@ -70,9 +82,20 @@
// Files that actually exist are handled by FileChanged.
}
- void EndOfMainFile() override {
-DepCollector.finishedMainFile();
+ void HasInclude(SourceLocation Loc, StringRef SpelledFilename, bool IsAngled,
+ const FileEntry *File,
+ SrcMgr::CharacteristicKind FileType) override {
+if (!File)
+ return;
+StringRef Filename =
+llvm::sys::path::remove_leading_dotslash(File->getName());
+DepCollector.maybeAddDependency(Filename, /*FromModule=*/false,
+/*IsSystem=*/isSystem(FileType),
+/*IsModuleFile=*/false,
+/*IsMissing=*/false);
}
+
+ void EndOfMainFile() override { DepCollector.finishedMainFile(Diags); }
};
struct DepCollectorMMCallbacks : public ModuleMapCallbacks {
@@ -117,9 +140,16 @@
void DependencyCollector::maybeAddDependency(StringRef Filename, bool FromModule,
bool IsSystem, bool IsModuleFile,
bool IsMissing) {
- if (Seen.insert(Filename).second &&
- sawDependency(Filename, FromModule, IsSystem, IsModuleFile, IsMissing))
+ if (sawDependency(Filename, FromModule, IsSystem, IsModuleFile, IsMissing))
+addDependency(Filename);
+}
+
+bool DependencyCollector::addDependency(StringRef Filename) {
+ if (Seen.insert(Filename).second) {
Dependencies.push_back(Filename);
+return true;
+ }
+ return false;
}
static bool isSpecialFilename(StringRef Filename) {
@@ -138,8 +168,8 @@
DependencyCollector::~DependencyCollector() { }
void DependencyCollector::attachToPreprocessor(Preprocessor ) {
- PP.addPPCallbacks(
- llvm::make_unique(*this, PP.getSourceManager()));
+ PP.addPPCallbacks(llvm::make_unique(
+ *this, PP.getSourceManager(), PP.getDiagnostics()));
PP.getHeaderSearchInfo().getModuleMap().addModuleMapCallbacks(
llvm::make_unique(*this));
}
@@ -147,206 +177,57 @@
R.addListener(llvm::make_unique(*this));
}
-namespace {
-/// Private implementation for DependencyFileGenerator
-class DFGImpl : public PPCallbacks {
- std::vector Files;
- llvm::StringSet<> FilesSet;
- const Preprocessor *PP;
- std::string OutputFile;
- std::vector Targets;
- bool IncludeSystemHeaders;
- bool PhonyTarget;
- bool AddMissingHeaderDeps;
- bool SeenMissingHeader;
- bool IncludeModuleFiles;
- DependencyOutputFormat OutputFormat;
- unsigned InputFileIndex;
-
-private:
- bool FileMatchesDepCriteria(const char *Filename,
- SrcMgr::CharacteristicKind FileType);
-
