Skip to content
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

fix(cpp1): support assignment from expression list #487

Merged
merged 4 commits into from
Aug 10, 2023

Conversation

JohelEGP
Copy link
Contributor

@JohelEGP JohelEGP commented Jun 2, 2023

Resolves #321.
Resolves #449.

Now fixing #408 becomes necessary.
The reason is explained at #451 (comment):

Next, consider if #321 were fixed.
a = {x, y} would happen to work, but actually construct a t and then move-assign to a
(see https://cpp2.godbolt.org/z/4s64rnsEd, after changing the type to avoid narrowing braces).

Testing summary.
100% tests passed, 0 tests failed out of 684

Total Test time (real) = 109.49 sec
Acknowledgements.

// go through the whole grammar, and surround it with braces
if (
x.op->type() == lexeme::Assignment
&& x.expr->is_expression_list()
Copy link
Owner

@hsutter hsutter Aug 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, but it needs

                && std::ssize(x.expr->get_expression_list()->expressions.size()) != 1

otherwise x = (move y); will grow braces we don't need. I'll make the change directly and merge.

Thanks!

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, I was wrong -- we want those { } in other cases. Will revert my changes...

hsutter added 3 commits August 9, 2023 21:40
Added size check so we don't add `{` `}` around single-element lists

Signed-off-by: Herb Sutter <[email protected]>
`std::ssize` for consistency, and `!= 1` so we still emit `{` `}` around empty lists.

Signed-off-by: Herb Sutter <[email protected]>
Signed-off-by: Herb Sutter <[email protected]>
@hsutter
Copy link
Owner

hsutter commented Aug 10, 2023

Thanks!

@hsutter hsutter merged commit d7857bb into hsutter:main Aug 10, 2023
@JohelEGP JohelEGP deleted the assign_expression_list branch August 10, 2023 15:38
zaucy pushed a commit to zaucy/cppfront that referenced this pull request Dec 5, 2023
* fix(cpp1): support assignment from expression list

* Update cppfront.cpp

Added size check so we don't add `{` `}` around single-element lists

Signed-off-by: Herb Sutter <[email protected]>

* Update cppfront.cpp

`std::ssize` for consistency, and `!= 1` so we still emit `{` `}` around empty lists.

Signed-off-by: Herb Sutter <[email protected]>

* Update cppfront.cpp

Signed-off-by: Herb Sutter <[email protected]>

---------

Signed-off-by: Herb Sutter <[email protected]>
Co-authored-by: Herb Sutter <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants