Skip to content

Commit

Permalink
Fix default member initialization in assignment, closes #290
Browse files Browse the repository at this point in the history
Note this change adds `{` `}` around member assignment statements in assignment operators.

That change now triggers Clang's `-Wbraced-scalar-init` warning, but this warning appears to be spurious. See the open Clang issue here llvm/llvm-project#54702 which mentions the same justification I had in mind (disallow implicit narrowing, that's the #1 reason to recommend using `{` `}` init where possible). Unfortunately that Clang issue still open without comments after a year and so it's hard to know the status.

(BTW, what a coincidence -- I actually hit this on the exact first anniversary of that issue, which also happens to be April 1 but I swear this is not a joke and I think the LLVM issue wasn't either...)
  • Loading branch information
hsutter committed Apr 1, 2023
1 parent a75816b commit feff640
Show file tree
Hide file tree
Showing 14 changed files with 65 additions and 48 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
pure2-types-smf-and-that-1-provide-everything.cpp2:24:16: warning: braces around scalar initializer [-Wbraced-scalar-init]
addr = {"123 Ford Dr."};
^~~~~~~~~~~~~~~~
1 warning generated.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
pure2-types-smf-and-that-2-provide-mvconstruct-and-cpassign.cpp2:24:16: warning: braces around scalar initializer [-Wbraced-scalar-init]
addr = {"123 Ford Dr."};
^~~~~~~~~~~~~~~~
1 warning generated.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
pure2-types-smf-and-that-3-provide-mvconstruct-and-mvassign.cpp2:24:16: warning: braces around scalar initializer [-Wbraced-scalar-init]
addr = {"123 Ford Dr."};
^~~~~~~~~~~~~~~~
1 warning generated.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
pure2-types-smf-and-that-4-provide-cpassign-and-mvassign.cpp2:24:16: warning: braces around scalar initializer [-Wbraced-scalar-init]
addr = {"123 Ford Dr."};
^~~~~~~~~~~~~~~~
1 warning generated.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
pure2-types-smf-and-that-5-provide-nothing-but-general-case.cpp2:24:16: warning: braces around scalar initializer [-Wbraced-scalar-init]
addr = {"123 Ford Dr."};
^~~~~~~~~~~~~~~~
1 warning generated.
8 changes: 4 additions & 4 deletions regression-tests/test-results/pure2-types-basics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ namespace N {
}
#line 6 "pure2-types-basics.cpp2"
auto myclass::operator=(cpp2::in<int> x) -> myclass& {
data = x;
more = std::to_string(42 * 12);
data = {x};
more = {std::to_string(42 * 12)};

#line 9 "pure2-types-basics.cpp2"
std::cout << "myclass: implicit from int\n";
Expand All @@ -106,8 +106,8 @@ namespace N {
}
#line 13 "pure2-types-basics.cpp2"
auto myclass::operator=(cpp2::in<std::string> s) -> myclass& {
data = 99;
more = s;
data = {99};
more = {s};

#line 16 "pure2-types-basics.cpp2"
std::cout << "myclass: explicit from string\n";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ namespace N {
#line 10 "pure2-types-order-independence-and-nesting.cpp2"
auto X::operator=(cpp2::out<Y> y) -> X& {
y.construct(&(*this));
py = &y.value();
py = {&y.value()};

#line 31 "pure2-types-order-independence-and-nesting.cpp2"
std::cout << "made a safely initialized cycle\n";
Expand All @@ -140,7 +140,7 @@ namespace N {
{ }
#line 49 "pure2-types-order-independence-and-nesting.cpp2"
auto Y::operator=(cpp2::in<X*> x) -> Y& {
px = x;
px = {x};
return *this;
#line 49 "pure2-types-order-independence-and-nesting.cpp2"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ auto main() -> int;
}

auto myclass::operator=(myclass const& that) -> myclass& {
name = that.name;
addr = that.addr + "(AC)";
name = {that.name};
addr = {that.addr + "(AC)"};

#line 15 "pure2-types-smf-and-that-1-provide-everything.cpp2"
std::cout << "assign - copy ";
Expand All @@ -79,8 +79,8 @@ auto main() -> int;
}

auto myclass::operator=(myclass&& that) -> myclass& {
name = std::move(that).name;
addr = std::move(that).addr;
name = {std::move(that).name};
addr = {std::move(that).addr};
#line 19 "pure2-types-smf-and-that-1-provide-everything.cpp2"
std::cout << "assign - move ";
return *this;
Expand All @@ -96,8 +96,8 @@ auto main() -> int;
}
#line 22 "pure2-types-smf-and-that-1-provide-everything.cpp2"
auto myclass::operator=(cpp2::in<std::string> x) -> myclass& {
name = x;
addr = "123 Ford Dr.";
name = {x};
addr = {"123 Ford Dr."};

#line 24 "pure2-types-smf-and-that-1-provide-everything.cpp2"
std::cout << "ctor - from string ";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ auto main() -> int;
}

auto myclass::operator=(myclass const& that) -> myclass& {
name = that.name;
addr = that.addr + "(AC)";
name = {that.name};
addr = {that.addr + "(AC)"};

#line 15 "pure2-types-smf-and-that-2-provide-mvconstruct-and-cpassign.cpp2"
std::cout << "assign - copy ";
Expand All @@ -82,8 +82,8 @@ auto main() -> int;
}
#line 13 "pure2-types-smf-and-that-2-provide-mvconstruct-and-cpassign.cpp2"
auto myclass::operator=(myclass&& that) -> myclass& {
name = std::move(that).name;
addr = std::move(that).addr + "(AC)";
name = {std::move(that).name};
addr = {std::move(that).addr + "(AC)"};

#line 15 "pure2-types-smf-and-that-2-provide-mvconstruct-and-cpassign.cpp2"
std::cout << "assign - copy ";
Expand All @@ -101,8 +101,8 @@ auto main() -> int;
}
#line 22 "pure2-types-smf-and-that-2-provide-mvconstruct-and-cpassign.cpp2"
auto myclass::operator=(cpp2::in<std::string> x) -> myclass& {
name = x;
addr = "123 Ford Dr.";
name = {x};
addr = {"123 Ford Dr."};

#line 24 "pure2-types-smf-and-that-2-provide-mvconstruct-and-cpassign.cpp2"
std::cout << "ctor - from string ";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ auto main() -> int;
}
#line 4 "pure2-types-smf-and-that-3-provide-mvconstruct-and-mvassign.cpp2"
auto myclass::operator=(myclass const& that) -> myclass& {
name = that.name;
addr = that.addr;
name = {that.name};
addr = {that.addr};
#line 5 "pure2-types-smf-and-that-3-provide-mvconstruct-and-mvassign.cpp2"
std::cout << "ctor - copy (GENERAL)";
return *this;
Expand All @@ -83,8 +83,8 @@ auto main() -> int;

#line 18 "pure2-types-smf-and-that-3-provide-mvconstruct-and-mvassign.cpp2"
auto myclass::operator=(myclass&& that) -> myclass& {
name = std::move(that).name;
addr = std::move(that).addr;
name = {std::move(that).name};
addr = {std::move(that).addr};
#line 19 "pure2-types-smf-and-that-3-provide-mvconstruct-and-mvassign.cpp2"
std::cout << "assign - move ";
return *this;
Expand All @@ -100,8 +100,8 @@ auto main() -> int;
}
#line 22 "pure2-types-smf-and-that-3-provide-mvconstruct-and-mvassign.cpp2"
auto myclass::operator=(cpp2::in<std::string> x) -> myclass& {
name = x;
addr = "123 Ford Dr.";
name = {x};
addr = {"123 Ford Dr."};

#line 24 "pure2-types-smf-and-that-3-provide-mvconstruct-and-mvassign.cpp2"
std::cout << "ctor - from string ";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ auto main() -> int;

#line 13 "pure2-types-smf-and-that-4-provide-cpassign-and-mvassign.cpp2"
auto myclass::operator=(myclass const& that) -> myclass& {
name = that.name;
addr = that.addr + "(AC)";
name = {that.name};
addr = {that.addr + "(AC)"};

#line 15 "pure2-types-smf-and-that-4-provide-cpassign-and-mvassign.cpp2"
std::cout << "assign - copy ";
Expand All @@ -83,8 +83,8 @@ auto main() -> int;
}

auto myclass::operator=(myclass&& that) -> myclass& {
name = std::move(that).name;
addr = std::move(that).addr;
name = {std::move(that).name};
addr = {std::move(that).addr};
#line 19 "pure2-types-smf-and-that-4-provide-cpassign-and-mvassign.cpp2"
std::cout << "assign - move ";
return *this;
Expand All @@ -100,8 +100,8 @@ auto main() -> int;
}
#line 22 "pure2-types-smf-and-that-4-provide-cpassign-and-mvassign.cpp2"
auto myclass::operator=(cpp2::in<std::string> x) -> myclass& {
name = x;
addr = "123 Ford Dr.";
name = {x};
addr = {"123 Ford Dr."};

#line 24 "pure2-types-smf-and-that-4-provide-cpassign-and-mvassign.cpp2"
std::cout << "ctor - from string ";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ auto main() -> int;
}
#line 4 "pure2-types-smf-and-that-5-provide-nothing-but-general-case.cpp2"
auto myclass::operator=(myclass const& that) -> myclass& {
name = that.name;
addr = that.addr;
name = {that.name};
addr = {that.addr};
#line 5 "pure2-types-smf-and-that-5-provide-nothing-but-general-case.cpp2"
std::cout << "ctor - copy (GENERAL)";
return *this;
Expand All @@ -88,8 +88,8 @@ auto main() -> int;
}
#line 4 "pure2-types-smf-and-that-5-provide-nothing-but-general-case.cpp2"
auto myclass::operator=(myclass&& that) -> myclass& {
name = std::move(that).name;
addr = std::move(that).addr;
name = {std::move(that).name};
addr = {std::move(that).addr};
#line 5 "pure2-types-smf-and-that-5-provide-nothing-but-general-case.cpp2"
std::cout << "ctor - copy (GENERAL)";
return *this;
Expand All @@ -106,8 +106,8 @@ auto main() -> int;
}
#line 22 "pure2-types-smf-and-that-5-provide-nothing-but-general-case.cpp2"
auto myclass::operator=(cpp2::in<std::string> x) -> myclass& {
name = x;
addr = "123 Ford Dr.";
name = {x};
addr = {"123 Ford Dr."};

#line 24 "pure2-types-smf-and-that-5-provide-nothing-but-general-case.cpp2"
std::cout << "ctor - from string ";
Expand Down
8 changes: 4 additions & 4 deletions regression-tests/test-results/pure2-types-that-parameters.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ auto main() -> int;
}
#line 6 "pure2-types-that-parameters.cpp2"
auto myclass::operator=(myclass const& that) -> myclass& {
name = that.name;
addr = that.addr;
name = {that.name};
addr = {that.addr};
return *this;

#line 9 "pure2-types-that-parameters.cpp2"
Expand All @@ -71,8 +71,8 @@ auto main() -> int;
}
#line 11 "pure2-types-that-parameters.cpp2"
auto myclass::operator=(myclass&& that) -> myclass& {
name = std::move(that).name;
addr = std::move(that).addr;
name = {std::move(that).name};
addr = {std::move(that).addr};
return *this;

#line 14 "pure2-types-that-parameters.cpp2"
Expand Down
13 changes: 5 additions & 8 deletions source/cppfront.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4288,11 +4288,10 @@ class cppfront

current_functions.back().prolog.statements.push_back(
(*object)->name()->to_string(true) +
" = " +
" = {" +
initializer +
";"
"};"
);
separator = ", ";
}
// (b) ... if this isn't assignment, only need to emit it if it was
// explicit or is a 'that' initializer
Expand Down Expand Up @@ -4356,14 +4355,12 @@ class cppfront
if (is_assignment)
{
auto initializer = print_to_string( *(*object)->initializer, false );
current_functions.back().prolog.mem_inits.push_back(
separator +
current_functions.back().prolog.statements.push_back(
(*object)->name()->to_string(true) +
"{ " +
" = {" +
initializer +
" }"
"};"
);
separator = ", ";
}
}
else
Expand Down

0 comments on commit feff640

Please sign in to comment.