[PATCH] D111724: [clang][deps] NFC: Remove redundant CompilerInstance reference

[Jan Svoboda via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG24616664afd4: [clang][deps] NFC: Remove redundant 
CompilerInstance reference (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111724

Files:
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp


Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -138,11 +138,11 @@
   // This has to be delayed as the context hash can change at the start of
   // `CompilerInstance::ExecuteAction`.
   if (MDC.ContextHash.empty()) {
-MDC.ContextHash = Instance.getInvocation().getModuleHash();
+MDC.ContextHash = MDC.Instance.getInvocation().getModuleHash();
 MDC.Consumer.handleContextHash(MDC.ContextHash);
   }
 
-  SourceManager &SM = Instance.getSourceManager();
+  SourceManager &SM = MDC.Instance.getSourceManager();
 
   // Dependency generation really does want to go all the way to the
   // file entry for a source location to find out what is depended on.
@@ -185,12 +185,13 @@
 }
 
 void ModuleDepCollectorPP::EndOfMainFile() {
-  FileID MainFileID = Instance.getSourceManager().getMainFileID();
+  FileID MainFileID = MDC.Instance.getSourceManager().getMainFileID();
   MDC.MainFile = std::string(
-  Instance.getSourceManager().getFileEntryForID(MainFileID)->getName());
+  
MDC.Instance.getSourceManager().getFileEntryForID(MainFileID)->getName());
 
-  if (!Instance.getPreprocessorOpts().ImplicitPCHInclude.empty())
-MDC.FileDeps.push_back(Instance.getPreprocessorOpts().ImplicitPCHInclude);
+  if (!MDC.Instance.getPreprocessorOpts().ImplicitPCHInclude.empty())
+MDC.FileDeps.push_back(
+MDC.Instance.getPreprocessorOpts().ImplicitPCHInclude);
 
   for (const Module *M : DirectModularDeps) {
 // A top-level module might not be actually imported as a module when
@@ -229,7 +230,7 @@
   MD.ImplicitModulePCMPath = std::string(M->getASTFile()->getName());
   MD.IsSystem = M->IsSystem;
 
-  const FileEntry *ModuleMap = Instance.getPreprocessor()
+  const FileEntry *ModuleMap = MDC.Instance.getPreprocessor()
.getHeaderSearchInfo()
.getModuleMap()
.getModuleMapFileForUniquing(M);
@@ -319,7 +320,7 @@
   OriginalInvocation(std::move(OriginalCI)), OptimizeArgs(OptimizeArgs) {}
 
 void ModuleDepCollector::attachToPreprocessor(Preprocessor &PP) {
-  PP.addPPCallbacks(std::make_unique(Instance, *this));
+  PP.addPPCallbacks(std::make_unique(*this));
 }
 
 void ModuleDepCollector::attachToASTReader(ASTReader &R) {}
Index: clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
===
--- clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -141,8 +141,7 @@
 /// \c DependencyConsumer of the parent \c ModuleDepCollector.
 class ModuleDepCollectorPP final : public PPCallbacks {
 public:
-  ModuleDepCollectorPP(CompilerInstance &I, ModuleDepCollector &MDC)
-  : Instance(I), MDC(MDC) {}
+  ModuleDepCollectorPP(ModuleDepCollector &MDC) : MDC(MDC) {}
 
   void FileChanged(SourceLocation Loc, FileChangeReason Reason,
SrcMgr::CharacteristicKind FileType,
@@ -159,8 +158,6 @@
   void EndOfMainFile() override;
 
 private:
-  /// The compiler instance for the current translation unit.
-  CompilerInstance &Instance;
   /// The parent dependency collector.
   ModuleDepCollector &MDC;
   /// Working set of direct modular dependencies.


Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -138,11 +138,11 @@
   // This has to be delayed as the context hash can change at the start of
   // `CompilerInstance::ExecuteAction`.
   if (MDC.ContextHash.empty()) {
-MDC.ContextHash = Instance.getInvocation().getModuleHash();
+MDC.ContextHash = MDC.Instance.getInvocation().getModuleHash();
 MDC.Consumer.handleContextHash(MDC.ContextHash);
   }
 
-  SourceManager &SM = Instance.getSourceManager();
+  SourceManager &SM = MDC.Instance.getSourceManager();
 
   // Dependency generation really does want to go all the way to the
   // file entry for a source location to find out what is depended on.
@@ -185,12 +185,13 @@
 }
 
 void ModuleDepCollectorPP::EndOfMainFile() {
-  FileID MainFileID = Instance.getSourceManager().getMainFileI

[PATCH] D111724: [clang][deps] NFC: Remove redundant CompilerInstance reference

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111724

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111724: [clang][deps] NFC: Remove redundant CompilerInstance reference

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The `ModuleDepCollectorPP` class holds a reference to `ModuleDepCollector` as 
well as `ModuleDepCollector`'s `CompilerInstance`. The fact that these refer to 
the same object is non-obvious.

This patch removes the `CompilerInvocation` reference from 
`ModuleDepCollectorPP` and accesses it through `ModuleDepCollector` instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111724

Files:
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp


Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -133,11 +133,11 @@
   // This has to be delayed as the context hash can change at the start of
   // `CompilerInstance::ExecuteAction`.
   if (MDC.ContextHash.empty()) {
-MDC.ContextHash = Instance.getInvocation().getModuleHash();
+MDC.ContextHash = MDC.Instance.getInvocation().getModuleHash();
 MDC.Consumer.handleContextHash(MDC.ContextHash);
   }
 
-  SourceManager &SM = Instance.getSourceManager();
+  SourceManager &SM = MDC.Instance.getSourceManager();
 
   // Dependency generation really does want to go all the way to the
   // file entry for a source location to find out what is depended on.
@@ -180,12 +180,13 @@
 }
 
 void ModuleDepCollectorPP::EndOfMainFile() {
-  FileID MainFileID = Instance.getSourceManager().getMainFileID();
+  FileID MainFileID = MDC.Instance.getSourceManager().getMainFileID();
   MDC.MainFile = std::string(
-  Instance.getSourceManager().getFileEntryForID(MainFileID)->getName());
+  
MDC.Instance.getSourceManager().getFileEntryForID(MainFileID)->getName());
 
-  if (!Instance.getPreprocessorOpts().ImplicitPCHInclude.empty())
-MDC.FileDeps.push_back(Instance.getPreprocessorOpts().ImplicitPCHInclude);
+  if (!MDC.Instance.getPreprocessorOpts().ImplicitPCHInclude.empty())
+MDC.FileDeps.push_back(
+MDC.Instance.getPreprocessorOpts().ImplicitPCHInclude);
 
   for (const Module *M : DirectModularDeps) {
 // A top-level module might not be actually imported as a module when
@@ -224,7 +225,7 @@
   MD.ImplicitModulePCMPath = std::string(M->getASTFile()->getName());
   MD.IsSystem = M->IsSystem;
 
-  const FileEntry *ModuleMap = Instance.getPreprocessor()
+  const FileEntry *ModuleMap = MDC.Instance.getPreprocessor()
.getHeaderSearchInfo()
.getModuleMap()
.getModuleMapFileForUniquing(M);
@@ -314,7 +315,7 @@
   OriginalInvocation(std::move(OriginalCI)), OptimizeArgs(OptimizeArgs) {}
 
 void ModuleDepCollector::attachToPreprocessor(Preprocessor &PP) {
-  PP.addPPCallbacks(std::make_unique(Instance, *this));
+  PP.addPPCallbacks(std::make_unique(*this));
 }
 
 void ModuleDepCollector::attachToASTReader(ASTReader &R) {}
Index: clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
===
--- clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -141,8 +141,7 @@
 /// \c DependencyConsumer of the parent \c ModuleDepCollector.
 class ModuleDepCollectorPP final : public PPCallbacks {
 public:
-  ModuleDepCollectorPP(CompilerInstance &I, ModuleDepCollector &MDC)
-  : Instance(I), MDC(MDC) {}
+  ModuleDepCollectorPP(ModuleDepCollector &MDC) : MDC(MDC) {}
 
   void FileChanged(SourceLocation Loc, FileChangeReason Reason,
SrcMgr::CharacteristicKind FileType,
@@ -159,8 +158,6 @@
   void EndOfMainFile() override;
 
 private:
-  /// The compiler instance for the current translation unit.
-  CompilerInstance &Instance;
   /// The parent dependency collector.
   ModuleDepCollector &MDC;
   /// Working set of direct modular dependencies.


Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -133,11 +133,11 @@
   // This has to be delayed as the context hash can change at the start of
   // `CompilerInstance::ExecuteAction`.
   if (MDC.ContextHash.empty()) {
-MDC.ContextHash = Instance.getInvocation().getModuleHash();
+MDC.ContextHash = MDC.Instance.getInvocation().getModuleHash();
 MDC.Consumer.handleContextHash(MDC.ContextHash);
   }
 
-  SourceManager &SM = Instance.getSourceManager();
+  SourceManager &SM = MDC.Instance.getSourceManager();
 
   /