Skip to content

Commit 70e30c1

Browse files
hsutterhsutter
committed
Fix 'operator= memberwise generation from that`
The `operator=` memberwise generation code didn't correctly handle the case of not-explicitly-initialized members where there are no more statements in the function (including all of them in an empty function). And it was a silly mistake -- the main loop was over all objects *and statements*, and ended when we ran out of statements -- and then there was a whole 'nother separate loop for the rest of the objects not initialized yet. When you say something twice, you get it wrong the second time... the separate loop wasn't doing all the work of the main loop. The fix is to jettison that separate loop, make the main loop handle all objects (only) and have a conditional guard for the case where we've run out of explicit statements to try first as the preferred explicit value-set candidate.
1 parent 2724888 commit 70e30c1

File tree

2 files changed

+112
-110
lines changed

2 files changed

+112
-110
lines changed

‎regression-tests/test-results/pure2-types-value-types-via-meta-functions.cpp

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -93,16 +93,16 @@ template<typename T> auto test() -> void;
9393
#line 4 "pure2-types-value-types-via-meta-functions.cpp2"
9494
}
9595

96-
97-
widget::widget(widget const& that){}
96+
widget::widget(widget const& that)
97+
: val{ that.val }{}
9898

9999
auto widget::operator=(widget const& that) -> widget& {
100-
val = 0;
100+
val = that.val;
101101
return *this;}
102-
103-
widget::widget(widget&& that){}
102+
widget::widget(widget&& that)
103+
: val{ std::move(that).val }{}
104104
auto widget::operator=(widget&& that) -> widget& {
105-
val = 0;
105+
val = std::move(that).val;
106106
return *this;}
107107

108108
widget::widget(){}
@@ -118,16 +118,16 @@ template<typename T> auto test() -> void;
118118
#line 9 "pure2-types-value-types-via-meta-functions.cpp2"
119119
}
120120

121-
122-
w_widget::w_widget(w_widget const& that){}
121+
w_widget::w_widget(w_widget const& that)
122+
: val{ that.val }{}
123123

124124
auto w_widget::operator=(w_widget const& that) -> w_widget& {
125-
val = 0;
125+
val = that.val;
126126
return *this;}
127-
128-
w_widget::w_widget(w_widget&& that){}
127+
w_widget::w_widget(w_widget&& that)
128+
: val{ std::move(that).val }{}
129129
auto w_widget::operator=(w_widget&& that) -> w_widget& {
130-
val = 0;
130+
val = std::move(that).val;
131131
return *this;}
132132

133133
w_widget::w_widget(){}
@@ -143,16 +143,16 @@ template<typename T> auto test() -> void;
143143
#line 14 "pure2-types-value-types-via-meta-functions.cpp2"
144144
}
145145

146-
147-
p_widget::p_widget(p_widget const& that){}
146+
p_widget::p_widget(p_widget const& that)
147+
: val{ that.val }{}
148148

149149
auto p_widget::operator=(p_widget const& that) -> p_widget& {
150-
val = 0;
150+
val = that.val;
151151
return *this;}
152-
153-
p_widget::p_widget(p_widget&& that){}
152+
p_widget::p_widget(p_widget&& that)
153+
: val{ std::move(that).val }{}
154154
auto p_widget::operator=(p_widget&& that) -> p_widget& {
155-
val = 0;
155+
val = std::move(that).val;
156156
return *this;}
157157

158158
p_widget::p_widget(){}

‎source/cppfront.cpp

Lines changed: 94 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -4383,78 +4383,80 @@ class cppfront
43834383
auto object = objects.begin();
43844384
auto statement = statements.begin();
43854385
auto separator = std::string{": "};
4386-
while (
4387-
object != objects.end()
4388-
&& statement != statements.end()
4389-
)
4390-
{
4391-
assert (*statement);
43924386

4387+
while (object != objects.end())
4388+
{
43934389
auto object_name = canonize_object_name(*object);
43944390

43954391
auto is_object_before_base =
43964392
n.get_decl_if_type_scope_object_name_before_a_base_type(*(*object)->name());
43974393

4398-
// If this is an assignment statement, get the lhs and rhs
4399-
auto lhs = std::string{};
4400-
auto rhs = std::string{};
4401-
{
4402-
auto exprs = (*statement)->get_lhs_rhs_if_simple_assignment();
4403-
if (exprs.lhs) {
4404-
if (auto tok = exprs.lhs->get_first_token_ignoring_this()) {
4405-
lhs = *tok;
4406-
}
4407-
else {
4408-
lhs = print_to_string( *exprs.lhs );
4409-
}
4410-
}
4411-
if (exprs.rhs) {
4412-
rhs = print_to_string( *exprs.rhs );
4413-
}
4414-
}
4415-
4416-
// If this is an initialization of an 'out' parameter, stash it
4417-
if (n.has_out_parameter_named(lhs)){
4418-
out_inits.push_back( print_to_string(**statement, false) );
4419-
(*statement)->emitted = true;
4420-
++statement;
4421-
continue;
4422-
}
4423-
4424-
// Now we're ready to check whether this is an assignment to *object
4425-
44264394
auto found_explicit_init = false;
44274395
auto found_default_init = false;
4396+
auto stmt_pos = source_position{};
44284397

4429-
auto stmt_pos = (*statement)->position();
44304398
auto initializer = std::string{};
44314399

4432-
if (!lhs.empty())
4400+
// If we're at an assignment statement, get the lhs and rhs
4401+
if (statement != statements.end())
44334402
{
4434-
// First, see if it's an assignment 'name = something'
4435-
found_explicit_init = object_name == lhs;
4403+
assert (*statement);
4404+
stmt_pos = (*statement)->position();
44364405

4437-
// Otherwise, see if it's 'this.name = something'
4438-
if (!found_explicit_init)
4406+
auto lhs = std::string{};
4407+
auto rhs = std::string{};
44394408
{
4440-
// If it's of the form 'this.name', check 'name'
4441-
if (
4442-
starts_with( lhs, "(*this).")
4443-
&& object_name == lhs.substr(8)
4444-
)
4445-
{
4446-
found_explicit_init = true;
4409+
auto exprs = (*statement)->get_lhs_rhs_if_simple_assignment();
4410+
if (exprs.lhs) {
4411+
if (auto tok = exprs.lhs->get_first_token_ignoring_this()) {
4412+
lhs = *tok;
4413+
}
4414+
else {
4415+
lhs = print_to_string( *exprs.lhs );
4416+
}
4417+
}
4418+
if (exprs.rhs) {
4419+
rhs = print_to_string( *exprs.rhs );
44474420
}
44484421
}
44494422

4450-
if (found_explicit_init)
4451-
{
4452-
initializer = rhs;
4453-
4454-
// We've used this statement, so note it
4455-
// and move 'statement' forward
4423+
// If this is an initialization of an 'out' parameter, stash it
4424+
if (n.has_out_parameter_named(lhs)){
4425+
out_inits.push_back( print_to_string(**statement, false) );
44564426
(*statement)->emitted = true;
44574427
++statement;
4428+
continue;
4429+
}
4430+
4431+
// Now we're ready to check whether this is an assignment to *object
4432+
4433+
if (!lhs.empty())
4434+
{
4435+
// First, see if it's an assignment 'name = something'
4436+
found_explicit_init = object_name == lhs;
4437+
4438+
// Otherwise, see if it's 'this.name = something'
4439+
if (!found_explicit_init)
4440+
{
4441+
// If it's of the form 'this.name', check 'name'
4442+
if (
4443+
starts_with( lhs, "(*this).")
4444+
&& object_name == lhs.substr(8)
4445+
)
4446+
{
4447+
found_explicit_init = true;
4448+
}
4449+
}
4450+
4451+
if (found_explicit_init)
4452+
{
4453+
initializer = rhs;
4454+
4455+
// We've used this statement, so note it
4456+
// and move 'statement' forward
4457+
(*statement)->emitted = true;
4458+
++statement;
4459+
}
44584460
}
44594461
}
44604462

@@ -4600,45 +4602,45 @@ class cppfront
46004602
++object;
46014603
}
46024604

4603-
// Each remaining object is required to have a default initializer,
4604-
// since it had no explicit initialization statement
4605-
for (
4606-
;
4607-
object != objects.end();
4608-
++object
4609-
)
4610-
{
4611-
auto object_name = canonize_object_name(*object);
4612-
4613-
if ((*object)->initializer)
4614-
{
4615-
// Good. Entering here avoids emitting the error on the 'else'
4616-
4617-
auto initializer = print_to_string( *(*object)->initializer, false );
4618-
if (initializer.empty()) {
4619-
initializer = "{}";
4620-
}
4621-
4622-
// Need to actually emit the initializer in an assignment operator
4623-
if (is_assignment)
4624-
{
4625-
current_functions.back().prolog.statements.push_back(
4626-
object_name +
4627-
" = " +
4628-
initializer +
4629-
";"
4630-
);
4631-
}
4632-
}
4633-
else
4634-
{
4635-
errors.emplace_back(
4636-
n.position(),
4637-
object_name + " was not initialized in the operator= body and has no default initializer - " + error_msg
4638-
);
4639-
return;
4640-
}
4641-
}
4605+
//// Each remaining object is required to have a default initializer,
4606+
//// since it had no explicit initialization statement
4607+
//for (
4608+
// ;
4609+
// object != objects.end();
4610+
// ++object
4611+
// )
4612+
//{
4613+
// auto object_name = canonize_object_name(*object);
4614+
4615+
// if ((*object)->initializer)
4616+
// {
4617+
// // Good. Entering here avoids emitting the error on the 'else'
4618+
4619+
// auto initializer = print_to_string( *(*object)->initializer, false );
4620+
// if (initializer.empty()) {
4621+
// initializer = "{}";
4622+
// }
4623+
4624+
// // Need to actually emit the initializer in an assignment operator
4625+
// if (is_assignment)
4626+
// {
4627+
// current_functions.back().prolog.statements.push_back(
4628+
// object_name +
4629+
// " = " +
4630+
// initializer +
4631+
// ";"
4632+
// );
4633+
// }
4634+
// }
4635+
// else
4636+
// {
4637+
// errors.emplace_back(
4638+
// n.position(),
4639+
// object_name + " was not initialized in the operator= body and has no default initializer - " + error_msg
4640+
// );
4641+
// return;
4642+
// }
4643+
//}
46424644

46434645
// Now no data members should be left over
46444646
if (object != objects.end())

0 commit comments

Comments
 (0)