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

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions regression-tests/pure2-bugfix-for-nested-lists.cpp2
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
point: @value type = {
public x: int = 0;
public y: int = 0;
operator=: (implicit out this, x_: int, y_: int) = {
x = x_;
y = y_;
}
}

check: (p: point) p;

main: () = {
assert(check((17, 29)).x == 17);
assert(check((17, 29)).y == 29);

board: std::array<std::array<u8, 3>, 3> = ((
('O', 'X', 'O'),
(' ', ('X'), 'X'),
('X', 'O', 'O')
));
assert(board[0] == :std::array<u8, 3> = ('O', 'X', 'O'));
assert(board[1] == :std::array<u8, 3> = (' ', 'X', 'X'));
assert(board[2] == :std::array<u8, 3> = ('X', 'O', 'O'));

// Still parentheses (for now?)
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.

}

issue_1283: () = {
f := :() = { };
_ = :() = (f&$*)();
}
4 changes: 2 additions & 2 deletions regression-tests/pure2-hashable.cpp2
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ base: @struct @hashable type = {
}

mystruct: @struct @hashable type = {
this: base = (1);
this: base = 1;
i: i32;
j: std::string;
k: u64;
Expand All @@ -12,4 +12,4 @@ mystruct: @struct @hashable type = {
main: () = {
x: mystruct = (2, "three", 4u);
std::cout << x.hash();
}
}
4 changes: 2 additions & 2 deletions regression-tests/test-results/mixed-default-arguments.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ auto cxx2(cpp2::impl::in<int> x, cpp2::impl::in<std::string> y) -> void{
#line 9 "mixed-default-arguments.cpp2"
auto main() -> int{
cxx(1, "test");
cxx({}, {});
cxx({ }, { });
cxx2(1, "test");
cxx2({}, {});
cxx2({ }, { });
}

95 changes: 95 additions & 0 deletions regression-tests/test-results/pure2-bugfix-for-nested-lists.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@

#define CPP2_IMPORT_STD Yes

//=== Cpp2 type declarations ====================================================


#include "cpp2util.h"

#line 1 "pure2-bugfix-for-nested-lists.cpp2"
class point;
#line 2 "pure2-bugfix-for-nested-lists.cpp2"


//=== Cpp2 type definitions and function declarations ===========================

#line 1 "pure2-bugfix-for-nested-lists.cpp2"
class point {
#line 2 "pure2-bugfix-for-nested-lists.cpp2"
public: int x {0};
public: int y {0};
public: point(cpp2::impl::in<int> x_, cpp2::impl::in<int> y_);
public: [[nodiscard]] auto operator<=>(point const& that) const& -> std::strong_ordering = default;
public: point(point const& that);

public: auto operator=(point const& that) -> point& ;
public: point(point&& that) noexcept;
public: auto operator=(point&& that) noexcept -> point& ;
public: explicit point();

#line 8 "pure2-bugfix-for-nested-lists.cpp2"
};

[[nodiscard]] auto check(cpp2::impl::in<point> p) -> auto;

auto main() -> int;

#line 29 "pure2-bugfix-for-nested-lists.cpp2"
auto issue_1283() -> void;

//=== Cpp2 function definitions =================================================

#line 1 "pure2-bugfix-for-nested-lists.cpp2"

#line 4 "pure2-bugfix-for-nested-lists.cpp2"
point::point(cpp2::impl::in<int> x_, cpp2::impl::in<int> y_)
: x{ x_ }
, y{ y_ }{

#line 7 "pure2-bugfix-for-nested-lists.cpp2"
}


point::point(point const& that)
: x{ that.x }
, y{ that.y }{}

auto point::operator=(point const& that) -> point& {
x = that.x;
y = that.y;
return *this;}
point::point(point&& that) noexcept
: x{ std::move(that).x }
, y{ std::move(that).y }{}
auto point::operator=(point&& that) noexcept -> point& {
x = std::move(that).x;
y = std::move(that).y;
return *this;}
point::point(){}
#line 10 "pure2-bugfix-for-nested-lists.cpp2"
[[nodiscard]] auto check(cpp2::impl::in<point> p) -> auto { return p; }

#line 12 "pure2-bugfix-for-nested-lists.cpp2"
auto main() -> int{
if (cpp2::cpp2_default.is_active() && !(check({ 17, 29 }).x == 17) ) { cpp2::cpp2_default.report_violation(""); }
if (cpp2::cpp2_default.is_active() && !(check({ 17, 29 }).y == 29) ) { cpp2::cpp2_default.report_violation(""); }

std::array<std::array<cpp2::u8,3>,3> board {{ {
'O', 'X', 'O' }, {
' ', { 'X' }, 'X' }, {
'X', 'O', 'O' } }};

if (cpp2::cpp2_default.is_active() && !(CPP2_ASSERT_IN_BOUNDS_LITERAL(board, 0) == std::array<cpp2::u8,3>{'O', 'X', 'O'}) ) { cpp2::cpp2_default.report_violation(""); }
if (cpp2::cpp2_default.is_active() && !(CPP2_ASSERT_IN_BOUNDS_LITERAL(board, 1) == std::array<cpp2::u8,3>{' ', 'X', 'X'}) ) { cpp2::cpp2_default.report_violation(""); }
if (cpp2::cpp2_default.is_active() && !(CPP2_ASSERT_IN_BOUNDS_LITERAL(cpp2::move(board), 2) == std::array<cpp2::u8,3>{'X', 'O', 'O'}) ) { cpp2::cpp2_default.report_violation(""); }

// Still parentheses (for now?)
if (cpp2::cpp2_default.is_active() && !(CPP2_UFCS(size)((std::vector{17, 29})) == 2) ) { cpp2::cpp2_default.report_violation(""); }
}

#line 29 "pure2-bugfix-for-nested-lists.cpp2"
auto issue_1283() -> void{
auto f {[]() -> void{}};
static_cast<void>([_0 = (&f)]() mutable -> void { (*cpp2::impl::assert_not_null(_0))(); });
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
pure2-bugfix-for-nested-lists.cpp2... ok (all Cpp2, passes safety checks)

2 changes: 1 addition & 1 deletion regression-tests/test-results/pure2-hashable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.

, i{ CPP2_FORWARD(i_) }
, j{ CPP2_FORWARD(j_) }
, k{ CPP2_FORWARD(k_) }{}
Expand Down
32 changes: 23 additions & 9 deletions source/parse.h
Original file line number Diff line number Diff line change
Expand Up @@ -699,7 +699,6 @@ struct expression_list_node
token const* open_paren = {};
token const* close_paren = {};
bool inside_initializer = false;
bool default_initializer = false;

struct term {
passing_style pass = {};
Expand Down Expand Up @@ -5772,7 +5771,6 @@ class parser
};
mutable std::vector<function_body_extent> function_body_extents;
mutable bool is_function_body_extents_sorted = false;
bool is_inside_call_expr = false;

public:
auto is_within_function_body(source_position p) const
Expand Down Expand Up @@ -6139,8 +6137,6 @@ class parser
expr_list->inside_initializer = false;
}
n->expression_list_is_fold_expression = expr_list->is_fold_expression();
expr_list->default_initializer =
is_inside_call_expr && std::empty(expr_list->expressions);

n->expr = std::move(expr_list);
return n;
Expand Down Expand Up @@ -6296,9 +6292,7 @@ class parser
// Next should be an expression-list followed by a ')'
// If not, then this wasn't a call expression so backtrack to
// the '(' which will be part of the next grammar production
is_inside_call_expr = true;
term.expr_list = expression_list(term.op, lexeme::RightParen);
is_inside_call_expr = false;
term.expr_list = expression_list(term.op, lexeme::RightParen, true);

if (
term.expr_list
Expand Down Expand Up @@ -6842,8 +6836,28 @@ class parser
return n;
}

auto add_expression = [&](expression_list_node::term t) {
std::function<void(expression_list_node::term&)> mark_nested_inside_initializer{
[&](expression_list_node::term& u) {
if (
inside_initializer
&& u.expr->is_expression_list()
)
{
auto l = const_cast<expression_list_node*>(u.expr->get_expression_list());
l->inside_initializer = true;
for (auto& e : l->expressions) {
mark_nested_inside_initializer(e);
}
}
}
};
mark_nested_inside_initializer(t);
n->expressions.push_back(std::move(t));
};

// Otherwise remember the first expression
n->expressions.push_back( { pass, std::move(x) } );
add_expression( { pass, std::move(x) } );
// and see if there are more...
while (curr().type() == lexeme::Comma) {
next();
Expand All @@ -6859,7 +6873,7 @@ class parser
error("invalid text in expression list", true, {}, true);
return {};
}
n->expressions.push_back( { pass, std::move(expr) } );
add_expression( { pass, std::move(expr) } );
}

return n;
Expand Down
24 changes: 16 additions & 8 deletions source/to_cpp1.h
Original file line number Diff line number Diff line change
Expand Up @@ -4074,13 +4074,6 @@ class cppfront
!(n.inside_initializer && current_declarations.back()->initializer->position() != n.open_paren->position())
;

if (n.default_initializer) {
if (add_parens) {
printer.print_cpp2("{}", n.position());
}
return;
}

if (add_parens) {
printer.print_cpp2( *n.open_paren, n.position());
}
Expand Down Expand Up @@ -4114,7 +4107,22 @@ class cppfront

assert(x.expr);
current_args.push_back( {x.pass} );
emit(*x.expr);
// In a nested expression-list in an initializer, we can
// take over direct control of emitting it without needing to
// go through the whole grammar, and surround it with braces
if (
n.inside_initializer
&& x.expr->is_expression_list()
)
{
printer.print_cpp2( "{ ", n.position() );
emit(*x.expr->get_expression_list(), false);
printer.print_cpp2( " }", n.position() );
}
// Otherwise, just emit the general expression as usual
else {
emit(*x.expr);
}
current_args.pop_back();

if (is_out) {
Expand Down
Loading