Skip to content
New issue

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

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

Already on GitHub? # to your account

Make member getter functions return by value for builtin types #519

Merged
merged 2 commits into from
Dec 1, 2023
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
19 changes: 19 additions & 0 deletions python/podio_gen/generator_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,25 @@ def getter_name(self, get_syntax):
return self.name
return _prefix_name(self.name, "get")

def getter_return_type(self, for_array=False):
"""Get the return type for a getter function for a variable

All types that are builtin will be returned by value, the rest will be
returned as const&

Args:
for_array (bool, optional): Whether the type should be for an indexed
array access
"""
if for_array:
if self.is_builtin_array:
return self.array_type
return f"const {self.array_type}&"
if self.is_builtin:
return self.full_type
# everything else will just be by const referene
return f"const {self.full_type}&"

def setter_name(self, get_syntax, is_relation=False):
"""Get the setter name of the variable"""
if is_relation:
Expand Down
6 changes: 3 additions & 3 deletions python/templates/macros/declarations.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@
{% macro member_getters(members, get_syntax) %}
{%for member in members %}
/// Access the {{ member.docstring }}
const {{ member.full_type }}& {{ member.getter_name(get_syntax) }}() const;
{{ member.getter_return_type() }} {{ member.getter_name(get_syntax) }}() const;
{% if member.is_array %}
/// Access item i of the {{ member.docstring }}
const {{ member.array_type }}& {{ member.getter_name(get_syntax) }}(size_t i) const;
{{ member.getter_return_type(True) }} {{ member.getter_name(get_syntax) }}(size_t i) const;
{%- endif %}
{% if member.sub_members %}
{% for sub_member in member.sub_members %}
/// Access the member of {{ member.docstring }}
const {{ sub_member.full_type }}& {{ sub_member.getter_name(get_sytnax) }}() const;
{{ sub_member.getter_return_type() }} {{ sub_member.getter_name(get_sytnax) }}() const;
{% endfor %}
{% endif %}

Expand Down
6 changes: 3 additions & 3 deletions python/templates/macros/implementations.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,13 @@ Mutable{{ type }} {{ full_type }}::clone() const {
{% macro member_getters(class, members, get_syntax, prefix='') %}
{% set class_type = prefix + class.bare_type %}
{% for member in members %}
const {{ member.full_type }}& {{ class_type }}::{{ member.getter_name(get_syntax) }}() const { return m_obj->data.{{ member.name }}; }
{{ member.getter_return_type() }} {{ class_type }}::{{ member.getter_name(get_syntax) }}() const { return m_obj->data.{{ member.name }}; }
{% if member.is_array %}
const {{ member.array_type }}& {{ class_type }}::{{ member.getter_name(get_syntax) }}(size_t i) const { return m_obj->data.{{ member.name }}.at(i); }
{{ member.getter_return_type(True) }} {{ class_type }}::{{ member.getter_name(get_syntax) }}(size_t i) const { return m_obj->data.{{ member.name }}.at(i); }
{% endif %}
{% if member.sub_members %}
{% for sub_member in member.sub_members %}
const {{ sub_member.full_type }}& {{ class_type }}::{{ sub_member.getter_name(get_syntax) }}() const { return m_obj->data.{{ member.name }}.{{ sub_member.name }}; }
{{ sub_member.getter_return_type() }} {{ class_type }}::{{ sub_member.getter_name(get_syntax) }}() const { return m_obj->data.{{ member.name }}.{{ sub_member.name }}; }
{% endfor %}
{%- endif %}
{% endfor %}
Expand Down
22 changes: 22 additions & 0 deletions tests/unittests/unittest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@
#include "datamodel/ExampleForCyclicDependency1Collection.h"
#include "datamodel/ExampleForCyclicDependency2Collection.h"
#include "datamodel/ExampleHitCollection.h"
#include "datamodel/ExampleWithArray.h"
#include "datamodel/ExampleWithArrayComponent.h"
#include "datamodel/ExampleWithComponent.h"
#include "datamodel/ExampleWithFixedWidthIntegers.h"
#include "datamodel/ExampleWithOneRelationCollection.h"
#include "datamodel/ExampleWithUserInitCollection.h"
#include "datamodel/ExampleWithVectorMemberCollection.h"
Expand Down Expand Up @@ -356,6 +360,24 @@ TEST_CASE("Arrays") {
}
*/

TEST_CASE("member getter return types", "[basics][code-gen]") {
// Check that the return types of the getter functions are as expected
// Builtin member types are returned by value, including fixed width integers
STATIC_REQUIRE(std::is_same_v<decltype(std::declval<ExampleHit>().energy()), double>);
STATIC_REQUIRE(std::is_same_v<decltype(std::declval<ExampleWithFixedWidthIntegers>().fixedU64()), std::uint64_t>);
// Arrays are returend by const reference
STATIC_REQUIRE(std::is_same_v<decltype(std::declval<ExampleWithArray>().myArray()), const std::array<int, 4>&>);
// But if we index into that array we get back a value
STATIC_REQUIRE(std::is_same_v<decltype(std::declval<ExampleWithArray>().myArray(0)), int>);
// Non-builtin member types are returned by const reference
STATIC_REQUIRE(std::is_same_v<decltype(std::declval<ExampleWithArrayComponent>().s()), const SimpleStruct&>);
// Accessing sub members also works as expected: builtin types by value,
// everything else by const reference
STATIC_REQUIRE(std::is_same_v<decltype(std::declval<ExampleWithArrayComponent>().x()), int>);
STATIC_REQUIRE(std::is_same_v<decltype(std::declval<ExampleWithArrayComponent>().p()), const std::array<int, 4>&>);
STATIC_REQUIRE(std::is_same_v<decltype(std::declval<ExampleWithArray>().data()), const SimpleStruct&>);
}

TEST_CASE("Extracode", "[basics][code-gen]") {
auto ev = MutableEventInfo();
ev.setNumber(42);
Expand Down