Skip to content

fix(cpp1): emit discard with correct precedence #570

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

Merged
merged 3 commits into from
Aug 11, 2023
Merged
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
16 changes: 16 additions & 0 deletions regression-tests/pure2-bugfix-for-discard-precedence.cpp2
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
quantity: type = {
number: i32;
operator=: (out this, _: std::in_place_t, x: i32) = {
number = x;
_ = _;
}
operator+=: (inout this, that) -> forward quantity = {
_ = number += that.number;
return this;
}
}

main: () = {
x: quantity = (std::in_place, 1729);
_ = x += x;
}
4 changes: 4 additions & 0 deletions regression-tests/test-results/clang-18/clang-version.output
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
clang version 18.0.0 (https://git.uplinklabs.net/mirrors/llvm-project.git c0abd3814564a568dfc607c216e6407eaa314f46)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /home/johel/root/clang-main/bin
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,6 @@ auto insert_at(cpp2::in<int> where, cpp2::in<int> val) -> void
cpp2::Default.expects(cpp2::cmp_less_eq(0,where) && cpp2::cmp_less_eq(where,CPP2_UFCS_0(ssize, vec)), "");
auto post_21_5 = cpp2::finally_success([_0 = CPP2_UFCS_0(ssize, vec)]{cpp2::Default.expects(CPP2_UFCS_0(ssize, vec)==_0 + 1, "");} );
#line 23 "mixed-captures-in-expressions-and-postconditions.cpp2"
(void) CPP2_UFCS(insert, vec, CPP2_UFCS_0(begin, vec) + where, val);
static_cast<void>(CPP2_UFCS(insert, vec, CPP2_UFCS_0(begin, vec) + where, val));
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@

#define CPP2_USE_MODULES Yes

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


#include "cpp2util.h"

#line 1 "pure2-bugfix-for-discard-precedence.cpp2"
class quantity;


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

#line 1 "pure2-bugfix-for-discard-precedence.cpp2"
class quantity {
private: cpp2::i32 number;
public: explicit quantity(cpp2::in<std::in_place_t> _, cpp2::in<cpp2::i32> x);


#line 7 "pure2-bugfix-for-discard-precedence.cpp2"
public: [[nodiscard]] auto operator+=(quantity const& that) -> quantity&;

public: quantity(quantity const&) = delete; /* No 'that' constructor, suppress copy */
public: auto operator=(quantity const&) -> void = delete;


#line 11 "pure2-bugfix-for-discard-precedence.cpp2"
};

auto main() -> int;


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


#line 3 "pure2-bugfix-for-discard-precedence.cpp2"
quantity::quantity(cpp2::in<std::in_place_t> _, cpp2::in<cpp2::i32> x)
: number{ x }
#line 3 "pure2-bugfix-for-discard-precedence.cpp2"
{

static_cast<void>(_);
}
[[nodiscard]] auto quantity::operator+=(quantity const& that) -> quantity&{
static_cast<void>(number += that.number);
return (*this);
}

#line 13 "pure2-bugfix-for-discard-precedence.cpp2"
auto main() -> int{
quantity x {std::in_place, 1729};
static_cast<void>(x += std::move(x));
}

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

Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ auto test_generic(auto const& x, auto const& msg) -> void;
test_generic(a, "any");
test_generic(o, "optional<int>");

(void) CPP2_UFCS_TEMPLATE(emplace, (<0>), v, 1);
static_cast<void>(CPP2_UFCS_TEMPLATE(emplace, (<0>), v, 1));
a = 2;
o = 3;
test_generic(42, "int");
Expand Down
2 changes: 1 addition & 1 deletion regression-tests/test-results/pure2-stdio-with-raii.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@
[[nodiscard]] auto main() -> int{
std::string s {"Freddy"};
auto myfile {cpp2::fopen("xyzzy", "w")};
(void) CPP2_UFCS(fprintf, std::move(myfile), "Hello %s with UFCS!", CPP2_UFCS_0(c_str, std::move(s)));
static_cast<void>(CPP2_UFCS(fprintf, std::move(myfile), "Hello %s with UFCS!", CPP2_UFCS_0(c_str, std::move(s))));
}

4 changes: 2 additions & 2 deletions regression-tests/test-results/pure2-stdio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
[[nodiscard]] auto main() -> int{
std::string s {"Fred"};
auto myfile {fopen("xyzzy", "w")};
(void) CPP2_UFCS(fprintf, myfile, "Hello %s with UFCS!", CPP2_UFCS_0(c_str, std::move(s)));
(void) CPP2_UFCS_0(fclose, std::move(myfile));
static_cast<void>(CPP2_UFCS(fprintf, myfile, "Hello %s with UFCS!", CPP2_UFCS_0(c_str, std::move(s))));
static_cast<void>(CPP2_UFCS_0(fclose, std::move(myfile)));
}

2 changes: 1 addition & 1 deletion regression-tests/test-results/pure2-type-safety-1.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ auto print(cpp2::in<std::string> msg, cpp2::in<bool> b) -> void;

std::cout << "\n";

(void) CPP2_UFCS_TEMPLATE(emplace, (<1>), v, 1);
static_cast<void>(CPP2_UFCS_TEMPLATE(emplace, (<1>), v, 1));
a = 2;
o = 3;
test_generic(42, "int");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ auto test_generic(auto const& x, auto const& msg) -> void;
test_generic(a, "any");
test_generic(o, "optional<int>");

(void) CPP2_UFCS_TEMPLATE(emplace, (<2>), v, 1);
static_cast<void>(CPP2_UFCS_TEMPLATE(emplace, (<2>), v, 1));
a = 2;
o = 3;
test_generic(42, "int");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,23 +40,23 @@ extern int y;
#line 1 "pure2-ufcs-member-access-and-chaining.cpp2"
[[nodiscard]] auto main() -> int{
auto i {42};
(void) CPP2_UFCS_0(ufcs, std::move(i));
static_cast<void>(CPP2_UFCS_0(ufcs, std::move(i)));

auto j {fun()};
(void) CPP2_UFCS_0(ufcs, j.i);
static_cast<void>(CPP2_UFCS_0(ufcs, j.i));

(void) CPP2_UFCS_0(ufcs, fun().i);
static_cast<void>(CPP2_UFCS_0(ufcs, fun().i));

auto k {fun().i};
(void) CPP2_UFCS_0(ufcs, std::move(k));
static_cast<void>(CPP2_UFCS_0(ufcs, std::move(k)));

(void) CPP2_UFCS_0(ufcs, get_i(j));
static_cast<void>(CPP2_UFCS_0(ufcs, get_i(j)));

(void) CPP2_UFCS_0(ufcs, get_i(fun()));
static_cast<void>(CPP2_UFCS_0(ufcs, get_i(fun())));

auto res {CPP2_UFCS_0(ufcs, (42))};

(void) CPP2_UFCS_0(ufcs, (std::move(j).i));
static_cast<void>(CPP2_UFCS_0(ufcs, (std::move(j).i)));

CPP2_UFCS_0(no_return, 42);
}
Expand Down
52 changes: 29 additions & 23 deletions source/cppfront.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ class positional_printer
std::vector<comment> const* pcomments = {}; // Cpp2 comments data
source const* psource = {};
parser const* pparser = {};

source_position curr_pos = {}; // current (line,col) in output
lineno_t generated_pos_line = {}; // current line in generated output
int last_line_indentation = {};
Expand Down Expand Up @@ -1229,7 +1229,7 @@ class cppfront

//---------------------------------------------------------------------
// Do lowered file prolog
//
//
// Only emit extra lines if we actually have Cpp2, because
// we want pure-Cpp1 files to pass through with zero changes
if (source.has_cpp2())
Expand Down Expand Up @@ -1287,7 +1287,7 @@ class cppfront
}
}


//---------------------------------------------------------------------
// Do phase1_type_defs_func_decls
//
Expand Down Expand Up @@ -1644,7 +1644,7 @@ class cppfront
{
add_move = false;
}

if (
emitting_move_that_function
&& *n.identifier == "that"
Expand Down Expand Up @@ -2261,7 +2261,7 @@ class cppfront
return;
} else if (
is_literal(tok->type()) || n.expression->expr->is_result_a_temporary_variable()
)
)
{
errors.emplace_back(
n.position(),
Expand Down Expand Up @@ -2557,7 +2557,7 @@ class cppfront
)
-> bool
{
if (!fun_node) {
if (!fun_node) {
return false;
}
if (addr_cnt > deref_cnt) {
Expand All @@ -2584,11 +2584,11 @@ class cppfront
)
-> bool
{
if (!type_id_node) {
if (!type_id_node) {
return false;
}
if (addr_cnt > deref_cnt) {
return true;
return true;
}

if ( type_id_node->dereference_of ) {
Expand Down Expand Up @@ -2765,7 +2765,7 @@ class cppfront
{
auto& unqual = std::get<id_expression_node::unqualified>(id->id);
assert(unqual);
// TODO: Generalize this:
// TODO: Generalize this:
// - we don't recognize pointer types from Cpp1
// - we don't deduce pointer types from parameter_declaration_list_node
if ( is_pointer_declaration(unqual->identifier) ) {
Expand Down Expand Up @@ -3452,8 +3452,8 @@ class cppfront
{
suppress_move_from_last_use = true;
}
// If it's "_ =" then emit (void)
bool suppress_operator = false;
// If it's "_ =" then emit static_cast<void>()
bool emit_discard = false;
if (
!n.terms.empty()
&& n.terms.front().op->type() == lexeme::Assignment
Expand All @@ -3462,8 +3462,8 @@ class cppfront
&& *n.expr->get_postfix_expression_node()->get_first_token_ignoring_this() == "_"
)
{
printer.print_cpp2( "(void)", n.position() );
suppress_operator = true;
printer.print_cpp2( "static_cast<void>(", n.position() );
emit_discard = true;
}
else
{
Expand Down Expand Up @@ -3514,6 +3514,7 @@ class cppfront
}
}

auto first = true;
for (auto const& x : n.terms) {
assert(x.op);
assert(x.expr);
Expand All @@ -3533,17 +3534,16 @@ class cppfront
else
{
// For the first operator only, if we are emitting a "_ =" discard
// then we've already emitted the cast to void and don't need the =
if (suppress_operator) {
assert( x.op->type() == lexeme::Assignment );
suppress_operator = false;
}
else {
// then we don't need the =
if (
!emit_discard
|| !first
) {
printer.print_cpp2(" ", n.position());
emit(*x.op);
printer.print_cpp2(" ", n.position());
}
printer.print_cpp2(" ", n.position());


// When assigning a single expression-list, we can
// take over direct control of emitting it without needing to
// go through the whole grammar, and surround it with braces
Expand All @@ -3561,6 +3561,12 @@ class cppfront
emit(*x.expr);
}
}

first = false;
}
// Finish emitting the "_ =" discard.
if (emit_discard) {
printer.print_cpp2( ")", n.position() );
}
}

Expand Down Expand Up @@ -4868,7 +4874,7 @@ class cppfront
}

// If this is a generated declaration (negative source line number),
// add a line break before
// add a line break before
if (
printer.get_phase() == printer.phase2_func_defs
&& n.position().lineno < 1
Expand Down Expand Up @@ -5504,7 +5510,7 @@ class cppfront
// A2) This is '(out this, move that)'
// and no '(inout this, move that)' was written by the user
// (*) and no '(inout this, that)' was written by the user (*)
//
//
// (*) This third test is to tie-break M2 and A2 in favor of M2. Both M2 and A2
// can generate a missing '(inout this, move that)', and if we have both
// options then we should prefer to use M2 (generate move assignment from
Expand Down