Eliminate singleton configuration with custom FrontendActionFactory. (#281)

This PR eliminates the need for the singleton instantiation of the chimera::Configuration by implementing custom factory classes that allow the configuration to be passed to the ASTConsumer directly.

Author: psigen

.dockerignore

@@ -1,6 +1,5 @@
 build/
 ci/
-test/
 .codecov.yml
 .gitignore
 .travis.yml

CMakeLists.txt

@@ -95,9 +95,9 @@ set(${PROJECT_NAME}_SOURCES
   src/configuration.cpp
   src/consumer.cpp
   src/frontend_action.cpp
-  src/visitor.cpp
-  src/util.cpp
   src/mstch.cpp
+  src/util.cpp
+  src/visitor.cpp
 )
 
 ## Use LLVM CMake macro to collate and order link libraries.
@@ -171,8 +171,11 @@ install(FILES "${CMAKE_CURRENT_BINARY_DIR}/${PROJECT_NAME}Config.cmake"
 )
 
 # Enable unit testing by including the `test` subdirectory.
-enable_testing()
-add_subdirectory(test EXCLUDE_FROM_ALL)
+include(CTest)
+if(BUILD_TESTING)
+  enable_testing()
+  add_subdirectory(test EXCLUDE_FROM_ALL)
+endif()
 
 #####################
 ## CODE FORMATTING ##

Dockerfile

@@ -1,14 +1,15 @@
-FROM ubuntu:16.04 as builder
+FROM ubuntu:18.04 as builder
 
 # Install APT dependencies required for building chimera.
 RUN apt-get update -qq \
   && apt-get install -y -qq \
   cmake \
   clang \
-  llvm-3.7-dev \
-  llvm-3.7-tools \
+  clang-format \
+  llvm-6.0-dev \
+  llvm-6.0-tools \
   libboost-dev \
-  libclang-3.7-dev \
+  libclang-6.0-dev \
   libedit-dev \
   libyaml-cpp-dev \
   libz-dev \
@@ -22,13 +23,13 @@ RUN mkdir build && cd build \
   && make install
 
 #############################################################################
-FROM ubuntu:16.04
+FROM ubuntu:18.04
 
 # Install APT dependencies required for running chimera.
 RUN apt-get update -qq \
   && apt-get install -y -qq \
-  llvm-3.7-tools \
-  libclang-3.7 \
+  llvm-6.0-tools \
+  libclang-6.0 \
   libedit2 \
   libyaml-cpp0.5v5 \
   zlib1g \

include/chimera/configuration.h

@@ -34,14 +34,10 @@ class CompiledConfiguration;
 class Configuration
 {
 public:
+    Configuration();
     Configuration(const Configuration &) = delete;
     Configuration &operator=(const Configuration &) = delete;
 
-    /**
-     * Gets the chimera configuration singleton for this process.
-     */
-    static Configuration &GetInstance();
-
     /**
      * Loads the specified file to use as the YAML configuration.
      */
@@ -108,9 +104,6 @@ class Configuration
      */
     const std::string &GetOutputModuleName() const;
 
-private:
-    Configuration();
-
 protected:
     YAML::Node configNode_;
     std::string bindingName_;

include/chimera/consumer.h

@@ -1,6 +1,8 @@
 #ifndef __CHIMERA_CONSUMER_H__
 #define __CHIMERA_CONSUMER_H__
 
+#include "chimera/configuration.h"
+
 #include <clang/AST/ASTConsumer.h>
 #include <clang/Frontend/CompilerInstance.h>
 
@@ -11,13 +13,14 @@ class Consumer : public clang::ASTConsumer
 {
 public:
     // Overrides the constructor in order to receive CompilerInstance.
-    Consumer(clang::CompilerInstance *ci);
+    Consumer(clang::CompilerInstance *ci, const chimera::Configuration &config);
 
     // Overrides method to call our ChimeraVisitor on the entire source file.
     void HandleTranslationUnit(clang::ASTContext &context) override;
 
 private:
     clang::CompilerInstance *ci_;
+    const chimera::Configuration &config_;
 };
 
 } // namespace chimera

include/chimera/frontend_action.h

@@ -1,9 +1,14 @@
 #ifndef __CHIMERA_FRONTEND_ACTION_H__
 #define __CHIMERA_FRONTEND_ACTION_H__
 
+#include "chimera/configuration.h"
+#include "chimera/util.h"
+
+#include <memory>
 #include <clang/AST/ASTConsumer.h>
 #include <clang/Frontend/CompilerInstance.h>
-#include "clang/Frontend/FrontendActions.h"
+#include <clang/Frontend/FrontendActions.h>
+#include <clang/Tooling/Tooling.h>
 
 namespace chimera
 {
@@ -14,10 +19,41 @@ namespace chimera
 class FrontendAction : public clang::ASTFrontendAction
 {
 public:
+    // Overrides the constructor in order to receive ChimeraConfiguration.
+    FrontendAction(const chimera::Configuration &config);
+
     std::unique_ptr<clang::ASTConsumer> CreateASTConsumer(
         clang::CompilerInstance &CI, clang::StringRef file) override;
+
+protected:
+    const chimera::Configuration &config_;
+};
+
+// Create a custom action factory that forwards the ChimeraConfiguration.
+class ChimeraFrontendActionFactory
+  : public clang::tooling::FrontendActionFactory
+{
+public:
+    ChimeraFrontendActionFactory(const chimera::Configuration &config);
+
+// Between Clang 9 and Clang 10, the return value for
+// FrontendActionFactory::create() changed from raw pointer to std::unique_ptr.
+#if LLVM_VERSION_AT_LEAST(10, 0, 0)
+    std::unique_ptr<clang::FrontendAction> create() override;
+#else
+    clang::FrontendAction *create() override;
+#endif
+
+protected:
+    const chimera::Configuration &config_;
 };
 
+/**
+ * Custom frontend factory that forwards a ChimeraConfiguration.
+ */
+std::unique_ptr<clang::tooling::FrontendActionFactory> newFrontendActionFactory(
+    const chimera::Configuration &config);
+
 } // namespace chimera
 
 #endif // __CHIMERA_FRONTEND_ACTION_H__

src/chimera.cpp

@@ -10,7 +10,6 @@
 #include <string>
 #include <clang/Tooling/ArgumentsAdjusters.h>
 #include <clang/Tooling/CommonOptionsParser.h>
-#include <clang/Tooling/Tooling.h>
 #include <llvm/Support/CommandLine.h>
 
 #define STR_DETAIL(x) #x
@@ -99,38 +98,40 @@ int run(int argc, const char **argv)
     // Create parser that handles clang options.
     CommonOptionsParser OptionsParser(argc, argv, ChimeraCategory);
 
+    // Create a new configuration that will be used for the run.
+    chimera::Configuration Config;
+
     // Parse the YAML configuration file if it exists, otherwise initialize it
     // to an empty node.
     if (!ConfigFilename.empty())
-        chimera::Configuration::GetInstance().LoadFile(ConfigFilename);
+        Config.LoadFile(ConfigFilename);
 
     // If a binding definition was specified, set configuration to use it.
     if (!BindingName.empty())
-        chimera::Configuration::GetInstance().SetBindingName(BindingName);
+        Config.SetBindingName(BindingName);
 
     // If an output path was specified, set configuration to use it.
     if (!OutputPath.empty())
-        chimera::Configuration::GetInstance().SetOutputPath(OutputPath);
+        Config.SetOutputPath(OutputPath);
 
     // If a top-level binding file was specified, set configuration to use it.
     if (!OutputModuleName.empty())
-        chimera::Configuration::GetInstance().SetOutputModuleName(
-            OutputModuleName);
+        Config.SetOutputModuleName(OutputModuleName);
 
     // Add top-level namespaces to the configuration.
     if (NamespaceNames.size())
         for (const std::string &name : NamespaceNames)
-            chimera::Configuration::GetInstance().AddInputNamespaceName(name);
+            Config.AddInputNamespaceName(name);
 
     // Add compilation source paths to the configuration.
     // These will be made available to templates.
     if (!SuppressSources)
         for (const std::string &path : OptionsParser.getSourcePathList())
-            chimera::Configuration::GetInstance().AddSourcePath(path);
+            Config.AddSourcePath(path);
 
     // If strict option is on, treats unresolvable configuration as errors.
     if (Strict)
-        chimera::Configuration::GetInstance().SetStrict(true);
+        Config.SetStrict(true);
 
     // Create tool that uses the command-line options.
     ClangTool Tool(OptionsParser.getCompilations(),
@@ -153,7 +154,7 @@ int run(int argc, const char **argv)
         ArgumentInsertPosition::END));
 
     // Run the instantiated tool on the Chimera frontend.
-    return Tool.run(newFrontendActionFactory<chimera::FrontendAction>().get());
+    return Tool.run(chimera::newFrontendActionFactory(Config).get());
 }
 
 } // namespace chimera

src/configuration.cpp

@@ -67,12 +67,6 @@ chimera::Configuration::Configuration()
     // Do nothing.
 }
 
-chimera::Configuration &chimera::Configuration::GetInstance()
-{
-    static chimera::Configuration config;
-    return config;
-}
-
 void chimera::Configuration::LoadFile(const std::string &filename)
 {
     try

src/consumer.cpp

@@ -1,23 +1,22 @@
 #include "chimera/consumer.h"
-#include "chimera/configuration.h"
 #include "chimera/visitor.h"
 
 #include <iostream>
 
 using namespace clang;
 
-chimera::Consumer::Consumer(CompilerInstance *ci) : ci_(ci)
+chimera::Consumer::Consumer(CompilerInstance *ci,
+                            const chimera::Configuration &config)
+  : ci_(ci), config_(config)
 {
     // Do nothing.
 }
 
 void chimera::Consumer::HandleTranslationUnit(ASTContext &context)
 {
     // Use the current translation unit to resolve the YAML configuration.
-    chimera::Configuration &config = chimera::Configuration::GetInstance();
-
     std::unique_ptr<chimera::CompiledConfiguration> compiled_config
-        = config.Process(ci_);
+        = config_.Process(ci_);
     chimera::Visitor visitor(ci_, *compiled_config);
 
     // We can use ASTContext to get the TranslationUnitDecl, which is

src/frontend_action.cpp

@@ -5,6 +5,12 @@
 
 using namespace clang;
 
+chimera::FrontendAction::FrontendAction(const chimera::Configuration &config)
+  : config_(config)
+{
+    // Do nothing.
+}
+
 std::unique_ptr<clang::ASTConsumer> chimera::FrontendAction::CreateASTConsumer(
     CompilerInstance &CI, StringRef /*file*/)
 {
@@ -17,5 +23,36 @@ std::unique_ptr<clang::ASTConsumer> chimera::FrontendAction::CreateASTConsumer(
     is_first = false;
 
     CI.getPreprocessor().getDiagnostics().setIgnoreAllWarnings(true);
-    return std::unique_ptr<chimera::Consumer>(new chimera::Consumer(&CI));
+    return std::unique_ptr<chimera::Consumer>(
+        new chimera::Consumer(&CI, config_));
+}
+
+// Create a custom action factory that forwards the ChimeraConfiguration.
+chimera::ChimeraFrontendActionFactory::ChimeraFrontendActionFactory(
+    const chimera::Configuration &config)
+  : config_(config)
+{
+    // Do nothing.
+}
+
+// Between Clang 9 and Clang 10, the return value for
+// FrontendActionFactory::create() changed from raw pointer to std::unique_ptr.
+#if LLVM_VERSION_AT_LEAST(10, 0, 0)
+std::unique_ptr<FrontendAction> chimera::ChimeraFrontendActionFactory::create()
+{
+    return std::unique_ptr<FrontendAction>(
+        new chimera::FrontendAction(config_));
+}
+#else
+FrontendAction *chimera::ChimeraFrontendActionFactory::create()
+{
+    return new chimera::FrontendAction(config_);
+}
+#endif
+
+std::unique_ptr<tooling::FrontendActionFactory>
+chimera::newFrontendActionFactory(const chimera::Configuration &config)
+{
+    return std::unique_ptr<tooling::FrontendActionFactory>(
+        new chimera::ChimeraFrontendActionFactory(config));
 }