-
Notifications
You must be signed in to change notification settings - Fork 252
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
[BUG] Type with a member and defined operator=: (out this, t : int )
produces an assignment operator with an initializer list
#290
Comments
operator=: (out this, t : int )
produces assignment operator with initialiser listoperator=: (out this, t : int )
produces an assignment operator with an initializer list
Similarly to #291 you can make this work by resigning from using the default initializer. The following code works: element: type = {
children: std::vector<int>;
operator=: (out this, t : int ) = {
children = std::vector<int>();
}
} also it is worth noting that I would prefer to write: operator=: (out this, t : int ) = {
children = (1,2,3);
} to initialize public: explicit element(cpp2::in<int> t)
: children{ 1, 2, 3 }
#line 5 "../tests/bug_assignement_operator.cpp2"
{
}
#line 4 "../tests/bug_assignement_operator.cpp2"
public: auto operator=(cpp2::in<int> t) -> element& {
children = 1, 2, 3;
#line 5 "../tests/bug_assignement_operator.cpp2"
return *this;
#line 6 "../tests/bug_assignement_operator.cpp2"
} Constructor is fine - it is using initializer list to initialize the vector with 1,2,3. The assignment is bad: children = 1, 2, 3; Workaround for this will be using: children = std::vector<int>(1,2,3); That will generate: class element {
private: std::vector<int> children {};
public: explicit element(cpp2::in<int> t)
: children{ std::vector<int>(1, 2, 3) }
#line 4 "/Users/filipsajdak/dev/execspec/external/tests/bug_assignement_operator.cpp2"
{
}
#line 4 "/Users/filipsajdak/dev/execspec/external/tests/bug_assignement_operator.cpp2"
public: auto operator=(cpp2::in<int> t) -> element& {
children = std::vector<int>(1, 2, 3);
return *this;
#line 6 "/Users/filipsajdak/dev/execspec/external/tests/bug_assignement_operator.cpp2"
}
}; |
Thanks! Fixed. See also the comment in the closing commit for more information about a Clang warning that my fix now generates, but that I think is spurious. |
@hsutter I confirmed. The issue is solved! The warning also appears with my compiler:
|
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 hsutter#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...)
In the current implementation of cppfront (fbf55ad) the following code:
Generates:
The issue is that the assignment operator has an initializer list and fails to compile with the error:
The text was updated successfully, but these errors were encountered: