fix(cpp1): emit requires clause in forward declaration (hsutter#486)

* fix(cpp1): emit requires clause in forward declaration * refactor: prevent warning on token-eating macro Co-authored-by: Max Sagebaum <[email protected]> Signed-off-by: Johel Ernesto Guerrero Peña <[email protected]> * test: add `test-results/gcc-13` * Delete gcc-version.output Signed-off-by: Herb Sutter <[email protected]> --------- Signed-off-by: Johel Ernesto Guerrero Peña <[email protected]> Signed-off-by: Herb Sutter <[email protected]> Co-authored-by: Max Sagebaum <[email protected]> Co-authored-by: Herb Sutter <[email protected]>
zaucy · Dec 5, 2023 · 26f1061 · 26f1061
1 parent a3d28b4
commit 26f1061
Show file tree

Hide file tree

Showing 11 changed files with 128 additions and 54 deletions.
diff --git a/include/cpp2util.h b/include/cpp2util.h
@@ -1771,5 +1771,12 @@ inline constexpr auto as_() -> decltype(auto)
 
 using cpp2::cpp2_new;
 
+// Workaround GCC 10 not supporting requires in forward declarations in some cases.
+// See commit 5a0d77f8e297902c0b9712c5aafb6208cfa4c139.
+#if !defined(__clang__) && defined(__GNUC__) && __GNUC__ == 10
+    #define CPP2_REQUIRES(...) /* empty */
+#else
+    #define CPP2_REQUIRES(...) requires (__VA_ARGS__)
+#endif
 
 #endif
diff --git a/regression-tests/pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2 b/regression-tests/pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2
@@ -0,0 +1,5 @@
+element: type = {
+  name: std::string;
+  operator=: (out this, forward n: std::string) = { name = n; }
+}
+main: () = { }
diff --git a/...test-results/gcc-13/pure2-bugfix-for-requires-clause-in-forward-declaration.cpp.execution b/...test-results/gcc-13/pure2-bugfix-for-requires-clause-in-forward-declaration.cpp.execution
diff --git a/...ts/test-results/gcc-13/pure2-bugfix-for-requires-clause-in-forward-declaration.cpp.output b/...ts/test-results/gcc-13/pure2-bugfix-for-requires-clause-in-forward-declaration.cpp.output
diff --git a/regression-tests/test-results/mixed-forwarding.cpp b/regression-tests/test-results/mixed-forwarding.cpp
@@ -25,11 +25,17 @@ auto copy_from(auto x) -> void;
 auto use(auto const& x) -> void;
 
 // invoking each of these with an rvalue std::pair argument ...
-auto apply_implicit_forward(auto&& t) -> void;
+auto apply_implicit_forward(auto&& t) -> void
+CPP2_REQUIRES (std::is_same_v<CPP2_TYPEOF(t), std::pair<X,X>>)
+#line 16 "mixed-forwarding.cpp2"
+;
 
 
 #line 20 "mixed-forwarding.cpp2"
-auto apply_explicit_forward(auto&& t) -> void;
+auto apply_explicit_forward(auto&& t) -> void
+CPP2_REQUIRES (std::is_same_v<CPP2_TYPEOF(t), std::pair<X,X>>)
+#line 20 "mixed-forwarding.cpp2"
+;
 
 
 #line 25 "mixed-forwarding.cpp2"

diff --git a/regression-tests/test-results/mixed-parameter-passing-with-forward.cpp b/regression-tests/test-results/mixed-parameter-passing-with-forward.cpp
@@ -23,7 +23,10 @@ auto parameter_styles(
     std::string& c, 
     std::string&& d, 
     auto&& e
-    ) -> void;
+    ) -> void
+CPP2_REQUIRES (std::is_same_v<CPP2_TYPEOF(e), std::string>)
+#line 8 "mixed-parameter-passing-with-forward.cpp2"
+;
 
 #line 42 "mixed-parameter-passing-with-forward.cpp2"
 [[nodiscard]] auto main() -> int;

diff --git a/regression-tests/test-results/pure2-bugfix-for-requires-clause-in-forward-declaration.cpp b/regression-tests/test-results/pure2-bugfix-for-requires-clause-in-forward-declaration.cpp
@@ -0,0 +1,55 @@
+
+#define CPP2_USE_MODULES         Yes
+
+//=== Cpp2 type declarations ====================================================
+
+
+#include "cpp2util.h"
+
+#line 1 "pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2"
+class element;
+
+
+//=== Cpp2 type definitions and function declarations ===========================
+
+#line 1 "pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2"
+class element {
+  private: std::string name; 
+  public: explicit element(auto&& n)
+CPP2_REQUIRES (std::is_same_v<CPP2_TYPEOF(n), std::string>)
+#line 3 "pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2"
+  ;
+#line 3 "pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2"
+  public: auto operator=(auto&& n) -> element& 
+CPP2_REQUIRES (std::is_same_v<CPP2_TYPEOF(n), std::string>)
+#line 3 "pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2"
+  ;
+
+  public: element(element const&) = delete; /* No 'that' constructor, suppress copy */
+  public: auto operator=(element const&) -> void = delete;
+#line 4 "pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2"
+};
+auto main() -> int;
+
+
+//=== Cpp2 function definitions =================================================
+
+
+#line 3 "pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2"
+  element::element(auto&& n)
+requires (std::is_same_v<CPP2_TYPEOF(n), std::string>)
+                                                    : name{ CPP2_FORWARD(n) }
+#line 3 "pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2"
+  {}
+#line 3 "pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2"
+  auto element::operator=(auto&& n) -> element& 
+requires (std::is_same_v<CPP2_TYPEOF(n), std::string>)
+#line 3 "pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2"
+  {
+                                                    name = CPP2_FORWARD(n);
+                                                    return *this;
+#line 3 "pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2"
+  }
+
+auto main() -> int{}
+
diff --git a/...on-tests/test-results/pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2.output b/...on-tests/test-results/pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2.output
@@ -0,0 +1,2 @@
+pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2... ok (all Cpp2, passes safety checks)
+
diff --git a/regression-tests/test-results/pure2-function-multiple-forward-arguments.cpp b/regression-tests/test-results/pure2-function-multiple-forward-arguments.cpp
@@ -11,7 +11,10 @@
 //=== Cpp2 type definitions and function declarations ===========================
 
 #line 1 "pure2-function-multiple-forward-arguments.cpp2"
-auto fun(auto&& s1, auto&& s2, auto&& s3) -> void;
+auto fun(auto&& s1, auto&& s2, auto&& s3) -> void
+CPP2_REQUIRES (std::is_same_v<CPP2_TYPEOF(s1), std::string> && std::is_same_v<CPP2_TYPEOF(s2), std::string> && std::is_same_v<CPP2_TYPEOF(s3), std::string>)
+#line 1 "pure2-function-multiple-forward-arguments.cpp2"
+;
 
 
 #line 5 "pure2-function-multiple-forward-arguments.cpp2"

diff --git a/regression-tests/test-results/pure2-requires-clauses.cpp b/regression-tests/test-results/pure2-requires-clauses.cpp
@@ -31,7 +31,11 @@ class X {
 };
 
 template<typename T, typename U> [[nodiscard]] auto f
-    (auto&& a, auto&& b) -> int; 
+    (auto&& a, auto&& b) -> int
+CPP2_REQUIRES (std::is_same_v<T,int> && std::is_same_v<U,int> && std::is_same_v<CPP2_TYPEOF(a), int> && std::is_same_v<CPP2_TYPEOF(b), int>)
+#line 10 "pure2-requires-clauses.cpp2"
+;
+
 
 #line 18 "pure2-requires-clauses.cpp2"
 auto main() -> int;

diff --git a/source/cppfront.cpp b/source/cppfront.cpp
@@ -5545,11 +5545,47 @@ class cppfront
                 }
             }
 
-            //  *** LOCATION (A) -- SEE NOTE REGARDING (A) BELOW
+            auto const emit_requires_clause = [&]() {
+                if (
+                    n.requires_clause_expression
+                    || !function_requires_conditions.empty()
+                    )
+                {
+                    printer.print_extra("\n");
+                    printer.ignore_alignment( true, n.position().colno + 4 );
+                    if (printer.get_phase() == printer.phase1_type_defs_func_decls) {
+                        // Workaround GCC 10 not supporting requires in forward declarations in some cases.
+                        // See commit 5a0d77f8e297902c0b9712c5aafb6208cfa4c139.
+                        printer.print_extra("CPP2_REQUIRES (");
+                    }
+                    else {
+                        printer.print_extra("requires (");
+                    }
+
+                    if (n.requires_clause_expression) {
+                        emit(*n.requires_clause_expression);
+                        if (!function_requires_conditions.empty()) {
+                            printer.print_extra(" && ");
+                        }
+                    }
+
+                    if (!function_requires_conditions.empty()) {
+                        printer.print_extra(function_requires_conditions.front());
+                        for (auto it = std::cbegin(function_requires_conditions)+1; it != std::cend(function_requires_conditions); ++it) {
+                            printer.print_extra(" && " + *it);
+                        }
+                    }
+
+                    printer.print_extra(")");
+                    function_requires_conditions = {};
+                    printer.ignore_alignment( false );
+                }
+            };
 
             //  If we're only emitting declarations, end the function declaration
             if (printer.get_phase() == printer.phase1_type_defs_func_decls)
             {
+                emit_requires_clause();
                 printer.print_cpp2( ";\n", n.position() );
                 //  Note: Not just early "return;" here because we may need to
                 //  recurse to emit the generated operator= declarations too,
@@ -5639,54 +5675,7 @@ class cppfront
 
                 printer.preempt_position_push( n.equal_sign );
 
-                //  *** NOTE =====================================================
-                // 
-                //      This branch to emit the requires-clause should maybe be
-                //      moved to location (A) above, so that it's also emitted
-                //      on the function declaration. But moving it to (A) triggers
-                //      a bug in GCC 10.x (that was fixed in 11.x), where it would
-                //      break using a 'forward' parameter of a concrete type and
-                //      also explicitly user-written requires-clauses that do
-                //      similar decltype tests.
-                // 
-                //      I don't want to neednessly break compatibility with a
-                //      decently conforming C++20 compiler that works well for
-                //      everything else that Cpp2 needs from C++20. If the
-                //      'requires' down here doesn't cause a problem, I'll keep
-                //      it here for now... if we do encounter a reason it needs to
-                //      also be on the declaration, move this code to (A).
-                // 
-                //  Handle requires clause - an explicit one the user wrote,
-                //  and/or any conditions we generated while processing the
-                //  parameters (i.e., forwarding a concrete type)
-                if (
-                    n.requires_clause_expression
-                    || !function_requires_conditions.empty()
-                    )
-                {
-                    printer.print_extra("\n");
-                    printer.ignore_alignment( true, n.position().colno + 4 );
-                    printer.print_extra("requires (");
-
-                    if (n.requires_clause_expression) {
-                        emit(*n.requires_clause_expression);
-                        if (!function_requires_conditions.empty()) {
-                            printer.print_extra(" && ");
-                        }
-                    }
-
-                    if (!function_requires_conditions.empty()) {
-                        printer.print_extra(function_requires_conditions.front());
-                        for (auto it = std::cbegin(function_requires_conditions)+1; it != std::cend(function_requires_conditions); ++it) {
-                            printer.print_extra(" && " + *it);
-                        }
-                    }
-
-                    printer.print_extra(")");
-                    function_requires_conditions = {};
-                    printer.ignore_alignment( false );
-                }
-                //  *** END NOTE =================================================
+                emit_requires_clause();
 
                 emit(
                     *n.initializer,
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2... ok (all Cpp2, passes safety checks)