C++: use references for non-trivial types in accessors (#1607)

* Extend smoke tests for accessors C++ attribute
* Cpp: use references for non-trivial types in accessors
* Implement functional test for generated C++ accessors

The generated accessors used by-value return and parameter
types even for non-trivial types. This change is intended
to improve the situation by introducing usage of reference
for the mentioned non-trivial types.

This merge request:
 - introduces usage of const l-references for non-trivial types
    in getters defined via accessors attribute for structures
    when it is called on l-value
 - introduces usage of r-references for non-trivial types
    in getters defined via accessors attribute for structures
    when it is called on r-value
 - introduces usage of references for non-trivial types in setters
   (two overloads are provided -- const lvalue ref and rvalue ref)
 - strips constness and references from type used to select a hash
    in 'StructHashImpl.mustache' because std::hash does not
    accept references/constness for T
 - updates output of smoke tests to show, that since this commit
    the generated accessors use const references for non-trivial
    types
 - implements a new functional tests for C++ accessors

---------

Signed-off-by: Patryk Wrobel <[email protected]>

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Unreleased
 ### Bug fixes:
+ * C++: Fixed generation of getters and setters for accessors attribute -- non-trivial types use references now.
  * Added missing generation of conversion functions for lambdas defined in structs for Swift.
  * Fixed a bug related to exporting nested types defined in a type annotated as internal.
 

functional-tests/functional/CMakeLists.txt

@@ -1,4 +1,4 @@
-# Copyright (C) 2016-2019 HERE Europe B.V.
+# Copyright (C) 2016-2024 HERE Europe B.V.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -135,6 +135,7 @@ feature(Structs cpp android swift dart SOURCES
     input/src/cpp/PlainDataStructures.cpp
 
     input/lime/Structs.lime
+    input/lime/Accessors.lime
 )
 
 feature(StructsInTypes cpp android swift dart SOURCES

functional-tests/functional/cpp/CMakeLists.txt

@@ -1,4 +1,4 @@
-# Copyright (C) 2016-2019 HERE Europe B.V.
+# Copyright (C) 2016-2024 HERE Europe B.V.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -51,6 +51,7 @@ add_subdirectory(${CMAKE_CURRENT_BINARY_DIR}/googletest-src
                  EXCLUDE_FROM_ALL)
 
 set(SOURCES
+  tests/AccessorsTest.cpp
   tests/EnumTest.cpp
   tests/EquatableTest.cpp
   tests/FieldConstructorsTest.cpp

functional-tests/functional/cpp/tests/AccessorsTest.cpp

@@ -0,0 +1,195 @@
+// -------------------------------------------------------------------------------------------------
+// Copyright (C) 2024 HERE Europe B.V.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+// SPDX-License-Identifier: Apache-2.0
+// License-Filename: LICENSE
+//
+// -------------------------------------------------------------------------------------------------
+
+#include "test/SomeMutableStructWithCppAccessors.h"
+
+#include <gmock/gmock.h>
+
+#include <cstdint>
+#include <string>
+#include <type_traits>
+#include <vector>
+
+namespace test
+{
+
+using namespace ::testing;
+using namespace test;
+
+namespace {
+
+using TrivialInt = int32_t;
+using TrivialDouble = double;
+using NontrivialString = std::string;
+using NontrivialList = std::vector<std::string>;
+
+SomeMutableStructWithCppAccessors get_struct() {
+    return SomeMutableStructWithCppAccessors {
+        TrivialInt{32},
+        TrivialDouble{27.5},
+        NontrivialString{"Some string"},
+        NontrivialList{{"S1", "S2", "S3"}}
+    };
+}
+
+const SomeMutableStructWithCppAccessors get_const_struct() {
+    return SomeMutableStructWithCppAccessors {
+        TrivialInt{64},
+        TrivialDouble{52.3},
+        NontrivialString{"Another string"},
+        NontrivialList{{"A1", "A2", "A3"}}
+    };
+}
+
+} // namespace
+
+TEST( AccessorsOfMutableStruct, GettingFieldsOnLvalue )
+{
+    // Given an object of mutable struct with accessors.
+    SomeMutableStructWithCppAccessors object = get_struct();
+
+    // When using getters on lvalue.
+    const auto& int_field = object.get_trivial_int_field();
+    const auto& double_field = object.get_trivial_double_field();
+    const auto& string_field = object.get_nontrivial_string_field();
+    const auto& list_field = object.get_nontrivial_list_field();
+
+    // Then access can be made.
+    EXPECT_EQ(TrivialInt{32}, int_field);
+    EXPECT_DOUBLE_EQ(TrivialDouble{27.5}, double_field);
+    EXPECT_EQ(NontrivialString{"Some string"}, string_field);
+
+    const NontrivialList expected_list{"S1", "S2", "S3"};
+    EXPECT_EQ(expected_list, list_field);
+}
+
+TEST( AccessorsOfMutableStruct, GettingFieldsOnConstLvalue )
+{
+    // Given const reference to object of mutable struct with accessors.
+    const auto& object = get_const_struct();
+
+    // When using getters on const lvalue.
+    const auto& int_field = object.get_trivial_int_field();
+    const auto& double_field = object.get_trivial_double_field();
+    const auto& string_field = object.get_nontrivial_string_field();
+    const auto& list_field = object.get_nontrivial_list_field();
+
+    // Then access can be made.
+    EXPECT_EQ(TrivialInt{64}, int_field);
+    EXPECT_DOUBLE_EQ(TrivialDouble{52.3}, double_field);
+    EXPECT_EQ(NontrivialString{"Another string"}, string_field);
+
+    const NontrivialList expected_list{"A1", "A2", "A3"};
+    EXPECT_EQ(expected_list, list_field);
+}
+
+TEST( AccessorsOfMutableStruct, GettingFieldsOnRvalue )
+{
+    // When using getter on rvalue.
+    auto int_field = get_struct().get_trivial_int_field();
+    auto double_field = get_struct().get_trivial_double_field();
+    auto string_field = get_struct().get_nontrivial_string_field();
+    auto list_field = get_struct().get_nontrivial_list_field();
+
+    // Then access can be made.
+    EXPECT_EQ(TrivialInt{32}, int_field);
+    EXPECT_DOUBLE_EQ(TrivialDouble{27.5}, double_field);
+    EXPECT_EQ(NontrivialString{"Some string"}, string_field);
+
+    const NontrivialList expected_list{"S1", "S2", "S3"};
+    EXPECT_EQ(expected_list, list_field);
+}
+
+TEST( AccessorsOfMutableStruct, GettingFieldsOnConstRvalue )
+{
+    // When using getters on const rvalue.
+    auto int_field = get_const_struct().get_trivial_int_field();
+    auto double_field = get_const_struct().get_trivial_double_field();
+    auto string_field = get_const_struct().get_nontrivial_string_field();
+    auto list_field = get_const_struct().get_nontrivial_list_field();
+
+    // Then access can be made.
+    EXPECT_EQ(TrivialInt{64}, int_field);
+    EXPECT_DOUBLE_EQ(TrivialDouble{52.3}, double_field);
+    EXPECT_EQ(NontrivialString{"Another string"}, string_field);
+
+    const NontrivialList expected_list{"A1", "A2", "A3"};
+    EXPECT_EQ(expected_list, list_field);
+}
+
+TEST( AccessorsOfMutableStruct, PreconditionsForNontrivialGetters )
+{
+    // String getters.
+    using LvalueStringGetterRetT = decltype(std::declval<SomeMutableStructWithCppAccessors&>().get_nontrivial_string_field());
+    static_assert(std::is_same_v<const std::string&, LvalueStringGetterRetT>, "String getter shall return const ref when called on L-value");
+
+    using ConstLvalueStringGetterRetT = decltype(std::declval<const SomeMutableStructWithCppAccessors&>().get_nontrivial_string_field());
+    static_assert(std::is_same_v<const std::string&, ConstLvalueStringGetterRetT>, "String getter shall return const ref when called on const L-value");
+
+    using RvalueStringGetterRetT = decltype(std::declval<SomeMutableStructWithCppAccessors&&>().get_nontrivial_string_field());
+    static_assert(std::is_same_v<std::string&&, RvalueStringGetterRetT>, "String getter shall return r-ref when called on R-value");
+
+    using ConstRvalueStringGetterRetT = decltype(std::declval<const SomeMutableStructWithCppAccessors&&>().get_nontrivial_string_field());
+    static_assert(std::is_same_v<const std::string&&, ConstRvalueStringGetterRetT>, "String getter shall return const r-ref when called on const R-value");
+
+    // List getters.
+    using LvalueListGetterRetT = decltype(std::declval<SomeMutableStructWithCppAccessors&>().get_nontrivial_list_field());
+    static_assert(std::is_same_v<const std::vector<std::string>&, LvalueListGetterRetT>, "List getter shall return const l-ref when called on L-value");
+
+    using ConstLvalueListGetterRetT = decltype(std::declval<const SomeMutableStructWithCppAccessors&>().get_nontrivial_list_field());
+    static_assert(std::is_same_v<const std::vector<std::string>&, ConstLvalueListGetterRetT>, "List getter shall return const l-ref when called on const L-value");
+
+    using RvalueListGetterRetT = decltype(std::declval<SomeMutableStructWithCppAccessors&&>().get_nontrivial_list_field());
+    static_assert(std::is_same_v<std::vector<std::string>&&, RvalueListGetterRetT>, "List getter shall return r-ref when called on R-value");
+
+    using ConstRvalueListGetterRetT = decltype(std::declval<const SomeMutableStructWithCppAccessors&&>().get_nontrivial_list_field());
+    static_assert(std::is_same_v<const std::vector<std::string>&&, ConstRvalueListGetterRetT>, "List getter shall return const r-ref when called on const R-value");
+}
+
+TEST( AccessorsOfMutableStruct, PreconditionsForTrivialGetters )
+{
+    // Int getter.
+    using LvalueIntGetterRetT = decltype(std::declval<SomeMutableStructWithCppAccessors&>().get_trivial_int_field());
+    static_assert(std::is_same_v<int, LvalueIntGetterRetT>, "int getter shall always return by-value");
+
+    using ConstLvalueIntGetterRetT = decltype(std::declval<const SomeMutableStructWithCppAccessors&>().get_trivial_int_field());
+    static_assert(std::is_same_v<int, ConstLvalueIntGetterRetT>, "int getter shall always return by-value");
+
+    using RvalueIntGetterRetT = decltype(std::declval<SomeMutableStructWithCppAccessors&&>().get_trivial_int_field());
+    static_assert(std::is_same_v<int, RvalueIntGetterRetT>, "int getter shall always return by-value");
+
+    using ConstRvalueIntGetterRetT = decltype(std::declval<const SomeMutableStructWithCppAccessors&&>().get_trivial_int_field());
+    static_assert(std::is_same_v<int, ConstRvalueIntGetterRetT>, "int getter shall always return by-value");
+
+    // Double getter.
+    using LvalueDoubleGetterRetT = decltype(std::declval<SomeMutableStructWithCppAccessors&>().get_trivial_double_field());
+    static_assert(std::is_same_v<double, LvalueDoubleGetterRetT>, "double getter shall always return by-value");
+
+    using ConstLvalueDoubleGetterRetT = decltype(std::declval<const SomeMutableStructWithCppAccessors&>().get_trivial_double_field());
+    static_assert(std::is_same_v<double, ConstLvalueDoubleGetterRetT>, "double getter shall always return by-value");
+
+    using RvalueDoubleGetterRetT = decltype(std::declval<SomeMutableStructWithCppAccessors&&>().get_trivial_double_field());
+    static_assert(std::is_same_v<double, RvalueDoubleGetterRetT>, "double getter shall always return by-value");
+
+    using ConstRvalueDoubleGetterRetT = decltype(std::declval<const SomeMutableStructWithCppAccessors&&>().get_trivial_double_field());
+    static_assert(std::is_same_v<double, ConstRvalueDoubleGetterRetT>, "double getter shall always return by-value");
+}
+
+}  // test

functional-tests/functional/input/lime/Accessors.lime

@@ -0,0 +1,27 @@
+# Copyright (C) 2024 HERE Europe B.V.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# SPDX-License-Identifier: Apache-2.0
+# License-Filename: LICENSE
+
+package test
+
+@Cpp(Accessors)
+struct SomeMutableStructWithCppAccessors {
+    trivialIntField: Int
+    trivialDoubleField: Double
+
+    nontrivialStringField: String
+    nontrivialListField: List<String>
+}

gluecodium/src/main/resources/templates/cpp/CppStruct.mustache

@@ -1,6 +1,6 @@
 {{!!
   !
-  ! Copyright (C) 2016-2020 HERE Europe B.V.
+  ! Copyright (C) 2016-2024 HERE Europe B.V.
   !
   ! Licensed under the Apache License, Version 2.0 (the "License");
   ! you may not use this file except in compliance with the License.
@@ -51,10 +51,21 @@ public:
 }}{{#if attributes.cpp.accessors}}
 {{#set structElement=this}}{{#fields}}
 {{prefixPartial "cpp/CppFieldDoc" "    "}}
-    {{resolveName typeRef}} {{resolveName this "" "getter"}}( ) const { return {{resolveName}}; }{{!!
+    {{#ifPredicate typeRef "needsRefSuffix"}}const {{resolveName typeRef}}&{{!!
+}} {{resolveName this "" "getter"}}( ) const & { return {{resolveName}}; }
+    {{resolveName typeRef}}&& {{resolveName this "" "getter"}}( ) && { return std::move({{resolveName}}); }
+    const {{resolveName typeRef}}&& {{resolveName this "" "getter"}}( ) const && { return std::move({{resolveName}}); }{{/ifPredicate}}{{!!
+}}{{#unlessPredicate typeRef "needsRefSuffix"}}{{resolveName typeRef}}{{!!
+}} {{resolveName this "" "getter"}}( ) const { return {{resolveName}}; }{{/unlessPredicate}}{{!!
 }}{{#unless structElement.attributes.immutable}}
 {{prefixPartial "cpp/CppFieldDoc" "    "}}
-    void {{resolveName this "" "setter"}}( {{resolveName typeRef}} value_ ) { {{resolveName}} = value_; }{{!!
+{{#ifPredicate typeRef "needsRefSuffix"}}{{!!
+}}    void {{resolveName this "" "setter"}}( const {{resolveName typeRef}}& value_ ) { {{resolveName}} = value_; }
+    void {{resolveName this "" "setter"}}( {{resolveName typeRef}}&& value_ ) { {{resolveName}} = std::move(value_); }{{!!
+}}{{/ifPredicate}}{{!!
+}}{{#unlessPredicate typeRef "needsRefSuffix"}}{{!!
+}}    void {{resolveName this "" "setter"}}( {{resolveName typeRef}} value_ ) { {{resolveName}} = value_; }{{!!
+}}{{/unlessPredicate}}{{!!
 }}{{/unless}}
 {{/fields}}{{/set}}
 

gluecodium/src/main/resources/templates/cpp/StructHashImpl.mustache

@@ -1,6 +1,6 @@
 {{!!
   !
-  ! Copyright (C) 2016-2020 HERE Europe B.V.
+  ! Copyright (C) 2016-2024 HERE Europe B.V.
   !
   ! Licensed under the Apache License, Version 2.0 (the "License");
   ! you may not use this file except in compliance with the License.
@@ -23,8 +23,8 @@ hash< {{resolveName "FQN"}} >::operator( )( const {{resolveName "FQN"}}& t ) con
 {
     size_t hash_value = 43;
 {{#if attributes.cpp.accessors}}
-    {{#resolveName "FQN"}}{{#set structName=this}}{{#fields}}hash_value = (hash_value ^ {{>common/InternalNamespace}}hash< decltype({{!!
-    }}std::declval<{{structName}}>().{{resolveName this "" "getter"}}()) >()(t.{{resolveName this "" "getter"}}())) << 1;
+    {{#resolveName "FQN"}}{{#set structName=this}}{{#fields}}hash_value = (hash_value ^ {{>common/InternalNamespace}}hash< std::remove_cv_t< std::remove_reference_t< decltype({{!!
+    }}std::declval<{{structName}}>().{{resolveName this "" "getter"}}()) > > >()(t.{{resolveName this "" "getter"}}())) << 1;
     {{/fields}}{{/set}}{{/resolveName}}
 {{/if}}{{#unless attributes.cpp.accessors}}
 {{#fields}}

gluecodium/src/test/resources/smoke/equatable/output/cpp/src/smoke/EquatableStructWithAccessors.cpp

@@ -1,33 +1,43 @@
 // -------------------------------------------------------------------------------------------------
 //
+
 //
 // -------------------------------------------------------------------------------------------------
+
 #include "smoke/EquatableStructWithAccessors.h"
 #include <utility>
+
 namespace smoke {
+
 EquatableStructWithAccessors::EquatableStructWithAccessors( )
     : foo_field{ }
 {
 }
+
 EquatableStructWithAccessors::EquatableStructWithAccessors( ::std::string foo_field )
     : foo_field( std::move( foo_field ) )
 {
 }
+
 bool EquatableStructWithAccessors::operator==( const EquatableStructWithAccessors& rhs ) const
 {
     return foo_field == rhs.foo_field;
 }
+
 bool EquatableStructWithAccessors::operator!=( const EquatableStructWithAccessors& rhs ) const
 {
     return !( *this == rhs );
 }
+
+
 }
+
 namespace gluecodium {
 std::size_t
 hash< ::smoke::EquatableStructWithAccessors >::operator( )( const ::smoke::EquatableStructWithAccessors& t ) const
 {
     size_t hash_value = 43;
-    hash_value = (hash_value ^ ::gluecodium::hash< decltype(std::declval<::smoke::EquatableStructWithAccessors>().get_foo_field()) >()(t.get_foo_field())) << 1;
+    hash_value = (hash_value ^ ::gluecodium::hash< std::remove_cv_t< std::remove_reference_t< decltype(std::declval<::smoke::EquatableStructWithAccessors>().get_foo_field()) > > >()(t.get_foo_field())) << 1;
     return hash_value;
 }
 }

gluecodium/src/test/resources/smoke/structs/input/Structs.lime

@@ -1,4 +1,4 @@
-# Copyright (C) 2016-2019 HERE Europe B.V.
+# Copyright (C) 2016-2024 HERE Europe B.V.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -57,11 +57,21 @@ class Structs {
     @Cpp(Accessors)
     @Immutable
     struct ImmutableStructWithCppAccessors {
-        stringField: String
+        trivialIntField: Int
+        trivialDoubleField: Double
+
+        nontrivialStringField: String
+        nontrivialPointField: Point
+        nontrivialOptionalPoint: Point?
     }
     @Cpp(Accessors)
     struct MutableStructWithCppAccessors {
-        stringField: String
+        trivialIntField: Int
+        trivialDoubleField: Double
+
+        nontrivialStringField: String
+        nontrivialPointField: Point
+        nontrivialOptionalPoint: Point?
     }
 
     enum FooBar {