Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ExtraCode declarationFile and implementationFile directives #601

Merged
merged 13 commits into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion cmake/podioMacros.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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")
Expand Down
10 changes: 5 additions & 5 deletions doc/datamodel_syntax.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
60 changes: 40 additions & 20 deletions python/podio_gen/podio_config_reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import copy
import re
import os
import yaml

from podio_gen.generator_utils import (
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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"]:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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():
Expand All @@ -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():
Expand All @@ -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)

Expand All @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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)
12 changes: 10 additions & 2 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Expand Down
29 changes: 29 additions & 0 deletions tests/datalayout.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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"
Expand Down
4 changes: 4 additions & 0 deletions tests/extra_code/component_declarations.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
int reset() {
x = 0;
return x;
}
4 changes: 4 additions & 0 deletions tests/extra_code/declarations.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
bool gt(int i) const {
return number() > i;
}
bool lt(int i) const;
1 change: 1 addition & 0 deletions tests/extra_code/implementations.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
bool {name}::lt(int i) const { return number() < i; }
1 change: 1 addition & 0 deletions tests/extra_code/mutable_declarations.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
int reset();
4 changes: 4 additions & 0 deletions tests/extra_code/mutable_implementations.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
int {name}::reset() {
number() = 0;
return number();
}
24 changes: 24 additions & 0 deletions tests/unittests/unittest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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]") {
Expand Down Expand Up @@ -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();
Expand Down