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

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

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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
3 changes: 2 additions & 1 deletion functional-tests/functional/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion functional-tests/functional/cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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
Expand Down
195 changes: 195 additions & 0 deletions functional-tests/functional/cpp/tests/AccessorsTest.cpp
Original file line number Diff line number Diff line change
@@ -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);
}
Comment on lines +103 to +135
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: if these test cases used auto&&, then we would have dangling references.


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
27 changes: 27 additions & 0 deletions functional-tests/functional/input/lime/Accessors.lime
Original file line number Diff line number Diff line change
@@ -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>
}
17 changes: 14 additions & 3 deletions gluecodium/src/main/resources/templates/cpp/CppStruct.mustache
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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}}

Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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}}
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
16 changes: 13 additions & 3 deletions gluecodium/src/test/resources/smoke/structs/input/Structs.lime
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
Loading
Loading