Skip to content

Commit

Permalink
Add default ctor and copying/moving to union, closes #770
Browse files Browse the repository at this point in the history
  • Loading branch information
@hsutter
hsutter committed Oct 23, 2023
1 parent f41e325 commit 083c8a0
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 14 deletions.
87 changes: 80 additions & 7 deletions regression-tests/test-results/pure2-union.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,12 @@ public: auto set_num(cpp2::in<cpp2::i32> _value) & -> void;
public: auto set_num(auto&& ..._args) & -> void;
private: auto _destroy() & -> void;
public: ~name_or_number() noexcept;
public: explicit name_or_number();
public: name_or_number(name_or_number const& that);

public: name_or_number() = default;
public: name_or_number(name_or_number const&) = delete; /* No 'that' constructor, suppress copy */
public: auto operator=(name_or_number const&) -> void = delete;

public: auto operator=(name_or_number const& that) -> name_or_number& ;
public: name_or_number(name_or_number&& that) noexcept;
public: auto operator=(name_or_number&& that) noexcept -> name_or_number& ;

#line 5 "pure2-union.cpp2"
};
Expand All @@ -59,10 +60,12 @@ public: auto set_other(cpp2::in<T> _value) & -> void;
public: auto set_other(auto&& ..._args) & -> void;
private: auto _destroy() & -> void;
public: ~name_or_other() noexcept;
public: explicit name_or_other();
public: name_or_other(name_or_other const& that);
public: auto operator=(name_or_other const& that) -> name_or_other& ;
public: name_or_other(name_or_other&& that) noexcept;
public: auto operator=(name_or_other&& that) noexcept -> name_or_other& ;

public: name_or_other() = default;
public: name_or_other(name_or_other const&) = delete; /* No 'that' constructor, suppress copy */
public: auto operator=(name_or_other const&) -> void = delete;
#line 17 "pure2-union.cpp2"
};

Expand Down Expand Up @@ -98,6 +101,41 @@ auto name_or_number::_destroy() & -> void{
}

name_or_number::~name_or_number() noexcept{_destroy();}
name_or_number::name_or_number()
: _discriminator{ -1 }{}
name_or_number::name_or_number(name_or_number const& that)
: _storage{ that._storage }

This comment has been minimized.

Sign in to view

Copy link
@JohelEGP

JohelEGP Oct 23, 2023

Contributor

Now there's a redundant initialization/assignment of _storage.

This comment has been minimized.

Sign in to view

Copy link
@gregmarr

gregmarr Oct 23, 2023

Contributor

and also _discriminator is double-set. This also happens in the next three functions.
, _discriminator{ that._discriminator }{
if (CPP2_UFCS_0(is_name, that)) {set_name(CPP2_UFCS_0(name, that));}
if (CPP2_UFCS_0(is_num, that)) {set_num(CPP2_UFCS_0(num, that));}
_discriminator = that._discriminator;
}

auto name_or_number::operator=(name_or_number const& that) -> name_or_number& {
_storage = that._storage;
_discriminator = that._discriminator;
if (CPP2_UFCS_0(is_name, that)) {set_name(CPP2_UFCS_0(name, that));}
if (CPP2_UFCS_0(is_num, that)) {set_num(CPP2_UFCS_0(num, that));}
_discriminator = that._discriminator;
return *this;
}

name_or_number::name_or_number(name_or_number&& that) noexcept
: _storage{ std::move(that)._storage }
, _discriminator{ std::move(that)._discriminator }{
if (CPP2_UFCS_0(is_name, std::move(that))) {set_name(CPP2_UFCS_0(name, std::move(that)));}
if (CPP2_UFCS_0(is_num, std::move(that))) {set_num(CPP2_UFCS_0(num, std::move(that)));}
_discriminator = std::move(that)._discriminator;
}

auto name_or_number::operator=(name_or_number&& that) noexcept -> name_or_number& {
_storage = std::move(that)._storage;
_discriminator = std::move(that)._discriminator;
if (CPP2_UFCS_0(is_name, std::move(that))) {set_name(CPP2_UFCS_0(name, std::move(that)));}
if (CPP2_UFCS_0(is_num, std::move(that))) {set_num(CPP2_UFCS_0(num, std::move(that)));}
_discriminator = std::move(that)._discriminator;
return *this;
}
#line 12 "pure2-union.cpp2"
template <typename T> [[nodiscard]] auto name_or_other<T>::to_string() const& -> std::string{
if (is_name()) { return name(); }
Expand Down Expand Up @@ -128,7 +166,42 @@ template <typename T> auto name_or_other<T>::_destroy() & -> void{
}

template <typename T> name_or_other<T>::~name_or_other() noexcept{_destroy();}
template <typename T> name_or_other<T>::name_or_other()
: _discriminator{ -1 }{}
template <typename T> name_or_other<T>::name_or_other(name_or_other const& that)
: _storage{ that._storage }
, _discriminator{ that._discriminator }{
if (CPP2_UFCS_0(is_name, that)) {set_name(CPP2_UFCS_0(name, that));}
if (CPP2_UFCS_0(is_other, that)) {set_other(CPP2_UFCS_0(other, that));}
_discriminator = that._discriminator;
}


template <typename T> auto name_or_other<T>::operator=(name_or_other const& that) -> name_or_other& {
_storage = that._storage;
_discriminator = that._discriminator;
if (CPP2_UFCS_0(is_name, that)) {set_name(CPP2_UFCS_0(name, that));}
if (CPP2_UFCS_0(is_other, that)) {set_other(CPP2_UFCS_0(other, that));}
_discriminator = that._discriminator;
return *this;
}

template <typename T> name_or_other<T>::name_or_other(name_or_other&& that) noexcept
: _storage{ std::move(that)._storage }
, _discriminator{ std::move(that)._discriminator }{
if (CPP2_UFCS_0(is_name, std::move(that))) {set_name(CPP2_UFCS_0(name, std::move(that)));}
if (CPP2_UFCS_0(is_other, std::move(that))) {set_other(CPP2_UFCS_0(other, std::move(that)));}
_discriminator = std::move(that)._discriminator;
}

template <typename T> auto name_or_other<T>::operator=(name_or_other&& that) noexcept -> name_or_other& {
_storage = std::move(that)._storage;
_discriminator = std::move(that)._discriminator;
if (CPP2_UFCS_0(is_name, std::move(that))) {set_name(CPP2_UFCS_0(name, std::move(that)));}
if (CPP2_UFCS_0(is_other, std::move(that))) {set_other(CPP2_UFCS_0(other, std::move(that)));}
_discriminator = std::move(that)._discriminator;
return *this;
}
#line 19 "pure2-union.cpp2"
auto print_name(cpp2::in<name_or_number> non) -> void{
if (CPP2_UFCS_0(is_name, non)) {
Expand Down
35 changes: 28 additions & 7 deletions source/reflect.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class alias_declaration;
#line 841 "reflect.h2"
class value_member_info;

#line 1311 "reflect.h2"
#line 1327 "reflect.h2"
}

}
Expand Down Expand Up @@ -699,14 +699,14 @@ auto flag_enum(meta::type_declaration& t) -> void;

auto cpp2_union(meta::type_declaration& t) -> void;

#line 1197 "reflect.h2"
#line 1213 "reflect.h2"
//-----------------------------------------------------------------------
//
// print - output a pretty-printed visualization of t
//
auto print(cpp2::in<meta::type_declaration> t) -> void;

#line 1207 "reflect.h2"
#line 1223 "reflect.h2"
//-----------------------------------------------------------------------
//
// apply_metafunctions
Expand All @@ -717,7 +717,7 @@ auto print(cpp2::in<meta::type_declaration> t) -> void;
auto const& error
) -> bool;

#line 1311 "reflect.h2"
#line 1327 "reflect.h2"
}

}
Expand Down Expand Up @@ -1681,15 +1681,36 @@ std::string destroy = " private _destroy: (inout this) = {\n";
// Add the destructor
#line 1193 "reflect.h2"
CPP2_UFCS(add_member, t, " operator=: (move this) = { _destroy(); } ");

// Add default constructor
CPP2_UFCS(add_member, t, " operator=: (out this) = { _discriminator = -1; } ");
{
std::string value_set = " operator=: (out this, that) = {\n";

// Add value-set

#line 1200 "reflect.h2"
{
for (
auto const& a : alternatives ) {
value_set += " if that.is_" + cpp2::to_string(a.name) + "() { set_" + cpp2::to_string(a.name) + "( that." + cpp2::to_string(a.name) + "() ); }\n";
}

value_set += " _discriminator = that._discriminator;\n";
value_set += " }\n";
CPP2_UFCS(add_member, t, std::move(value_set));
}
}
#line 1210 "reflect.h2"
}

#line 1201 "reflect.h2"
#line 1217 "reflect.h2"
auto print(cpp2::in<meta::type_declaration> t) -> void
{
std::cout << CPP2_UFCS_0(print, t) << "\n";
}

#line 1211 "reflect.h2"
#line 1227 "reflect.h2"
[[nodiscard]] auto apply_metafunctions(
declaration_node& n,
type_declaration& rtype,
Expand Down Expand Up @@ -1789,7 +1810,7 @@ auto print(cpp2::in<meta::type_declaration> t) -> void
return true;
}

#line 1311 "reflect.h2"
#line 1327 "reflect.h2"
}

}
Expand Down
16 changes: 16 additions & 0 deletions source/reflect.h2
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1191,6 +1191,22 @@ union: (inout t : meta::type_declaration)

// Add the destructor
t.add_member( " operator=: (move this) = { _destroy(); } " );

// Add default constructor
t.add_member( " operator=: (out this) = { _discriminator = -1; } " );

// Add value-set
(copy value_set: std::string = " operator=: (out this, that) = {\n")
{
for alternatives
do (a) {
value_set += " if that.is_(a.name)$() { set_(a.name)$( that.(a.name)$() ); }\n";
}

value_set += " _discriminator = that._discriminator;\n";

This comment has been minimized.

Sign in to view

Copy link
@JohelEGP

JohelEGP Oct 23, 2023

Contributor

This should be redundant, given that the set_ function sets it.

This comment has been minimized.

Sign in to view

Copy link
@gregmarr

gregmarr Oct 23, 2023

Contributor

given that the set_ function sets it.

Unless it's currently valueless.

This comment has been minimized.

Sign in to view

Copy link
@JohelEGP

JohelEGP Oct 23, 2023

Contributor

You're right.
But there's still a bug.
If that is the default-initialized value,
destroy() is never called.

This comment has been minimized.

Sign in to view

Copy link
@gregmarr

gregmarr Oct 23, 2023

Contributor

It's not redundant if only the set_ functions are used. It's redundant, along with the set calls, if memberwise setting happens first. I hadn't seen that redundancy yet when I wrote this. :)

This comment has been minimized.

Sign in to view

Copy link
@JohelEGP

JohelEGP Oct 23, 2023

Contributor

Opened #776 for this.
value_set += " }\n";
t.add_member( value_set );
}
}


Expand Down

0 comments on commit 083c8a0

Please sign in to comment.