Hello,

I've not touched the ARC code much yet (shock there ;] ) so let me know if
I'm missing anything.

Currently ARCMigrate has code that heavily depends on datastructures and
logic provided by the Frontend library (CompilerInstance, etc). This seems
reasonable, certainly the Rewrite library and CodeGen library also have such
a dependence. However, the Frontend library *also* depends on ARCMigrate
which is down right confusing. This isn't just with header file #includes,
the CMake files have each library depending on the other. Either way, seemed
like a problem and layering violation.

When I looked more closely, the Frontend was only barely using anything from
ARCMigrate, it just had special code to dispatch to magic routines there and
do the migration steps. This seemed a bit ad-hoc, and bypassed the entire
FrontendAction system that the other libraries use to managed the
dependencies: FrontendTools depends on everyone and builds their concrete
FrontendAction subclasses, everyone depends on Frontend in order to
implement their concrete FrontendAction subclasses. This patch moves the
ARCMigration code to the same pattern. It implements FrontendAction
subclasses for each of the tools, and those are selected from the options in
FrontendTools. That way FrontendTools is what depends on ARCMigrate,
breaking the cycle. I also think its cleaner design than the previous
version. Thoughts? This look like a reasonable approach?
diff --git a/include/clang/ARCMigrate/ARCMTActions.h b/include/clang/ARCMigrate/ARCMTActions.h
new file mode 100644
index 0000000..d2d9410
--- /dev/null
+++ b/include/clang/ARCMigrate/ARCMTActions.h
@@ -0,0 +1,46 @@
+//===--- ARCMTActions.h - ARC Migrate Tool Frontend Actions -----*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_ARCMIGRATE_ARCMT_ACTION_H
+#define LLVM_CLANG_ARCMIGRATE_ARCMT_ACTION_H
+
+#include "clang/Frontend/FrontendAction.h"
+#include "llvm/ADT/OwningPtr.h"
+
+namespace clang {
+namespace arcmt {
+
+class CheckAction : public WrapperFrontendAction {
+protected:
+  virtual void ExecuteAction();
+
+public:
+  CheckAction(FrontendAction *WrappedAction);
+};
+
+class TransformationAction : public WrapperFrontendAction {
+protected:
+  virtual void ExecuteAction();
+
+public:
+  TransformationAction(FrontendAction *WrappedAction);
+};
+
+class InMemoryTransformationAction : public WrapperFrontendAction {
+protected:
+  virtual void ExecuteAction();
+
+public:
+  InMemoryTransformationAction(FrontendAction *WrappedAction);
+};
+
+}
+}
+
+#endif
diff --git a/include/clang/Frontend/FrontendAction.h b/include/clang/Frontend/FrontendAction.h
index ee0863a..2538f55 100644
--- a/include/clang/Frontend/FrontendAction.h
+++ b/include/clang/Frontend/FrontendAction.h
@@ -51,6 +51,7 @@ class FrontendAction {
   llvm::OwningPtr<ASTUnit> CurrentASTUnit;
   CompilerInstance *Instance;
   friend class ASTMergeAction;
+  friend class WrapperFrontendAction;
 
 private:
   ASTConsumer* CreateWrappedASTConsumer(CompilerInstance &CI,
@@ -253,6 +254,35 @@ public:
   virtual bool usesPreprocessorOnly() const { return true; }
 };
 
+/// WrapperFrontendAction - A frontend action which simply wraps some other
+/// runtime specified frontend action. Deriving from this class allows an
+/// action to inject custom logic around some existing action's behavior. It
+/// implements every virtual method in the FrontendAction interface by
+/// forwarding to the wrapped action.
+class WrapperFrontendAction : public FrontendAction {
+  llvm::OwningPtr<FrontendAction> WrappedAction;
+
+protected:
+  virtual ASTConsumer *CreateASTConsumer(CompilerInstance &CI,
+                                         llvm::StringRef InFile);
+  virtual bool BeginSourceFileAction(CompilerInstance &CI,
+                                     llvm::StringRef Filename);
+  virtual void ExecuteAction();
+  virtual void EndSourceFileAction();
+
+public:
+  /// Construct a WrapperFrontendAction from an existing action, taking
+  /// ownership of it.
+  WrapperFrontendAction(FrontendAction *WrappedAction);
+
+  virtual bool usesPreprocessorOnly() const;
+  virtual bool usesCompleteTranslationUnit();
+  virtual bool hasPCHSupport() const;
+  virtual bool hasASTFileSupport() const;
+  virtual bool hasIRSupport() const;
+  virtual bool hasCodeCompletionSupport() const;
+};
+
 }  // end namespace clang
 
 #endif
diff --git a/lib/ARCMigrate/ARCMTActions.cpp b/lib/ARCMigrate/ARCMTActions.cpp
new file mode 100644
index 0000000..a30c278
--- /dev/null
+++ b/lib/ARCMigrate/ARCMTActions.cpp
@@ -0,0 +1,58 @@
+//===--- ARCMTActions.cpp - ARC Migrate Tool Frontend Actions ---*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/ARCMigrate/ARCMTActions.h"
+#include "clang/ARCMigrate/ARCMT.h"
+#include "clang/Frontend/CompilerInstance.h"
+
+using namespace clang;
+using namespace arcmt;
+
+void CheckAction::ExecuteAction() {
+  CompilerInstance &CI = getCompilerInstance();
+  if (arcmt::checkForManualIssues(CI.getInvocation(), getCurrentFile(),
+                                  getCurrentFileKind(),
+                                  CI.getDiagnostics().getClient()))
+    return;
+
+  // We only want to see warnings reported from arcmt::checkForManualIssues.
+  CI.getDiagnostics().setIgnoreAllWarnings(true);
+  WrapperFrontendAction::ExecuteAction();
+}
+
+CheckAction::CheckAction(FrontendAction *WrappedAction)
+  : WrapperFrontendAction(WrappedAction) {}
+
+void TransformationAction::ExecuteAction() {
+  CompilerInstance &CI = getCompilerInstance();
+  if (arcmt::applyTransformations(CI.getInvocation(), getCurrentFile(),
+                                  getCurrentFileKind(),
+                                  CI.getDiagnostics().getClient()))
+    return;
+
+  WrapperFrontendAction::ExecuteAction();
+}
+
+TransformationAction::TransformationAction(FrontendAction *WrappedAction)
+  : WrapperFrontendAction(WrappedAction) {}
+
+void InMemoryTransformationAction::ExecuteAction() {
+  CompilerInstance &CI = getCompilerInstance();
+  if (arcmt::applyTransformationsInMemory(CI.getInvocation(), getCurrentFile(),
+                                          getCurrentFileKind(),
+                                          CI.getDiagnostics().getClient()))
+    return;
+
+  WrapperFrontendAction::ExecuteAction();
+}
+
+InMemoryTransformationAction::InMemoryTransformationAction(
+    FrontendAction *WrappedAction)
+  : WrapperFrontendAction(WrappedAction) {}
+
diff --git a/lib/ARCMigrate/CMakeLists.txt b/lib/ARCMigrate/CMakeLists.txt
index 3645a22..0049d2f 100644
--- a/lib/ARCMigrate/CMakeLists.txt
+++ b/lib/ARCMigrate/CMakeLists.txt
@@ -2,6 +2,7 @@ set(LLVM_USED_LIBS clangBasic clangAST clangParse clangFrontend clangRewrite)
 
 add_clang_library(clangARCMigrate
   ARCMT.cpp
+  ARCMTActions.cpp
   FileRemapper.cpp
   TransformActions.cpp
   Transforms.cpp
diff --git a/lib/Frontend/CMakeLists.txt b/lib/Frontend/CMakeLists.txt
index 5565b7b..7dcbebf 100644
--- a/lib/Frontend/CMakeLists.txt
+++ b/lib/Frontend/CMakeLists.txt
@@ -1,5 +1,4 @@
 set( LLVM_USED_LIBS
-  clangARCMigrate
   clangAST
   clangBasic
   clangDriver
diff --git a/lib/Frontend/CompilerInstance.cpp b/lib/Frontend/CompilerInstance.cpp
index b05c24a..38fcfe3 100644
--- a/lib/Frontend/CompilerInstance.cpp
+++ b/lib/Frontend/CompilerInstance.cpp
@@ -26,7 +26,6 @@
 #include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Frontend/VerifyDiagnosticsClient.h"
 #include "clang/Frontend/Utils.h"
-#include "clang/ARCMigrate/ARCMT.h"
 #include "clang/Serialization/ASTReader.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "llvm/Support/FileSystem.h"
@@ -597,34 +596,6 @@ bool CompilerInstance::ExecuteAction(FrontendAction &Act) {
     if (hasSourceManager())
       getSourceManager().clearIDTables();
 
-    switch (getFrontendOpts().ARCMTAction) {
-    default:
-      break;
-
-    case FrontendOptions::ARCMT_Check:
-      if (arcmt::checkForManualIssues(getInvocation(), InFile,
-                                      getFrontendOpts().Inputs[i].first,
-                                      getDiagnostics().getClient()))
-        continue;
-      // We only want to see warnings reported from arcmt::checkForManualIssues.
-      getDiagnostics().setIgnoreAllWarnings(true);
-      break;
-
-    case FrontendOptions::ARCMT_Modify:
-      if (arcmt::applyTransformations(getInvocation(), InFile,
-                                      getFrontendOpts().Inputs[i].first,
-                                      getDiagnostics().getClient()))
-        continue;
-      break;
-
-    case FrontendOptions::ARCMT_ModifyInMemory:
-      if (arcmt::applyTransformationsInMemory(getInvocation(), InFile,
-                                              getFrontendOpts().Inputs[i].first,
-                                              getDiagnostics().getClient()))
-        continue;
-      break;
-    }
-
     if (Act.BeginSourceFile(*this, InFile, getFrontendOpts().Inputs[i].first)) {
       Act.Execute();
       Act.EndSourceFile();
diff --git a/lib/Frontend/FrontendAction.cpp b/lib/Frontend/FrontendAction.cpp
index 42da44c..0024818 100644
--- a/lib/Frontend/FrontendAction.cpp
+++ b/lib/Frontend/FrontendAction.cpp
@@ -381,3 +381,41 @@ PreprocessorFrontendAction::CreateASTConsumer(CompilerInstance &CI,
                                               llvm::StringRef InFile) {
   llvm_unreachable("Invalid CreateASTConsumer on preprocessor action!");
 }
+
+ASTConsumer *WrapperFrontendAction::CreateASTConsumer(CompilerInstance &CI,
+                                                      llvm::StringRef InFile) {
+  return WrappedAction->CreateASTConsumer(CI, InFile);
+}
+bool WrapperFrontendAction::BeginSourceFileAction(CompilerInstance &CI,
+                                                  llvm::StringRef Filename) {
+  return WrappedAction->BeginSourceFileAction(CI, Filename);
+}
+void WrapperFrontendAction::ExecuteAction() {
+  WrappedAction->ExecuteAction();
+}
+void WrapperFrontendAction::EndSourceFileAction() {
+  WrappedAction->EndSourceFileAction();
+}
+
+bool WrapperFrontendAction::usesPreprocessorOnly() const {
+  return WrappedAction->usesPreprocessorOnly();
+}
+bool WrapperFrontendAction::usesCompleteTranslationUnit() {
+  return WrappedAction->usesCompleteTranslationUnit();
+}
+bool WrapperFrontendAction::hasPCHSupport() const {
+  return WrappedAction->hasPCHSupport();
+}
+bool WrapperFrontendAction::hasASTFileSupport() const {
+  return WrappedAction->hasASTFileSupport();
+}
+bool WrapperFrontendAction::hasIRSupport() const {
+  return WrappedAction->hasIRSupport();
+}
+bool WrapperFrontendAction::hasCodeCompletionSupport() const {
+  return WrappedAction->hasCodeCompletionSupport();
+}
+
+WrapperFrontendAction::WrapperFrontendAction(FrontendAction *WrappedAction)
+  : WrappedAction(WrappedAction) {}
+
diff --git a/lib/FrontendTool/CMakeLists.txt b/lib/FrontendTool/CMakeLists.txt
index 720ce2a..b8e4329 100644
--- a/lib/FrontendTool/CMakeLists.txt
+++ b/lib/FrontendTool/CMakeLists.txt
@@ -1,5 +1,6 @@
 set(LLVM_USED_LIBS clangDriver clangFrontend clangRewrite clangCodeGen 
-    clangStaticAnalyzerFrontend clangStaticAnalyzerCheckers clangStaticAnalyzerCore)
+    clangStaticAnalyzerFrontend clangStaticAnalyzerCheckers clangStaticAnalyzerCore
+    clangARCMigrate)
 
 add_clang_library(clangFrontendTool
   ExecuteCompilerInvocation.cpp
diff --git a/lib/FrontendTool/ExecuteCompilerInvocation.cpp b/lib/FrontendTool/ExecuteCompilerInvocation.cpp
index 664b533..7ad263e 100644
--- a/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ b/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -14,6 +14,7 @@
 
 #include "clang/FrontendTool/Utils.h"
 #include "clang/StaticAnalyzer/Frontend/FrontendActions.h"
+#include "clang/ARCMigrate/ARCMTActions.h"
 #include "clang/CodeGen/CodeGenAction.h"
 #include "clang/Driver/CC1Options.h"
 #include "clang/Driver/OptTable.h"
@@ -89,6 +90,21 @@ static FrontendAction *CreateFrontendAction(CompilerInstance &CI) {
   if (!Act)
     return 0;
 
+  // Potentially wrap the base FE action in an ARC Migrate Tool action.
+  switch (CI.getFrontendOpts().ARCMTAction) {
+  case FrontendOptions::ARCMT_None:
+    break;
+  case FrontendOptions::ARCMT_Check:
+    Act = new arcmt::CheckAction(Act);
+    break;
+  case FrontendOptions::ARCMT_Modify:
+    Act = new arcmt::TransformationAction(Act);
+    break;
+  case FrontendOptions::ARCMT_ModifyInMemory:
+    Act = new arcmt::InMemoryTransformationAction(Act);
+    break;
+  }
+
   // If there are any AST files to merge, create a frontend action
   // adaptor to perform the merge.
   if (!CI.getFrontendOpts().ASTMergeFiles.empty())
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to