C++: Fix code generation for union field types that are constructed with allocators

src/nunavut/lang/_config.py

@@ -34,7 +34,7 @@ def parse_string(s: str) -> typing.Optional[typing.Any]:  # annoying mypy cheat
 
 class SpecialMethod(Enum):
     """
-    Enum used in the Jinja templates to differentiate different kinds of constructrors
+    Enum used in the Jinja templates to differentiate different kinds of constructors
     """
 
     AllocatorConstructor = auto()
@@ -50,6 +50,18 @@ class SpecialMethod(Enum):
     """ Move constructor that also takes an allocator argument """
 
 
+class CompositeSubType(Enum):
+    """
+    Enum used in the Jinja templates to designate how fields are contained in a composite type
+    """
+
+    Structure = auto()
+    """ Object contains a set of sequential fields """
+
+    Union = auto()
+    """ Object contains one field which may hold any value from a set of types """
+
+
 class LanguageConfig:
     """
     Configuration storage encapsulating parsers and other configuration format details. For any configuration type used

src/nunavut/lang/cpp/__init__.py

@@ -28,7 +28,7 @@
 from nunavut._utilities import YesNoDefault
 from nunavut.jinja.environment import Environment
 from nunavut.lang._common import IncludeGenerator, TokenEncoder, UniqueNameGenerator
-from nunavut.lang._config import ConstructorConvention, SpecialMethod
+from nunavut.lang._config import ConstructorConvention, SpecialMethod, CompositeSubType
 from nunavut.lang._language import Language as BaseLanguage
 from nunavut.lang.c import _CFit
 from nunavut.lang.c import filter_literal as c_filter_literal
@@ -155,6 +155,7 @@ def _add_additional_globals(self, globals_map: typing.Dict[str, typing.Any]) ->
         """
         globals_map["ConstructorConvention"] = ConstructorConvention
         globals_map["SpecialMethod"] = SpecialMethod
+        globals_map["CompositeSubType"] = CompositeSubType
 
     def get_includes(self, dep_types: Dependencies) -> typing.List[str]:
         """
@@ -967,7 +968,7 @@ def needs_rhs(special_method: SpecialMethod) -> bool:
 
 
 def needs_allocator(instance: pydsdl.Any) -> bool:
-    """Helper method used by filter_value_initializer()"""
+    """Helper method used by filter_value_initializer() and filter_needs_allocator()"""
     return isinstance(instance.data_type, pydsdl.VariableLengthArrayType) or isinstance(
         instance.data_type, pydsdl.CompositeType
     )
@@ -980,6 +981,10 @@ def needs_vla_init_args(instance: pydsdl.Any, special_method: SpecialMethod) ->
     )
 
 
+def needs_variant_init_args(composite_subtype: CompositeSubType) -> bool:
+    return composite_subtype == CompositeSubType.Union
+
+
 def needs_move(special_method: SpecialMethod) -> bool:
     """Helper method used by filter_value_initializer()"""
     return special_method == SpecialMethod.MoveConstructorWithAllocator
@@ -994,12 +999,45 @@ def requires_initialization(instance: pydsdl.Any) -> bool:
     )
 
 
-def assemble_initializer_expression(
-    wrap: str, rhs: str, leading_args: typing.List[str], trailing_args: typing.List[str]
-) -> str:
+def prepare_initializer_args(
+    language: Language,
+    instance: pydsdl.Any,
+    special_method: SpecialMethod,
+    composite_subtype: CompositeSubType,
+) -> typing.Tuple[typing.List[str], str, typing.List[str]]:
+    rhs: str = ""
+    leading_args: typing.List[str] = []
+    trailing_args: typing.List[str] = []
+    if needs_variant_init_args(composite_subtype):
+        leading_args.append(
+            f"nunavut::support::in_place_index_t<VariantType::IndexOf::{language.filter_id(instance)}>{{}}"
+        )
+
+    if needs_initializing_value(special_method):
+        instance_id = language.filter_id(instance)
+        if needs_rhs(special_method):
+            rhs = "rhs."
+        rhs += f"get_{instance_id}()" if composite_subtype is CompositeSubType.Union else instance_id
+
+    if needs_vla_init_args(instance, special_method):
+        constructor_args = language.get_option("variable_array_type_constructor_args")
+        if isinstance(constructor_args, str) and len(constructor_args) > 0:
+            trailing_args.append(constructor_args.format(MAX_SIZE=instance.data_type.capacity))
+
+    if needs_allocator(instance):
+        if language.get_option("ctor_convention") == ConstructorConvention.UsesLeadingAllocator.value:
+            leading_args.extend(["std::allocator_arg", "allocator"])
+        else:
+            trailing_args.append("allocator")
+
+    if needs_move(special_method):
+        rhs = "std::move({})".format(rhs)
+
+    return (leading_args, rhs, trailing_args)
+
+
+def assemble_initializer_expression(rhs: str, leading_args: typing.List[str], trailing_args: typing.List[str]) -> str:
     """Helper method used by filter_value_initializer()"""
-    if wrap:
-        rhs = "{}({})".format(wrap, rhs)
     args = []
     if rhs:
         args.append(rhs)
@@ -1008,40 +1046,34 @@ def assemble_initializer_expression(
 
 
 @template_language_filter(__name__)
-def filter_value_initializer(language: Language, instance: pydsdl.Any, special_method: SpecialMethod) -> str:
+def filter_value_initializer(
+    language: Language,
+    instance: pydsdl.Any,
+    special_method: SpecialMethod,
+    composite_subtype: CompositeSubType = CompositeSubType.Structure,
+) -> str:
     """
     Emit an initialization expression for a C++ special method.
     """
 
     value_initializer: str = ""
     if requires_initialization(instance):
-        wrap: str = ""
         rhs: str = ""
         leading_args: typing.List[str] = []
         trailing_args: typing.List[str] = []
 
-        if needs_initializing_value(special_method):
-            if needs_rhs(special_method):
-                rhs = "rhs."
-            rhs += language.filter_id(instance)
-
-        if needs_vla_init_args(instance, special_method):
-            constructor_args = language.get_option("variable_array_type_constructor_args")
-            if isinstance(constructor_args, str) and len(constructor_args) > 0:
-                trailing_args.append(constructor_args.format(MAX_SIZE=instance.data_type.capacity))
-
-        if needs_allocator(instance):
-            if language.get_option("ctor_convention") == ConstructorConvention.UsesLeadingAllocator.value:
-                leading_args.extend(["std::allocator_arg", "allocator"])
-            else:
-                trailing_args.append("allocator")
+        leading_args, rhs, trailing_args = prepare_initializer_args(
+            language, instance, special_method, composite_subtype
+        )
+        value_initializer = assemble_initializer_expression(rhs, leading_args, trailing_args)
 
-        if needs_move(special_method):
-            wrap = "std::move"
+    return value_initializer
 
-        value_initializer = assemble_initializer_expression(wrap, rhs, leading_args, trailing_args)
 
-    return value_initializer
+@template_language_test(__name__)
+def filter_needs_allocator(language: Language, instance: pydsdl.Any) -> bool:
+    """Emit a boolean value for whether the instance's type needs an allocator or not"""
+    return needs_allocator(instance)
 
 
 @template_language_filter(__name__)

src/nunavut/lang/cpp/support/utility.j2

@@ -0,0 +1,39 @@
+// OpenCyphal common union composite type support routines
+//
+// AUTOGENERATED, DO NOT EDIT.
+//
+//---------------------------------------------------------------------------------------------------------------------
+// Language Options
+{% for key, value in options.items() -%}
+//     {{ key }}:  {{ value }}
+{% endfor %}
+
+#ifndef NUNAVUT_SUPPORT_UTILITIES_HPP_INCLUDED
+#define NUNAVUT_SUPPORT_UTILITIES_HPP_INCLUDED
+
+{% ifuses "std_variant" -%}
+#include <utility>
+{%- else -%}
+#include <cstdint>
+{%- endifuses %}
+
+namespace nunavut
+{
+namespace support
+{
+
+// Value-specialized type for template instantiation
+template<std::size_t I>
+{% ifuses "std_variant" -%}
+using in_place_index_t = std::in_place_index_t<I>;
+{%- else -%}
+struct in_place_index_t
+{
+    explicit in_place_index_t() = default;
+};
+{%- endifuses %}
+
+} // end namespace support
+} // end namespace nunavut
+
+#endif // NUNAVUT_SUPPORT_UTILITIES_HPP_INCLUDED

src/nunavut/lang/cpp/templates/_composite_type.j2

@@ -84,6 +84,14 @@ struct {% if composite_type.deprecated -%}
     {%- endif %}
 {%- endfor %}
     };
+{% if composite_type.inner_type is UnionType -%}
+{%- ifuses "std_variant" -%}
+{% include '_fields_as_variant.j2' %}
+{%- else -%}
+{% include '_fields_as_union.j2' %}
+{%- endifuses -%}
+{%- endif -%}
+
 {% if options.ctor_convention != ConstructorConvention.Default.value %}
     {%- if options.allocator_is_default_constructible %}
     // Default constructor
@@ -102,19 +110,26 @@ struct {% if composite_type.deprecated -%}
 
     // Allocator constructor
     explicit {{composite_type|short_reference_name}}(const allocator_type& allocator)
-    {%- if composite_type.fields_except_padding %} :{% endif %}
+    {%- if composite_type.fields_except_padding %} :
     {%- if composite_type.inner_type is UnionType %}
-        union_value{} // can't make use of the allocator with a union
+        union_value{{ composite_type.fields_except_padding[0] | value_initializer(SpecialMethod.AllocatorConstructor, CompositeSubType.Union) }}
     {%- else %}
     {%- for field in composite_type.fields_except_padding %}
         {{ field | id }}{{ field | value_initializer(SpecialMethod.AllocatorConstructor) }}{%if not loop.last %},{%endif %}
     {%- endfor %}
     {%- endif %}
+    {%- endif %}
     {
         (void)allocator; // avoid unused param warning
     }
 
-    {%- if composite_type.inner_type is not UnionType %}
+    {% if composite_type.inner_type is UnionType -%}
+    // Initializing constructor
+    template<std::size_t I, typename... Args>
+    {{composite_type|short_reference_name}}(nunavut::support::in_place_index_t<I> i, Args&&... args)
+        : union_value{i, std::forward<Args>(args)...}
+    {}
+    {%- else %}
     {% if composite_type.fields_except_padding %}
     // Initializing constructor
     {{ composite_type | explicit_decorator(SpecialMethod.InitializingConstructorWithAllocator)}}(
@@ -140,15 +155,26 @@ struct {% if composite_type.deprecated -%}
     {{composite_type|short_reference_name}}(const {{composite_type|short_reference_name}}& rhs, const allocator_type& allocator)
     {%- if composite_type.fields_except_padding %} :{% endif %}
     {%- if composite_type.inner_type is UnionType %}
-        union_value{rhs.union_value}  // can't make use of the allocator with a union
+        union_value{std::move(
+    {%- set ns = namespace(indent = "") %}
+    {%- for field in composite_type.fields_except_padding %}
+    {%- if not loop.last %}
+            {{ns.indent}}rhs.is_{{ field | id }}() ?
+    {%- set ns.indent = ns.indent ~ "\t" %}
+    {%- endif %}
+            {{ns.indent}}VariantType
+            {{- field | value_initializer(SpecialMethod.CopyConstructorWithAllocator, CompositeSubType.Union) }}
+    {%- if not loop.last %} :{% endif %}
+    {%- endfor %}
+        )}
     {%- else %}
     {%- for field in composite_type.fields_except_padding %}
         {{ field | id }}{{ field | value_initializer(SpecialMethod.CopyConstructorWithAllocator) }}{%if not loop.last %},{%endif %}
     {%- endfor %}
-    {% endif %}
+    {%- endif %}
     {
-        (void)rhs;       // avoid unused param warning
-        (void)allocator; // avoid unused param warning
+        (void)rhs;          // avoid unused param warning
+        (void)allocator;    // avoid unused param warning
     }
 
     // Move constructor
@@ -158,15 +184,26 @@ struct {% if composite_type.deprecated -%}
     {{composite_type|short_reference_name}}({{composite_type|short_reference_name}}&& rhs, const allocator_type& allocator)
     {%- if composite_type.fields_except_padding %} :{% endif %}
     {%- if composite_type.inner_type is UnionType %}
-        union_value{} // can't make use of the allocator with a union
+        union_value{std::move(
+    {%- set ns = namespace(indent = "") %}
+    {%- for field in composite_type.fields_except_padding %}
+    {%- if not loop.last %}
+            {{ns.indent}}rhs.is_{{ field | id }}() ?
+    {%- set ns.indent = ns.indent ~ "\t" %}
+    {%- endif %}
+            {{ns.indent}}VariantType
+            {{- field | value_initializer(SpecialMethod.MoveConstructorWithAllocator, CompositeSubType.Union) }}
+    {%- if not loop.last %} :{% endif %}
+    {%- endfor %}
+        )}
     {%- else %}
     {%- for field in composite_type.fields_except_padding %}
         {{ field | id }}{{ field | value_initializer(SpecialMethod.MoveConstructorWithAllocator) }}{%if not loop.last %},{%endif %}
     {%- endfor %}
     {%- endif %}
     {
-        (void)rhs;       // avoid unused param warning
-        (void)allocator; // avoid unused param warning
+        (void)rhs;          // avoid unused param warning
+        (void)allocator;    // avoid unused param warning
     }
 
     // Copy assignment
@@ -177,7 +214,7 @@ struct {% if composite_type.deprecated -%}
 
     // Destructor
     ~{{composite_type|short_reference_name}}() = default;
-{%- endif %}
+{% endif %}
 
 {%- for constant in composite_type.constants %}
     {% if loop.first %}
@@ -189,11 +226,6 @@ struct {% if composite_type.deprecated -%}
     static constexpr {{ constant.data_type | declaration }} {{ constant.name | id }} = {{ constant | constant_value }};
 {%- endfor -%}
 {%- if composite_type.inner_type is UnionType -%}
-{%- ifuses "std_variant" -%}
-{% include '_fields_as_variant.j2' %}
-{%- else -%}
-{% include '_fields_as_union.j2' %}
-{%- endifuses -%}
 {%- for field in composite_type.fields_except_padding %}
     bool is_{{field.name|id}}() const {
         return VariantType::IndexOf::{{field.name|id}} == union_value.index();
@@ -219,9 +251,10 @@ struct {% if composite_type.deprecated -%}
 
     template<class... Args> typename std::add_lvalue_reference<_traits_::TypeOf::{{field.name|id}}>::type
     set_{{field.name|id}}(Args&&...v){
-        return union_value.emplace<VariantType::IndexOf::{{field.name|id}}>(v...);
+        return union_value.emplace<VariantType::IndexOf::{{field.name|id}}>(std::forward<Args>(v)...);
     }
-{%- endfor %}
+{% endfor %}
+    VariantType union_value;
 {%- else -%}
 {% include '_fields.j2' %}
 {%- endif %}