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: lower (nested) _braced-init-list_ (argument) #927

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

JohelEGP
Copy link
Contributor

@JohelEGP JohelEGP commented Jan 10, 2024

Resolves #1343.
Resolves #1283.
Resolves #1037.
Resolves #568.
Resolves #542.

The main branch already treats a return type contextually,
only lowering a braced-init-list if it can work: https://cpp2.godbolt.org/z/sonKE4zan.

This PR recognizes two other places where an expression list can be an initializer to lower as a braced-init-list:

  • As a function argument.
  • As an expression in the expression list of an initializer (nested braced-init-list).

@JohelEGP JohelEGP marked this pull request as ready for review January 10, 2024 02:34
assert(board[2] == :std::array<u8, 3> = ('X', 'O', 'O'));

// Still parentheses
assert((:std::vector = (17, 29)).size() == 2);
Copy link
Contributor Author

@JohelEGP JohelEGP Jan 10, 2024

Choose a reason for hiding this comment

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

For some reason, a semicolon doesn't work to delimit the unnamed declaration:

Suggested change
assert((:std::vector = (17, 29)).size() == 2);
assert(:std::vector = (17, 29);.size() == 2);

Also, I think (0).f() should lower to CPP2_UFCS(f)({0}) (despite #542 (comment)),
just like f((0)) lowers to f({0}).

This comment was marked as outdated.

@JohelEGP
Copy link
Contributor Author

Commit 94bea67 did this for the empty list () (non-nested).

JohelEGP referenced this pull request Sep 24, 2024
Since now we might want to write flags afterwards, e.g., `<Group, flag1, flag2>`, programmers who want the default group `<Default, flag1, flag2>` might expect to be able to equivalently write `<_, flag1, flag2>`, so let's make that work

Until now, programmers would just omit `<Default>` and that continues the work as the basic default, as before
@hsutter
Copy link
Owner

hsutter commented Sep 25, 2024

Thanks and sorry for the lag!

This seems like it could be pri-3 (per #1287) if it were rebased to current main. Do you want to rebase this, or should we close this?

@hsutter hsutter added the question - further information requested Further information is requested label Sep 25, 2024
@JohelEGP
Copy link
Contributor Author

JohelEGP commented Oct 4, 2024

Rebased.

@@ -66,7 +66,7 @@ return ret;

mystruct::mystruct(auto&& i_, auto&& j_, auto&& k_)
requires (std::is_convertible_v<CPP2_TYPEOF(i_), std::add_const_t<cpp2::i32>&> && std::is_convertible_v<CPP2_TYPEOF(j_), std::add_const_t<std::string>&> && std::is_convertible_v<CPP2_TYPEOF(k_), std::add_const_t<cpp2::u64>&>)
: base{ { 1 } }
: base{ 1 }

This comment was marked as off-topic.

Copy link
Contributor Author

@JohelEGP JohelEGP Oct 4, 2024

Choose a reason for hiding this comment

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

Nah, it must be a bug that I don't omit the braces when the context already implies some.

t: @struct type = {
  this: std::vector<int> = (0, 1);
  v: i32;
}

lowers to

: std::vector<int>{ { 01 } }

(: std::vector<int>{ (01) } in the main branch: https://cpp2.godbolt.org/z/srM3vvscM).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem is that emit_special_member_function centralizes the lowering of Cpp1 constructors and assignment operators.
#844 (comment) lists some of the bugs from that.
Maybe one of those PRs fixes this particular issue, so I'm leaving it as-is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment