Add ExtraCode declarationFile and implementationFile directives (#601)

* add ExtraCode declarationFile and implementationFile directives * add declarationFile extracode for components * mend * fix pre-commit exclude * add test for mutable declarationFile and implementationFile * copy extra code files to root_io and sio_io for dumpmodel tests * revert embedding files with jinja * add including implementationFile and declarationFile during parsing * Update .pre-commit-config.yaml * remove checks for not yet implemented extra code, fix space Co-authored-by: tmadlener <[email protected]> * add documentation of file paths * add DEPENDS option to datamodel generating macro and use it with extra code files * Fix typo in documentation --------- Co-authored-by: tmadlener <[email protected]>
AIDASoft · May 22, 2024 · 0a5af60 · 0a5af60
1 parent 6eefdc4
commit 0a5af60
Show file tree

Hide file tree

Showing 12 changed files with 127 additions and 30 deletions.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -11,12 +11,12 @@ repos:
         name: clang-tidy
         entry: clang-tidy -warnings-as-errors='*,-clang-diagnostic-deprecated-declarations' -p compile_commands.json
         types: [c++]
-        exclude: (tests/(datamodel|src)/.*(h|cc)|podioVersion.in.h|SIOFrame.*h)
+        exclude: (tests/(datamodel|src|extra_code)/.*(h|cc)|podioVersion.in.h|SIOFrame.*h)
         language: system
       - id: clang-format
         name: clang-format
         entry: .github/scripts/clang-format-hook
-        exclude: (tests/(datamodel|src)/.*(h|cc)$|podioVersion.in.h)
+        exclude: (tests/(datamodel|src|extra_code)/.*(h|cc)$|podioVersion.in.h)
         types: [c++]
         language: system
       - id: pylint

diff --git a/cmake/podioMacros.cmake b/cmake/podioMacros.cmake
@@ -130,13 +130,14 @@ set_property(CACHE PODIO_USE_CLANG_FORMAT PROPERTY STRINGS AUTO ON OFF)
 #      SCHEMA_EVOLUTION     OPTIONAL: The path to the yaml file declaring the necessary schema evolution
 #      LANG                 OPTIONAL: The programming language choice
 #                           Default is cpp
+#      DEPENDS              OPTIONAL: List of files to be added as configure dependencies of the datamodel
 # )
 #
 # Note that the create_${datamodel} target will always be called, but if the YAML_FILE has not changed
 # this is essentially a no-op, and should not cause re-compilation.
 #---------------------------------------------------------------------------------------------------
 function(PODIO_GENERATE_DATAMODEL datamodel YAML_FILE RETURN_HEADERS RETURN_SOURCES)
-  CMAKE_PARSE_ARGUMENTS(ARG "" "OLD_DESCRIPTION;OUTPUT_FOLDER;UPSTREAM_EDM;SCHEMA_EVOLUTION" "IO_BACKEND_HANDLERS;LANG" ${ARGN})
+  CMAKE_PARSE_ARGUMENTS(ARG "" "OLD_DESCRIPTION;OUTPUT_FOLDER;UPSTREAM_EDM;SCHEMA_EVOLUTION" "IO_BACKEND_HANDLERS;LANG;DEPENDS" ${ARGN})
   IF(NOT ARG_OUTPUT_FOLDER)
     SET(ARG_OUTPUT_FOLDER ${CMAKE_CURRENT_SOURCE_DIR})
   ENDIF()
@@ -208,6 +209,7 @@ function(PODIO_GENERATE_DATAMODEL datamodel YAML_FILE RETURN_HEADERS RETURN_SOUR
     ${podio_PYTHON_DIR}/podio_gen/generator_base.py
     ${podio_PYTHON_DIR}/podio_gen/cpp_generator.py
     ${podio_PYTHON_DIR}/podio_gen/julia_generator.py
+    ${ARG_DEPENDS}
   )
 
   message(STATUS "Creating '${datamodel}' datamodel")

diff --git a/doc/datamodel_syntax.md b/doc/datamodel_syntax.md
@@ -119,17 +119,17 @@ The `includes` will be add to the header files of the generated classes.
       includes: <newline separated list of strings (optional)>
       declaration: <string>
       implementation : <string>
-      declarationFile: <string> (to be implemented!)
-      implementationFile: <string> (to be implemented!)
+      declarationFile: <string>
+      implementationFile: <string>
     MutableExtraCode:
       includes: <newline separated list of strings (optional)>
       declaration: <string>
       implementation : <string>
-      declarationFile: <string> (to be implemented!)
-      implementationFile: <string> (to be implemented!)
+      declarationFile: <string>
+      implementationFile: <string>
 ```
 
-The code being provided has to use the macro `{name}` in place of the concrete name of the class.
+The code being provided has to use the macro `{name}` in place of the concrete name of the class. The paths to the files given in `declarationFile` and `implementationFile` should be either absolute or relative to the datamodel yaml file. The cmake users are encouraged to specify these file via the `DEPENDS` option of `PODIO_GENERATE_DATAMODEL` to add the files as configuration dependency of the datamodel and cause the datamodel re-generation when they change.
 
 ## Definition of custom interfaces
 An interface type can be defined as follows (again using an example from the example datamodel)

diff --git a/python/podio_gen/podio_config_reader.py b/python/podio_gen/podio_config_reader.py
@@ -2,6 +2,7 @@
 
 import copy
 import re
+import os
 import yaml
 
 from podio_gen.generator_utils import (
@@ -171,9 +172,13 @@ class ClassDefinitionValidator:
     # it applies and also which accessor functions to generate
     required_interface_keys = required_datatype_keys + ("Members", "Types")
 
-    valid_extra_code_keys = ("declaration", "implementation", "includes")
-    # documented but not yet implemented
-    not_yet_implemented_extra_code = ("declarationFile", "implementationFile")
+    valid_extra_code_keys = (
+        "declaration",
+        "implementation",
+        "includes",
+        "declarationFile",
+        "implementationFile",
+    )
 
     @classmethod
     def validate(cls, datamodel, upstream_edm=None):
@@ -205,10 +210,10 @@ def _check_components(cls, datamodel, upstream_edm):
 
             if "ExtraCode" in component:
                 for key in component["ExtraCode"]:
-                    if key not in ("declaration", "includes"):
+                    if key not in ("declaration", "declarationFile", "includes"):
                         raise DefinitionError(
-                            f"'{key}' field found in 'ExtraCode' of component '{name}'."
-                            " Only 'declaration' and 'includes' are allowed here"
+                            f"'{key}' field found in 'ExtraCode' of component '{name}'. "
+                            "Only 'declaration', 'declarationFile' and 'includes' are allowed here"
                         )
 
             for member in component["Members"]:
@@ -367,15 +372,8 @@ def _check_keys(cls, classname, definition):
             extracode = definition["ExtraCode"]
             invalid_keys = [k for k in extracode if k not in cls.valid_extra_code_keys]
             if invalid_keys:
-                not_yet_impl = [k for k in invalid_keys if k in cls.not_yet_implemented_extra_code]
-                if not_yet_impl:
-                    not_yet_impl = f" (not yet implemented: {not_yet_impl})"
-                else:
-                    not_yet_impl = ""
-
                 raise DefinitionError(
-                    f"{classname} defines invalid 'ExtraCode' categories: "
-                    f"{invalid_keys}{not_yet_impl}"
+                    f"{classname} defines invalid 'ExtraCode' categories: " f"{invalid_keys}"
                 )
 
     @classmethod
@@ -442,8 +440,20 @@ def _handle_extracode(definition):
         """Handle the extra code definition. Currently simply returning a copy"""
         return copy.deepcopy(definition)
 
+    @staticmethod
+    def _expand_extracode(definitions, parent_path, directive):
+        """Expand extra code directives by including files from 'File' directives
+        Relative paths to files are extended with parent_path. Mutates definitions"""
+        if directive + "File" in definitions.keys():
+            filename = definitions.pop(directive + "File")
+            if not os.path.isabs(filename):
+                filename = os.path.join(parent_path, filename)
+            with open(filename, "r", encoding="utf-8") as stream:
+                contents = stream.read()
+            definitions[directive] = definitions.get(directive, "") + "\n" + contents
+
     @classmethod
-    def _read_component(cls, definition):
+    def _read_component(cls, definition, parent_path):
         """Read the component and put it into an easily digestible format."""
         component = {}
         for name, category in definition.items():
@@ -452,13 +462,17 @@ def _read_component(cls, definition):
                 for member in definition[name]:
                     # for components we do not require a description in the members
                     component["Members"].append(cls.member_parser.parse(member, False))
+            elif name == "ExtraCode":
+                extra_code = copy.deepcopy(category)
+                cls._expand_extracode(extra_code, parent_path, "declaration")
+                component[name] = extra_code
             else:
                 component[name] = copy.deepcopy(category)
 
         return component
 
     @classmethod
-    def _read_datatype(cls, value):
+    def _read_datatype(cls, value, parent_path):
         """Read the datatype and put it into an easily digestible format"""
         datatype = {}
         for category, definition in value.items():
@@ -468,6 +482,11 @@ def _read_datatype(cls, value):
                 for member in definition:
                     members.append(cls.member_parser.parse(member))
                 datatype[category] = members
+            elif category in ("ExtraCode", "MutableExtraCode"):
+                extra_code = copy.deepcopy(definition)
+                cls._expand_extracode(extra_code, parent_path, "implementation")
+                cls._expand_extracode(extra_code, parent_path, "declaration")
+                datatype[category] = extra_code
             else:
                 datatype[category] = copy.deepcopy(definition)
 
@@ -494,7 +513,7 @@ def _read_interface(cls, value):
         return interface
 
     @classmethod
-    def parse_model(cls, model_dict, package_name, upstream_edm=None):
+    def parse_model(cls, model_dict, package_name, upstream_edm=None, parent_path=None):
         """Parse a model from the dictionary, e.g. read from a yaml file."""
 
         try:
@@ -515,12 +534,12 @@ def parse_model(cls, model_dict, package_name, upstream_edm=None):
         components = {}
         if "components" in model_dict:
             for klassname, value in model_dict["components"].items():
-                components[klassname] = cls._read_component(value)
+                components[klassname] = cls._read_component(value, parent_path)
 
         datatypes = {}
         if "datatypes" in model_dict:
             for klassname, value in model_dict["datatypes"].items():
-                datatypes[klassname] = cls._read_datatype(value)
+                datatypes[klassname] = cls._read_datatype(value, parent_path)
 
         interfaces = {}
         if "interfaces" in model_dict:
@@ -549,5 +568,6 @@ def read(cls, yamlfile, package_name, upstream_edm=None):
         """Read the datamodel definition from the yamlfile."""
         with open(yamlfile, "r", encoding="utf-8") as stream:
             content = yaml.load(stream, yaml.SafeLoader)
+        parent_path = os.path.dirname(yamlfile)
 
-        return cls.parse_model(content, package_name, upstream_edm)
+        return cls.parse_model(content, package_name, upstream_edm, parent_path)
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -5,9 +5,17 @@ foreach( _conf ${CMAKE_CONFIGURATION_TYPES} )
   set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY_${_conf} ${CMAKE_CURRENT_BINARY_DIR} )
 endforeach()
 
+# files used in ExtraCode directives
+set(extra_code extra_code/component_declarations.cc
+  extra_code/declarations.cc
+  extra_code/implementations.cc
+  extra_code/mutable_declarations.cc
+  extra_code/mutable_implementations.cc
+)
+
 PODIO_GENERATE_DATAMODEL(datamodel datalayout.yaml headers sources
-  IO_BACKEND_HANDLERS ${PODIO_IO_HANDLERS}
-  )
+  IO_BACKEND_HANDLERS ${PODIO_IO_HANDLERS} DEPENDS ${extra_code}
+)
 
 # Use the cmake building blocks to add the different parts (conditionally)
 PODIO_ADD_DATAMODEL_CORE_LIB(TestDataModel "${headers}" "${sources}")

diff --git a/tests/datalayout.yaml b/tests/datalayout.yaml
@@ -53,6 +53,15 @@ components :
       - int i{42} // is there even another value to initialize ints to?
       - std::array<double, 10> arr {1.2, 3.4} // half initialized double array
 
+  StructWithExtraCode:
+    Members:
+      - int x
+    ExtraCode:
+      declaration: "
+        int negate() { x = -x; return x; }\n
+      "
+      declarationFile: "extra_code/component_declarations.cc"
+
 datatypes :
 
   EventInfo:
@@ -209,6 +218,26 @@ datatypes :
       - TypeWithEnergy manyEnergies // multiple relations
       - ex42::AnotherTypeWithEnergy moreEnergies // multiple namespace relations
 
+  ExampleWithExternalExtraCode:
+    Description: "Type showing usage of 'declaration', 'implementation', 'declarationFile' and 'implementationFile' directives'"
+    Author: "Mateusz Jakub Fila"
+    Members:
+      - int number // a number
+    ExtraCode:
+      declaration:
+        "int add(int i) const;"
+      declarationFile: "extra_code/declarations.cc"
+      implementation:
+        "int {name}::add(int i) const { return number() + i; }"
+      implementationFile: "extra_code/implementations.cc"
+    MutableExtraCode:
+      declaration:
+        int add_inplace(int i);
+      declarationFile: "extra_code/mutable_declarations.cc"
+      implementation:
+        int {name}::add_inplace(int i) { return number() += i; }
+      implementationFile: "extra_code/mutable_implementations.cc"
+
   nsp::EnergyInNamespace:
     Description: "A type with energy in a namespace"
     Author: "Thomas Madlener"

diff --git a/tests/extra_code/component_declarations.cc b/tests/extra_code/component_declarations.cc
@@ -0,0 +1,4 @@
+int reset() {
+  x = 0;
+  return x;
+}
diff --git a/tests/extra_code/declarations.cc b/tests/extra_code/declarations.cc
@@ -0,0 +1,4 @@
+bool gt(int i) const {
+  return number() > i;
+}
+bool lt(int i) const;
diff --git a/tests/extra_code/implementations.cc b/tests/extra_code/implementations.cc
@@ -0,0 +1 @@
+bool {name}::lt(int i) const { return number() < i; }
diff --git a/tests/extra_code/mutable_declarations.cc b/tests/extra_code/mutable_declarations.cc
@@ -0,0 +1 @@
+int reset();
diff --git a/tests/extra_code/mutable_implementations.cc b/tests/extra_code/mutable_implementations.cc
@@ -0,0 +1,4 @@
+int {name}::reset() {
+  number() = 0;
+  return number();
+}
diff --git a/tests/unittests/unittest.cpp b/tests/unittests/unittest.cpp
@@ -43,13 +43,17 @@
 #include "datamodel/ExampleWithArray.h"
 #include "datamodel/ExampleWithArrayComponent.h"
 #include "datamodel/ExampleWithComponent.h"
+#include "datamodel/ExampleWithExternalExtraCode.h"
 #include "datamodel/ExampleWithFixedWidthIntegers.h"
 #include "datamodel/ExampleWithOneRelationCollection.h"
 #include "datamodel/ExampleWithUserInitCollection.h"
 #include "datamodel/ExampleWithVectorMemberCollection.h"
 #include "datamodel/MutableExampleCluster.h"
 #include "datamodel/MutableExampleWithArray.h"
 #include "datamodel/MutableExampleWithComponent.h"
+#include "datamodel/MutableExampleWithExternalExtraCode.h"
+#include "datamodel/StructWithExtraCode.h"
+
 #include "podio/UserDataCollection.h"
 
 TEST_CASE("AutoDelete", "[basics][memory-management]") {
@@ -398,6 +402,26 @@ TEST_CASE("Extracode", "[basics][code-gen]") {
   REQUIRE(simple.z == 3);
 }
 
+TEST_CASE("ExtraCode declarationFile and implementationFile", "[basics][code-gen]") {
+  auto mutable_number = MutableExampleWithExternalExtraCode();
+  REQUIRE(mutable_number.reset() == 0);
+  REQUIRE(mutable_number.add(2) == 2);
+  REQUIRE(mutable_number.add_inplace(1) == 1);
+  REQUIRE(mutable_number.gt(-1));
+  REQUIRE(mutable_number.lt(100));
+  ExampleWithExternalExtraCode number = mutable_number;
+  REQUIRE(number.add(1) == 2);
+  REQUIRE(number.gt(-1));
+  REQUIRE(number.lt(100));
+}
+
+TEST_CASE("ExtraCode declarationFile in component", "[basics][code-gen]") {
+  auto value = StructWithExtraCode();
+  value.x = 1;
+  REQUIRE(value.negate() == -1);
+  REQUIRE(value.reset() == 0);
+}
+
 TEST_CASE("AssociativeContainer", "[basics]") {
   auto clu1 = MutableExampleCluster();
   auto clu2 = MutableExampleCluster();