Merge pull request #59 from chuckatkins/cleanup-adios_tcc-template-seperation

Seperate templete impl from public and private for ADIOS class

Contributing.md

@@ -3,10 +3,32 @@
 This guide will walk you through how to submit changes to ADIOS and interact
 with the project as a developer.
 
+Table of Contents
+=================
+
+   * [Contributor's Guide](#contributors-guide)
+   * [Table of Contents](#table-of-contents)
+      * [Workflow](#workflow)
+      * [Setup](#setup)
+      * [Making a change and submitting a pull request](#making-a-change-and-submitting-a-pull-request)
+         * [Create the topic branch](#create-the-topic-branch)
+            * [Do I need to merge master into my branch first?](#do-i-need-to-merge-master-into-my-branch-first)
+         * [Submit a pull request](#submit-a-pull-request)
+      * [Template implementation separation](#template-implementation-separation)
+         * [Example](#example)
+            * [Before separation of public and private template implementation](#before-separation-of-public-and-private-template-implementation)
+               * [Foo.h](#fooh-containing-all-implementation)
+            * [After separation of public and private template implementation](#after-separation-of-public-and-private-template-implementation)
+               * [Foo.h](#fooh-containing-only-prototypes-and-explicit-instantiation-declarations)
+               * [Foo.inl](#fooinl-containing-template-implementations-that-always-need-to-be-included)
+               * [Foo.tcc](#footcc-containing-template-implementations-that-should-be-restricted-to-only-known-types)
+               * [Foo.cpp](#foocpp-containing-non-template-implementations-and-explicit-instantiations-definitions-for-known-types)
+      * [Code formatting and style](#code-formatting-and-style)
+
 ## Workflow
 ADIOS uses the GitHub fork-and-branch model. In this, the project "lives" in it's main repository located at https://github.com/ornladios/adios2.git, while each individual developer has their own copy of the repo to work in.  Changes are then submitted to the main repository via pull-requests made with branches from your fork.
 
-### Setup
+## Setup
 To setup your local repository for development:
 
   1. Fork the main repository on GitHub:
@@ -49,10 +71,10 @@ Setting up git hooks...
 $
 ```
 
-### Making a change and submitting a pull request
+## Making a change and submitting a pull request
 At this point you are ready to get to work.  The first thing to do is to create a branch.  ADIOS uses a "branchy" workflow where all changes are committed through self-contained "topic branches".  This helps ensure a clean traceable git history and reduce conflicts.
 
-#### Create the topic branch
+### Create the topic branch
 
 1. Make sure you are starting from a current master:
 ```
@@ -79,7 +101,7 @@ Branch <your-topic-branch-name> set up to track remote branch <your-topic-branch
 $
 ```
 
-##### Do I need to merge master into my branch first?
+#### Do I need to merge master into my branch first?
 Not usually.  The only time to do that is to resolve conflicts.  You're pull request will be automatically rejected if merge-conflicts exist, in which case you can then resolve them by either re-basing your branch the current master (preferable):
 ```
 $ git fetch --all -p
@@ -95,7 +117,7 @@ $ git merge upstream/master
 $ git push -f
 ```
 
-#### Submit a pull request
+### Submit a pull request
 1. Log in to your GitHub fork.
 2. You should see a message at the top that informs you of your recently pushed branch, something like: `<your-topic-branch-name> (2 minutes ago)`.  On the right side, select the `[Compare & pull request]` button.
 3. Fill in the appropriate information for the name of the branch and a brief summary of the changes it contains.
@@ -104,14 +126,236 @@ $ git push -f
 
 You have now created a pull request (PR) that is pending several status checks before it can be merged.  Currently, the only check being performed is for source code formatting and style.  In the future, however, the will be a more in depth continuous integration system tied to the pull requests that tests for build and test failures every time a PR is submitted or updated.  Once the status checks pass, the PR will be eligible for merging by one of the project maintainers.
 
-### Code formatting and style
+## Template implementation separation
+The ADIOS C++ classes try to explicitly separate class declarations from their implementation.  Typically this is done by having a separate .h and .cpp file,  however it get's more complicated when templates are involved.  To maintain the distinct separation between definition and implementation, we use explicit instantiation with 4 different source file types:
+* ClassName.h
+  * The main header file containing *only* the class and member declarations with no implementation.  This also contains the declarations for explicitly instantiated members.
+* ClassName.inl
+  * A file containing inline function implementations that need to be made public.  This is to be included at the bottom of ClassName.h and should *only* contain implementations that need to be made public.
+* ClassName.tcc
+  * A file containing most of the template implementations that can be hidden through explicit instantiation.
+* ClassName.cpp
+  * A file containing the non-template implementations and the explicit instation of any template members.
+
+### Example
+Here is an example of a simple class `Foo` with template member functions `Bar1` and `Bar2`
+
+#### Before separation of public and private template implementation
+##### Foo.h containing all implementation
+```cpp
+#ifndef FOO_H_
+#define FOO_H_
+
+namespace adios
+{
+
+class Foo
+{
+public:
+    Foo()
+    : m_Bar1Calls(0), m_Bar2Calls(0), m_Bar3Calls(0);
+    {
+    }
+
+    virtual ~Foo() = default;
+
+    template<typename T>
+    void Bar1()
+    {
+        Bar1Helper<T>();
+    }
+
+    template<typename T>
+    void Bar2()
+    {
+        Bar2Helper<T>();
+    }
+
+    void Bar3()
+    {
+        Bar3Helper();
+    }
+
+private:
+    template<typename T>
+    void Bar1Helper()
+    {
+        ++m_Bar1Calls;
+    }
+
+    template<typename T>
+    void Bar2Helper()
+    {
+        ++m_Bar2Calls;
+    }
+
+    void Bar3Helper()
+    {
+        ++m_Bar3Calls;
+    }
+
+    size_t m_Bar1Calls;
+    size_t m_Bar2Calls;
+    size_t m_Bar3Calls;
+};
+
+} // end namespace adios
+#endif // FOO_H_
+```
+
+#### After separation of public and private template implementation
+
+In this example, we want to hide the template implementation from the header.  We will implement this such that `Bar1` is only callable from the core numeric types, i.e. ints, floats, and complex, while `Bar2` is callable from all types.  This will necessitate that `Bar1` and it's helper function is implemented in a .tcc file with explicit instantiation for the allowed types while `Bar2` and it's helper function will need to be inlined in the .inl file to be accessible for all types.  We will also use a helper macro ADIOS provides to iterate over the core numeric types for the explicit instantiation of `Bar1`.
+
+##### Foo.h containing only prototypes and explicit instantiation declarations
+```cpp
+#ifndef FOO_H_
+#define FOO_H_
+
+#include "ADIOSMacros.h"
+
+namespace adios
+{
+class Foo
+{
+public:
+    Foo();
+    virtual ~Foo() = default;
+
+    template<typename T>
+    void Bar1();
+
+    template<typename T>
+    void Bar2();
+
+    void Bar3();
+private:
+    template<typename T>
+    void Bar1Helper();
+
+    template<typename T>
+    void Bar2Helper();
+
+    void Bar3Helper;
+
+    size_t m_Bar1Calls;
+    size_t m_Bar2Calls;
+    size_t m_Bar3Calls;
+};
+
+// Create declarations for explicit instantiations
+#define declare_explicit_instantiation(T)       \
+    extern template void Foo::Bar1<T>();
+
+ADIOS_FOREACH_TYPE_1ARG(declare_explicit_instantiation)
+#undef(declare_explicit_instantiation)
+} // end namespace adios
+
+#include "Foo.inl"
+#endif // FOO_H_
+```
+Note here that Bar1Helper does not need an explicit instantiation because it's not a visible funtion in the callable interface.  It's implementaion will be available to Bar1 inside the tcc file where it's called from.
+
+##### Foo.inl containing template implementations that always need to be included
+```cpp
+#ifndef FOO_INL_
+#define FOO_INL_
+#ifndef FOO_H_
+#error "Inline file should only be included from it's header, never on it's own"
+#endif
+
+// No need to include Foo.h since it's where this is include from
+
+namespace adios
+{
+
+template<typename T>
+void Foo::Bar2()
+{
+    Bar2Helper<T>();
+}
+
+template<typename T>
+void Foo::Bar2Helper()
+{
+    ++m_Bar2Calls;
+}
+
+} // end namespace adios
+
+#endif // FOO_INL_
+```
+
+##### Foo.tcc containing template implementations that should be restricted to only known types
+```cpp
+#ifndef FOO_TCC_
+#define FOO_TCC_
+
+#include "Foo.h"
+namespace adios
+{
+
+template<typename T>
+void Foo::Bar1()
+{
+    Bar1Helper<T>();
+}
+
+template<typename T>
+void Foo::Bar1Helper()
+{
+    ++m_Bar1Calls;
+}
+
+} // end namespace adios
+
+#endif // FOO_TCC_
+```
+
+##### Foo.cpp containing non-template implementations and explicit instantiations definitions for known types.
+```cpp
+#include "Foo.h"
+#include "Foo.tcc"
+
+namespace adios
+{
+
+Foo::Foo()
+: m_Bar1Calls(0), m_Bar2Calls(0), m_Bar3Calls(0)
+{
+}
+
+void Foo::Bar3()
+{
+    Bar3Helper();
+}
+
+void Foo::Bar3Helper()
+{
+    ++m_Bar3Calls;
+}
+
+// Create explicit instantiations of existing definitions
+#define define_explicit_instantiation(T)  \
+    template void Foo::Bar1<T>();
+
+ADIOS_FOREACH_TYPE_1ARG(define_explicit_instantiation)
+#undef(define_explicit_instantiation)
+
+} // end namespace adios
+```
+
+## Code formatting and style
 ADIOS uses the clang-format tool to automatically enforce source code style and formatting rules.  There are various ways to integrate the clang-format tool into your IDE / Code Editor depending on if you use Emacs, Vim, Eclipse, KDevelop, Microsoft Visual Studio, etc. that are a bit outside the scope of this document but a quick google search for "integrate <insert-editor-here> clang-format" should point you in the right direction.  However, you can always reformat the code manually by running:
 ```
 clang-format -i SourceFile.cpp SourceFile.h
 ```
 That will apply the formatting rules used by the ADIOS project.
+
+While some of the formatting rules are fairly detailed, the main points are:
+
 1. Lines no longer than 80 characters.
 1. Always use braces { and }, even for 1 line if blocks.
 1. Use 4 spaces for indentation.
 
-There are more formatting rules but these three should at least get you close and prevent any drastic re-writes from the re-formatting tools.  More details can be found by looking at the .clang-format config file n the root of the repository and by looking at the clang-format documentation http://releases.llvm.org/3.8.0/tools/clang/docs/ClangFormatStyleOptions.html.  While the formatting rules are a bit more involved, the main points are:
+There are more formatting rules but these three should at least get you close and prevent any drastic re-writes from the re-formatting tools.  More details can be found by looking at the .clang-format config file in the root of the repository and by looking at the clang-format documentation http://releases.llvm.org/3.8.0/tools/clang/docs/ClangFormatStyleOptions.html.

include/ADIOS.h

@@ -343,19 +343,22 @@ protected: // no const to allow default empty and copy constructors
     std::map<unsigned int, Variable<T>> &GetVariableMap();
 };
 
+//------------------------------------------------------------------------------
+
 // Explicit declaration of the template methods
 #define declare_template_instantiation(T)                                      \
     extern template Variable<T> &ADIOS::DefineVariable<T>(                     \
         const std::string &name, const Dims, const Dims, const Dims);          \
-    extern template Variable<T> &ADIOS::GetVariable<T>(const std::string &);   \
-    extern template unsigned int ADIOS::GetVariableIndex<T>(                   \
-        const std::string &name);                                              \
-    template <>                                                                \
-    std::map<unsigned int, Variable<T>> &ADIOS::GetVariableMap<T>();
+                                                                               \
+    extern template Variable<T> &ADIOS::GetVariable<T>(const std::string &);
+
 ADIOS_FOREACH_TYPE_1ARG(declare_template_instantiation)
 extern template unsigned int ADIOS::GetVariableIndex<void>(const std::string &);
 #undef declare_template_instantiation
 
 } // end namespace adios
 
+// Include the inline implementations for the public interface
+#include "ADIOS.inl"
+
 #endif /* ADIOS_H_ */

include/ADIOS.tcc → include/ADIOS.inl

@@ -6,10 +6,11 @@
  *   This contains the template implementations for the ADIOS class
  */
 
-#ifndef ADIOS_TCC_
-#define ADIOS_TCC_
-
-#include "ADIOS.h"
+#ifndef ADIOS_INL_
+#define ADIOS_INL_
+#ifndef ADIOS_H_
+#error "Inline file should only be included from it's header, never on it's own"
+#endif
 
 namespace adios
 {
@@ -31,4 +32,4 @@ VariableCompound &ADIOS::DefineVariableCompound(const std::string &name,
 
 } // end namespace adios
 
-#endif /* ADIOS_TCC_ */
+#endif /* ADIOS_INL_ */

source/ADIOS.cpp

@@ -18,6 +18,7 @@
 
 #include "ADIOS.h"
 #include "ADIOS.tcc"
+#include "ADIOSMacros.h"
 
 #include "functions/adiosFunctions.h"
 
@@ -379,4 +380,19 @@ void ADIOS::CheckMethod(std::map<std::string, Method>::const_iterator itMethod,
     }
 }
 
+//------------------------------------------------------------------------------
+
+// Explicitly instantiate the necessary template implementations
+#define define_template_instantiation(T)                                       \
+    template Variable<T> &ADIOS::DefineVariable<T>(                            \
+        const std::string &, const Dims, const Dims, const Dims);              \
+                                                                               \
+    template Variable<T> &ADIOS::GetVariable<T>(const std::string &);
+
+ADIOS_FOREACH_TYPE_1ARG(define_template_instantiation)
+template unsigned int ADIOS::GetVariableIndex<void>(const std::string &);
+#undef define_template_instatiation
+
+//------------------------------------------------------------------------------
+
 } // end namespace adios

source/ADIOS_inst.cpp → source/ADIOS.tcc

@@ -6,7 +6,10 @@
  *   This contains the template specializatios for the ADIOS class
  */
 
-#include "ADIOS.tcc"
+#ifndef ADIOS_TCC_
+#define ADIOS_TCC_
+
+#include "ADIOS.h"
 #include "ADIOSMacros.h"
 
 namespace adios
@@ -114,8 +117,6 @@ ADIOS::GetVariableMap()
     return m_CLDouble;
 }
 
-// -----------------------------------------------------------------------------
-// explicit template instantiations of DefineVariable:
 // -----------------------------------------------------------------------------
 
 template <typename T>
@@ -133,14 +134,6 @@ ADIOS::DefineVariable(const std::string &name, const Dims globalDimensions,
     return variableMap.at(size);
 }
 
-#define define_template_instantiation(T)                                       \
-    template Variable<T> &ADIOS::DefineVariable<T>(                            \
-        const std::string &, const Dims, const Dims, const Dims);
-ADIOS_FOREACH_TYPE_1ARG(define_template_instantiation)
-#undef define_template_instatiation
-
-// -----------------------------------------------------------------------------
-// template specializations of GetVariable:
 // -----------------------------------------------------------------------------
 
 template <class T>
@@ -160,11 +153,6 @@ Variable<T> &ADIOS::GetVariable(const std::string &name)
     return GetVariableMap<T>().at(GetVariableIndex<T>(name));
 }
 
-#define define_template_instatiation(T)                                        \
-    template unsigned int ADIOS::GetVariableIndex<T>(const std::string &);     \
-    template Variable<T> &ADIOS::GetVariable<T>(const std::string &);
-ADIOS_FOREACH_TYPE_1ARG(define_template_instatiation)
-template unsigned int ADIOS::GetVariableIndex<void>(const std::string &);
-#undef define_template_instatiation
-
 } // end namespace adios
+
+#endif // ADIOS_TCC_

source/CMakeLists.txt

@@ -10,7 +10,7 @@ endif()
   
 foreach(adios2_target IN LISTS adios2_targets)
   add_library(${adios2_target}
-    ADIOS.cpp ADIOS_inst.cpp
+    ADIOS.cpp ADIOS.tcc
     #ADIOS_C.cpp
   
     capsule/heap/STLVector.cpp