Skip to content

[BUG] Missing braces in assignment operator when initialize vector with a list of values #321

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

Closed
filipsajdak opened this issue Apr 4, 2023 · 2 comments · Fixed by #487
Closed
Labels
bug Something isn't working

Comments

@filipsajdak
Copy link
Contributor

After f608b78 (done to fix #312). It broke the scenario described here: #312 (comment)

For the following code:

element: type = {
    children: std::vector<int> = (1,2,3);
    tab : std::vector<int>;

    operator=: (out this, t : int ) = {
        tab = (3,2,1);
    }
}

Cppfront will generate (skipping type declarations, type definitions and function declarations):

#line 5 "/Users/filipsajdak/dev/execspec/external/tests/bug_assignement_operator_4.cpp2"
    element::element(cpp2::in<int> t)
        : tab{ 3, 2, 1 }
#line 5 "/Users/filipsajdak/dev/execspec/external/tests/bug_assignement_operator_4.cpp2"
    {

    }
#line 5 "/Users/filipsajdak/dev/execspec/external/tests/bug_assignement_operator_4.cpp2"
    auto element::operator=(cpp2::in<int> t) -> element& {
        children = 1, 2, 3;
        tab = 3, 2, 1;
        return *this;

#line 7 "/Users/filipsajdak/dev/execspec/external/tests/bug_assignement_operator_4.cpp2"
    }

The issue is with element::operator=() that has the following lines:

        children = 1, 2, 3;
        tab = 3, 2, 1;

And will compile with the error (cut only the part that describes the problem):

../tests/bug_assignement_operator_4.cpp2:6:18: error: no viable overloaded '='
        children = 1, 2, 3;
        ~~~~~~~~ ^ ~
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/vector:444:13: note: candidate function not viable: no known conversion from 'int' to 'const std::vector<int>' for 1st argument
    vector& operator=(const vector& __x);
            ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/vector:454:13: note: candidate function not viable: no known conversion from 'int' to 'initializer_list<std::vector<int>::value_type>' (aka 'initializer_list<int>') for 1st argument
    vector& operator=(initializer_list<value_type> __il)
            ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/vector:469:13: note: candidate function not viable: no known conversion from 'int' to 'std::vector<int>' for 1st argument
    vector& operator=(vector&& __x)
            ^
../tests/bug_assignement_operator_4.cpp2:7:13: error: no viable overloaded '='
        tab = 3, 2, 1;
        ~~~ ^ ~

Changing problematic lines to the following:

        children = {1, 2, 3};
        tab = {3, 2, 1};

Fixes the issue.

@filipsajdak filipsajdak added the bug Something isn't working label Apr 4, 2023
@filipsajdak
Copy link
Contributor Author

I did a dirty fix to that (I don't know yet how to test if the initializer has expression_list_node, so I check if emitted initializer has a comma ,).

diff --git a/source/cppfront.cpp b/source/cppfront.cpp
index 816f5be..45c2a60 100644
--- a/source/cppfront.cpp
+++ b/source/cppfront.cpp
@@ -4321,11 +4321,13 @@ public:
                     }
                     out_inits = {};
 
+                    auto found_expression_list = initializer.find(',') != initializer.npos;
+
                     current_functions.back().prolog.statements.push_back(
                         (*object)->name()->to_string(true) +
-                        " = " +
+                        " = " + (found_expression_list ? "{" : "") +
                         initializer +
-                        ";"
+                        (found_expression_list ? "}" : "") + ";"
                     );
                 }
                 //  (b) ... if this isn't assignment, only need to emit it if it was
@@ -4393,12 +4395,12 @@ public:
                         if (initializer.empty()) {
                             initializer = "{}";
                         }
-
+                        auto found_expression_list = initializer.find(',') != initializer.npos;
                         current_functions.back().prolog.statements.push_back(
                             (*object)->name()->to_string(true) +
-                            " = " +
+                            " = " + (found_expression_list ? "{" : "") +
                             initializer +
-                            ";"
+                            (found_expression_list ? "}" : "")+ ";"
                         );
                     }
                 }

The dirty fix works. Maybe you know a better way of checking if initializer (statement_node) has expression_list_node?

@basic-bro
Copy link

If I've read the source correctly, initializer in this case is set on line 4263 with:

initializer = print_to_string( *exprs.rhs );

Now, the member .rhs has type logical_or_expression_node*, which does not reach an expression_list_node going down the binary_expression_node tree. This is unlike the .lhs member, which has type postfix_expression_node*, whose tree includes expression_list_node via the primary_expression_node expr pointer member or via the ops member vector of type term.

Hence, perhaps changing the type of the .rhs member in the assignment_expression_lhs_rhs struct is a way forward?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants