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

* test: add test case for discard precedence

* fix(cpp1): emit discard with correct precedence

* refactor: update comment

Author: JohelEGP

regression-tests/pure2-bugfix-for-discard-precedence.cpp2

@@ -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;
+}

regression-tests/test-results/clang-18/clang-version.output

@@ -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

regression-tests/test-results/clang-18/pure2-bugfix-for-discard-precedence.cpp.execution

@@ -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));
 }
 

regression-tests/test-results/clang-18/pure2-bugfix-for-discard-precedence.cpp.output

@@ -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));
+}
+

regression-tests/test-results/mixed-captures-in-expressions-and-postconditions.cpp

@@ -0,0 +1,2 @@
+pure2-bugfix-for-discard-precedence.cpp2... ok (all Cpp2, passes safety checks)
+

regression-tests/test-results/pure2-bugfix-for-discard-precedence.cpp

@@ -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");

regression-tests/test-results/pure2-bugfix-for-discard-precedence.cpp2.output

@@ -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))));
 }
 

regression-tests/test-results/pure2-inspect-expression-in-generic-function-multiple-types.cpp

@@ -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)));
 }
 

regression-tests/test-results/pure2-stdio-with-raii.cpp

@@ -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");

regression-tests/test-results/pure2-stdio.cpp

@@ -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");

regression-tests/test-results/pure2-type-safety-1.cpp

@@ -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);
 }

regression-tests/test-results/pure2-type-safety-2-with-inspect-expression.cpp

@@ -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       = {};
@@ -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())
@@ -1287,7 +1287,7 @@ class cppfront
             }
         }
 
-        
+
         //---------------------------------------------------------------------
         //  Do phase1_type_defs_func_decls
         //
@@ -1644,7 +1644,7 @@ class cppfront
         {
             add_move = false;
         }
-    
+
         if (
             emitting_move_that_function
             && *n.identifier == "that"
@@ -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(),
@@ -2557,7 +2557,7 @@ class cppfront
     )
         -> bool
     {
-        if (!fun_node) { 
+        if (!fun_node) {
             return false;
         }
         if (addr_cnt > deref_cnt) {
@@ -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 ) {
@@ -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) ) {
@@ -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
@@ -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
         {
@@ -3514,6 +3514,7 @@ class cppfront
             }
         }
 
+        auto first = true;
         for (auto const& x : n.terms) {
             assert(x.op);
             assert(x.expr);
@@ -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
@@ -3561,6 +3561,12 @@ class cppfront
                     emit(*x.expr);
                 }
             }
+
+            first = false;
+        }
+        // Finish emitting the "_ =" discard.
+        if (emit_discard) {
+            printer.print_cpp2( ")", n.position() );
         }
     }
 
@@ -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
@@ -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