Emit non-member binary streaming and arithmetic as friends

hsutter · hsutter · commit ca42e2c4a057 · 2023-09-23T11:09:28.000-07:00
These must be non-member friends:
- Streaming operators `&lt;&lt;` and `&gt;&gt;` (not as bit shift) need `this` to be a non-first parameter
- Arithmetic binary operators are typically non-members to get symmetry (conversions on both arguments) and cannot be static

Note: It's a known limitation that these won't yet work right on templated types. I will likely add that sometime in the future, and will require doing the dance Cpp1 requires to emit friend template specializations, and to handle dependent types more fully in `cpp2::in&lt;&gt;` for cases like `cpp2::in&lt;mytype&lt;T&gt;&gt;`.

And ban unsafe/now-unnecessary overloaded operators:
- Reject non-member comparison overloaded operators. These used to be the same as arithmetic operators, until C++20 standardized the design from Cpp2 which is symmetric and so there is no longer any reason to write them as non-members
- Reject non-member bitwise binary overloaded operators. ISTM these shouldn't be non-members... argument conversions seem actively undesirable, unlike with arithmetic binary operators
- Reject all `&amp;&amp;`, `||`, and ',' (comma) overloaded operators, we already teach that these should not be overloaded
diff --git a/regression-tests/test-results/pure2-types-down-upcast.cpp b/regression-tests/test-results/pure2-types-down-upcast.cpp
@@ -37,8 +37,8 @@ class B: public A {
 #line 11 "pure2-types-down-upcast.cpp2"
 };
 
-auto func_mut(A&  a) ->   void;
-auto func_mut(B&  b) ->   void;
+auto func_mut(A&  a) -> void;
+auto func_mut(B&  b) -> void;
 auto func_const(cpp2::in<A> a) -> void;
 auto func_const(cpp2::in<B> b) -> void;
 
@@ -69,8 +69,8 @@ auto test_down() -> void;
  auto A::mut_foo() & -> void{std::cout << "foo \n"; }
 
 #line 13 "pure2-types-down-upcast.cpp2"
-auto func_mut(A&  a) ->   void  {std::cout << "Call A mut: " + cpp2::to_string(a.i) << std::endl;}
-auto func_mut(B&  b) ->   void  {std::cout << "Call B mut: " + cpp2::to_string(b.d) << std::endl;}
+auto func_mut(A&  a) -> void    {std::cout << "Call A mut: " + cpp2::to_string(a.i) << std::endl;}
+auto func_mut(B&  b) -> void    {std::cout << "Call B mut: " + cpp2::to_string(b.d) << std::endl;}
 auto func_const(cpp2::in<A> a) -> void{std::cout << "Call A const: " + cpp2::to_string(a.i) << std::endl;}
 auto func_const(cpp2::in<B> b) -> void{std::cout << "Call B const: " + cpp2::to_string(b.d) << std::endl;}
 
diff --git a/regression-tests/test-results/version b/regression-tests/test-results/version
@@ -1,5 +1,5 @@
 
-cppfront compiler v0.2.1   Build 8921:1228
+cppfront compiler v0.2.1   Build 8922:0931
 Copyright(c) Herb Sutter   All rights reserved
 
 SPDX-License-Identifier: CC-BY-NC-ND-4.0
diff --git a/source/build.info b/source/build.info
@@ -1 +1 @@
-"8921:1228"
+"8922:0931"
diff --git a/source/cppfront.cpp b/source/cppfront.cpp
@@ -4033,7 +4033,7 @@ class cppfront
             && n.pass == passing_style::in
             )
         {
-            printer.print_cpp2( print_to_string(type_id), n.position() );
+            printer.print_cpp2( param_type, n.position() );
         }
         else if (
             type_id.is_wildcard()
@@ -4045,6 +4045,7 @@ class cppfront
             if (is_dependent_parameter_type) {
                 name = param_type;
             }
+
             switch (n.pass) {
             break;case passing_style::in     : printer.print_cpp2( name+" const&", n.position() );
             break;case passing_style::copy   : printer.print_cpp2( name,           n.position() );
@@ -4071,19 +4072,16 @@ class cppfront
             auto req = std::string{ "std::is_same_v<CPP2_TYPEOF(" };
             req += *name;
             req += "), ";
-            printer.emit_to_string(&req);
-            emit( type_id );
-            printer.emit_to_string();
+            req += param_type;
             req += ">";
             function_requires_conditions.push_back(req);
         }
         else {
-            auto type_name = print_to_string( type_id );
             if (is_returns) {
-                printer.print_extra( type_name );
+                printer.print_extra( param_type );
             }
             else {
-                printer.print_cpp2( type_name, type_id.position() );
+                printer.print_cpp2( param_type, type_id.position() );
             }
         }
 
@@ -4362,12 +4360,15 @@ class cppfront
             printer.print_cpp2( " -> ", n.position() );
             auto& r = std::get<function_type_node::id>(n.returns);
             assert(r.type);
+
+            auto return_type = print_to_string(*r.type);
+
             if (r.pass == passing_style::forward) {
                 if (r.type->is_wildcard()) {
                     printer.print_cpp2( "auto&&", n.position() );
                 }
                 else {
-                    emit(*r.type);
+                    printer.print_cpp2( return_type, n.position() );
                     if (is_type_scope_function_with_in_this) {
                         printer.print_cpp2( " const&", n.position() );
                     }
@@ -4377,7 +4378,7 @@ class cppfront
                 }
             }
             else {
-                emit(*r.type);
+                printer.print_cpp2( return_type, n.position() );
             }
         }
 
@@ -5511,7 +5512,30 @@ class cppfront
                 )
             )
         {
-            auto emit_as_friend = n.name() && n.name()->as_string_view().starts_with("operator<<");
+            auto is_streaming_operator = [](std::string_view sv) {
+                return
+                    sv == "operator<<"
+                    || sv == "operator>>"
+                    ;
+                };
+
+            auto is_binary_arithmetic_operator = [](std::string_view sv) {
+                return
+                    sv == "operator+"
+                    || sv == "operator-"
+                    || sv == "operator*"
+                    || sv == "operator/"
+                    || sv == "operator%"
+                    ;
+                };
+
+            auto emit_as_friend =
+                n.name()
+                && (
+                    is_streaming_operator( n.name()->as_string_view() )
+                    || (!n.is_function_with_this() && is_binary_arithmetic_operator( n.name()->as_string_view() ))
+                    )
+                ;
 
             //  Start fresh (there may be one spurious leftover
             //  requires-condition created during the declarations pass)
diff --git a/source/parse.h b/source/parse.h
@@ -2197,6 +2197,9 @@ struct function_type_node
     auto is_constructor_with_move_that() const
         -> bool;
 
+    auto is_comparison() const
+        -> bool;
+
     auto is_compound_assignment() const
         -> bool;
 
@@ -2252,6 +2255,12 @@ struct function_type_node
         return returns.index() != empty;
     }
 
+    auto parameter_count() const
+        -> int
+    {
+        return std::ssize(parameters->parameters);
+    }
+
     auto index_of_parameter_named(std::string_view s) const
         -> int
     {
@@ -2662,6 +2671,15 @@ struct declaration_node
         return initializer != nullptr;
     }
 
+    auto parameter_count() const
+        -> int
+    {
+        if (!is_function()) {
+            return -1;
+        }
+        return std::get<a_function>(type)->parameter_count();
+    }
+
     auto index_of_parameter_named(std::string_view s) const
         -> int
     {
@@ -3084,6 +3102,16 @@ struct declaration_node
         return false;
     }
 
+    auto is_comparison() const
+        -> bool
+    {
+        if (auto func = std::get_if<a_function>(&type)) {
+            return (*func)->is_comparison();
+        }
+        //  else
+        return false;
+    }
+
     auto is_compound_assignment() const
         -> bool
     {
@@ -3550,6 +3578,27 @@ auto function_type_node::is_constructor_with_move_that() const
 }
 
 
+auto function_type_node::is_comparison() const
+    -> bool
+{
+    if (
+        (
+            my_decl->has_name("operator==")
+            || my_decl->has_name("operator!=")
+            || my_decl->has_name("operator<")
+            || my_decl->has_name("operator<=")
+            || my_decl->has_name("operator>")
+            || my_decl->has_name("operator>=")
+            || my_decl->has_name("operator<=>")
+        )
+        )
+    {
+        return true;
+    }
+    return false;
+}
+
+
 auto function_type_node::is_compound_assignment() const
     -> bool
 {
diff --git a/source/reflect.h b/source/reflect.h
@@ -104,12 +104,11 @@ class compiler_services
     protected: [[nodiscard]] auto parse_statement(
 
         std::string_view source
-    ) & -> 
-        std::unique_ptr<statement_node>;
+    ) & -> std::unique_ptr<statement_node>;
 
 #line 113 "reflect.h2"
-    public: [[nodiscard]] virtual auto position() const& -> 
-        source_position; 
+    public: [[nodiscard]] virtual auto position() const& -> source_position;
+        
 
 #line 119 "reflect.h2"
     public: auto require(
@@ -367,24 +366,24 @@ class type_declaration
     public: [[nodiscard]] auto is_final() const& -> bool;
     public: [[nodiscard]] auto make_final() & -> bool;
 
-    public: [[nodiscard]] auto get_member_functions() const& -> 
-        std::vector<function_declaration>; 
+    public: [[nodiscard]] auto get_member_functions() const& -> std::vector<function_declaration>;
+        
 
 #line 397 "reflect.h2"
-    public: [[nodiscard]] auto get_member_objects() const& -> 
-        std::vector<object_declaration>; 
+    public: [[nodiscard]] auto get_member_objects() const& -> std::vector<object_declaration>;
+        
 
 #line 407 "reflect.h2"
-    public: [[nodiscard]] auto get_member_types() const& -> 
-        std::vector<type_declaration>; 
+    public: [[nodiscard]] auto get_member_types() const& -> std::vector<type_declaration>;
+        
 
 #line 417 "reflect.h2"
-    public: [[nodiscard]] auto get_member_aliases() const& -> 
-        std::vector<alias_declaration>; 
+    public: [[nodiscard]] auto get_member_aliases() const& -> std::vector<alias_declaration>;
+        
 
 #line 427 "reflect.h2"
-    public: [[nodiscard]] auto get_members() const& -> 
-        std::vector<declaration>; struct query_declared_value_set_functions__ret { bool out_this_in_that; bool out_this_move_that; bool inout_this_in_that; bool inout_this_move_that; };
+    public: [[nodiscard]] auto get_members() const& -> std::vector<declaration>;
+        struct query_declared_value_set_functions__ret { bool out_this_in_that; bool out_this_move_that; bool inout_this_in_that; bool inout_this_move_that; };
 
 
 
@@ -828,8 +827,8 @@ namespace meta {
     [[nodiscard]] auto compiler_services::parse_statement(
 
         std::string_view source
-    ) & -> 
-        std::unique_ptr<statement_node>
+    ) & -> std::unique_ptr<statement_node>
+
     {
         CPP2_UFCS(push_back, generated_lines, std::vector<source_line>());
         auto lines {&CPP2_UFCS_0(back, generated_lines)}; 
@@ -878,8 +877,8 @@ auto newline_pos = CPP2_UFCS(find, source, '\n');
         ); 
     }
 
-    [[nodiscard]] auto compiler_services::position() const& -> 
-        source_position
+    [[nodiscard]] auto compiler_services::position() const& -> source_position
+
     {
         return {  }; 
     }
@@ -1103,8 +1102,8 @@ declaration::declaration(declaration const& that)
     [[nodiscard]] auto type_declaration::is_final() const& -> bool { return CPP2_UFCS_0(is_type_final, (*cpp2::assert_not_null(n))); }
     [[nodiscard]] auto type_declaration::make_final() & -> bool { return CPP2_UFCS_0(make_type_final, (*cpp2::assert_not_null(n))); }
 
-    [[nodiscard]] auto type_declaration::get_member_functions() const& -> 
-        std::vector<function_declaration>
+    [[nodiscard]] auto type_declaration::get_member_functions() const& -> std::vector<function_declaration>
+
     {
         std::vector<function_declaration> ret {}; 
         for ( auto const& d : CPP2_UFCS(get_type_scope_declarations, (*cpp2::assert_not_null(n)), declaration_node::functions) ) {
@@ -1113,8 +1112,8 @@ declaration::declaration(declaration const& that)
         return ret; 
     }
 
-    [[nodiscard]] auto type_declaration::get_member_objects() const& -> 
-        std::vector<object_declaration>
+    [[nodiscard]] auto type_declaration::get_member_objects() const& -> std::vector<object_declaration>
+
     {
         std::vector<object_declaration> ret {}; 
         for ( auto const& d : CPP2_UFCS(get_type_scope_declarations, (*cpp2::assert_not_null(n)), declaration_node::objects) ) {
@@ -1123,8 +1122,8 @@ declaration::declaration(declaration const& that)
         return ret; 
     }
 
-    [[nodiscard]] auto type_declaration::get_member_types() const& -> 
-        std::vector<type_declaration>
+    [[nodiscard]] auto type_declaration::get_member_types() const& -> std::vector<type_declaration>
+
     {
         std::vector<type_declaration> ret {}; 
         for ( auto const& d : CPP2_UFCS(get_type_scope_declarations, (*cpp2::assert_not_null(n)), declaration_node::types) ) {
@@ -1133,8 +1132,8 @@ declaration::declaration(declaration const& that)
         return ret; 
     }
 
-    [[nodiscard]] auto type_declaration::get_member_aliases() const& -> 
-        std::vector<alias_declaration>
+    [[nodiscard]] auto type_declaration::get_member_aliases() const& -> std::vector<alias_declaration>
+
     {
         std::vector<alias_declaration> ret {}; 
         for ( auto const& d : CPP2_UFCS(get_type_scope_declarations, (*cpp2::assert_not_null(n)), declaration_node::aliases) ) {
@@ -1143,8 +1142,8 @@ declaration::declaration(declaration const& that)
         return ret; 
     }
 
-    [[nodiscard]] auto type_declaration::get_members() const& -> 
-        std::vector<declaration>
+    [[nodiscard]] auto type_declaration::get_members() const& -> std::vector<declaration>
+
     {
         std::vector<declaration> ret {}; 
         for ( auto const& d : CPP2_UFCS(get_type_scope_declarations, (*cpp2::assert_not_null(n)), declaration_node::all) ) {
diff --git a/source/sema.h b/source/sema.h
