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

python/podio_gen/generator_utils.py

@@ -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:

python/templates/macros/declarations.jinja2

@@ -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 %}
 

python/templates/macros/implementations.jinja2

@@ -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 %}

tests/unittests/unittest.cpp

@@ -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"
@@ -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);