Skip to content

Commit c39a94c

Browse files
committed
Improve explicit forward
Apply to first identifier Diagnose attempt to `forward` an expression that doesn't start with a `forward` parameter name That's roughed-in enough to experiment with and get feedback on whether we want to keep this feature... if we do, then we can add some more diagnostics for bad source (though this commit does make some attempt to check it's for a forwarding parameter), and consider changing the precedence of `forward` in parsing (the current parsing feels pretty good where `out` and `move` and `forward` apply to the whole argument, but maybe we'll see good reason to adjust that based on usage)
1 parent bef05a9 commit c39a94c

File tree

8 files changed

+88
-35
lines changed

8 files changed

+88
-35
lines changed

regression-tests/mixed-forwarding.cpp2

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,28 @@ struct X {
88
X(X && that) : i{that.i} { std::cout << "move X " << i << "\n"; }
99
};
1010

11-
discard: (copy x:_) = { }
11+
copy_from: (copy x:_) = { }
1212

13+
use: (x:_) = {}
14+
15+
// invoking each of these with an rvalue std::pair argument ...
1316
apply_implicit_forward: (forward t: std::pair<X, X>) = {
14-
discard(t.first);
15-
discard(t.second);
17+
copy_from(t.first); // copies
18+
copy_from(t.second); // moves
1619
}
17-
1820
apply_explicit_forward: (forward t: std::pair<X, X>) = {
19-
discard(forward t.first);
20-
discard(forward t.second);
21+
copy_from(forward t.first); // moves
22+
copy_from(forward t.second); // moves
2123
}
2224

2325
main: ()->int = {
2426
t1: std::pair<X,X> = (1,2);
2527
apply_implicit_forward(t1);
28+
use(t1);
29+
apply_implicit_forward(t1);
2630

2731
t2: std::pair<X,X> = (3,4);
2832
apply_explicit_forward(t2);
33+
use(t2);
34+
apply_explicit_forward(t2);
2935
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
+X 1
22
+X 2
33
copy X 1
4+
copy X 2
5+
copy X 1
46
move X 2
57
+X 3
68
+X 4
9+
copy X 3
10+
copy X 4
711
move X 3
812
move X 4
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
+X 1
22
+X 2
33
copy X 1
4+
copy X 2
5+
copy X 1
46
move X 2
57
+X 3
68
+X 4
9+
copy X 3
10+
copy X 4
711
move X 3
812
move X 4

regression-tests/test-results/mixed-forwarding.cpp

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,39 +12,47 @@ struct X {
1212
X(X && that) : i{that.i} { std::cout << "move X " << i << "\n"; }
1313
};
1414

15-
auto discard(auto x) -> void;
15+
auto copy_from(auto x) -> void;
1616
#line 13 "mixed-forwarding.cpp2"
17+
auto use(auto const& x) -> void;
18+
#line 16 "mixed-forwarding.cpp2"
1719
auto apply_implicit_forward(auto&& t) -> void;
18-
#line 18 "mixed-forwarding.cpp2"
20+
#line 20 "mixed-forwarding.cpp2"
1921
auto apply_explicit_forward(auto&& t) -> void;
20-
#line 23 "mixed-forwarding.cpp2"
22+
#line 25 "mixed-forwarding.cpp2"
2123
[[nodiscard]] auto main() -> int;
2224

2325
//=== Cpp2 definitions ==========================================================
2426

2527
#line 10 "mixed-forwarding.cpp2"
2628

27-
auto discard(auto x) -> void{}
29+
auto copy_from(auto x) -> void{}
2830

31+
auto use(auto const& x) -> void{}
32+
33+
// invoking each of these with an rvalue std::pair argument ...
2934
auto apply_implicit_forward(auto&& t) -> void
3035
requires std::is_same_v<CPP2_TYPEOF(t), std::pair<X,X>>
31-
#line 14 "mixed-forwarding.cpp2"
32-
{ discard(t.first);
33-
discard(CPP2_FORWARD(t).second);
36+
#line 17 "mixed-forwarding.cpp2"
37+
{ copy_from(t.first); // copies
38+
copy_from(CPP2_FORWARD(t).second);// moves
3439
}
35-
3640
auto apply_explicit_forward(auto&& t) -> void
3741
requires std::is_same_v<CPP2_TYPEOF(t), std::pair<X,X>>
38-
#line 19 "mixed-forwarding.cpp2"
39-
{ discard(CPP2_FORWARD(t.first));
40-
discard(CPP2_FORWARD(CPP2_FORWARD(t).second));
42+
#line 21 "mixed-forwarding.cpp2"
43+
{ copy_from(CPP2_FORWARD(t).first);// moves
44+
copy_from(CPP2_FORWARD(t).second);// moves
4145
}
4246

4347
[[nodiscard]] auto main() -> int{
4448
std::pair<X,X> t1 { 1, 2 };
49+
apply_implicit_forward(t1);
50+
use(t1);
4551
apply_implicit_forward(std::move(t1));
4652

4753
std::pair<X,X> t2 { 3, 4 };
54+
apply_explicit_forward(t2);
55+
use(t2);
4856
apply_explicit_forward(std::move(t2));
4957
}
5058

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
Microsoft (R) C/C++ Optimizing Compiler Version 19.34.31933 for x86
1+
Microsoft (R) C/C++ Optimizing Compiler Version 19.34.31937 for x86
22
Copyright (C) Microsoft Corporation. All rights reserved.
33

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
+X 1
22
+X 2
33
copy X 1
4+
copy X 2
5+
copy X 1
46
move X 2
57
+X 3
68
+X 4
9+
copy X 3
10+
copy X 4
711
move X 3
812
move X 4

source/cppfront.cpp

Lines changed: 43 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -645,11 +645,17 @@ class cppfront
645645
cpp2::parser parser;
646646
cpp2::sema sema;
647647

648-
bool source_loaded = true;
649-
bool last_postfix_expr_was_pointer = false;
650-
bool violates_bounds_safety = false;
651-
bool violates_initialization_safety = false;
652-
bool suppress_move_from_last_use = false;
648+
bool source_loaded = true;
649+
bool last_postfix_expr_was_pointer = false;
650+
bool violates_bounds_safety = false;
651+
bool violates_initialization_safety = false;
652+
bool suppress_move_from_last_use = false;
653+
654+
struct arg_info {
655+
passing_style pass = passing_style::in;
656+
token const* token = nullptr;
657+
};
658+
std::vector<arg_info> current_args = { {} };
653659

654660
// For lowering
655661
//
@@ -1090,11 +1096,17 @@ class cppfront
10901096
(in_synthesized_multi_return || (last_use && !suppress_move_from_last_use)) &&
10911097
!in_non_rvalue_context.back();
10921098

1099+
// For an explicit 'forward' apply forwarding to correct identifier
1100+
assert (!current_args.empty());
1101+
if (current_args.back().pass == passing_style::forward) {
1102+
add_std_forward = current_args.back().token == n.identifier;
1103+
}
1104+
10931105
if (add_std_move) {
10941106
printer.print_cpp2("std::move(", n.position());
10951107
}
10961108
if (add_std_forward) {
1097-
printer.print_cpp2("CPP2_FORWARD(", n.position());
1109+
printer.print_cpp2("CPP2_FORWARD(", {n.position().lineno, n.position().colno - 8});
10981110
}
10991111

11001112
assert(n.identifier);
@@ -1610,6 +1622,23 @@ class cppfront
16101622
assert(n.expr);
16111623
last_postfix_expr_was_pointer = false;
16121624

1625+
// Ensure that forwarding postfix-expressions start with a forwarded parameter name
1626+
//
1627+
assert (!current_args.empty());
1628+
if (current_args.back().pass == passing_style::forward)
1629+
{
1630+
assert (n.expr->get_token());
1631+
assert (!current_args.back().token);
1632+
current_args.back().token = n.expr->get_token();
1633+
auto decl = sema.get_local_declaration_of(*current_args.back().token);
1634+
if (!(decl && decl->parameter && decl->parameter->pass == passing_style::forward)) {
1635+
errors.emplace_back(
1636+
n.position(),
1637+
n.expr->get_token()->to_string(true) + " is not a forwarding parameter name"
1638+
);
1639+
}
1640+
}
1641+
16131642
// Check that this isn't pointer arithmentic
16141643
// (initial partial implementation)
16151644
if (n.expr->expr.index() == primary_expression_node::id_expression)
@@ -2066,23 +2095,19 @@ class cppfront
20662095

20672096
if (x.pass != passing_style::in) {
20682097
assert(
2069-
to_string_view(x.pass) == "out" ||
2070-
to_string_view(x.pass) == "move" ||
2071-
to_string_view(x.pass) == "forward"
2098+
x.pass == passing_style::out ||
2099+
x.pass == passing_style::move ||
2100+
x.pass == passing_style::forward
20722101
);
2073-
if (to_string_view(x.pass) == "out") {
2102+
if (x.pass == passing_style::out) {
20742103
is_out = true;
20752104
printer.print_cpp2("&", n.position());
20762105
offset = -3; // because we're replacing "out " (followed by at least one space) with "&"
20772106
}
2078-
else if (to_string_view(x.pass) == "move") {
2107+
else if (x.pass == passing_style::move) {
20792108
printer.print_cpp2("std::move(", n.position());
20802109
offset = 6; // because we're replacing "move " (followed by at least one space) with "std::move("
20812110
}
2082-
else if (to_string_view(x.pass) == "forward") {
2083-
printer.print_cpp2("CPP2_FORWARD(", n.position());
2084-
offset = 10; // because we're replacing "forward " (followed by at least one space) with "CPP2_FORWARD("
2085-
}
20862111
}
20872112

20882113
if (is_out) {
@@ -2091,14 +2116,16 @@ class cppfront
20912116

20922117
assert(x.expr);
20932118
adjust_remaining_token_columns_on_this_line_visitor v(x.expr->position(), offset);
2119+
current_args.push_back( {x.pass} );
20942120
x.expr->visit(v, 0);
20952121
emit(*x.expr);
2122+
current_args.pop_back();
20962123

20972124
if (is_out) {
20982125
in_non_rvalue_context.pop_back();
20992126
}
21002127

2101-
if (to_string_view(x.pass) == "move" || to_string_view(x.pass) == "forward") {
2128+
if (x.pass == passing_style::move) {
21022129
printer.print_cpp2(")", n.position());
21032130
}
21042131
}

source/sema.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ struct declaration_sym {
3434
declaration_node const* declaration = nullptr;
3535
token const* identifier = nullptr;
3636
statement_node const* initializer = nullptr;
37-
parameter_declaration_node const* parameter = nullptr;
37+
parameter_declaration_node const* parameter = nullptr;
3838

3939
declaration_sym(
4040
bool s = false,

0 commit comments

Comments
 (0)