Skip to content

Commit

Permalink
fix(cpp1): emit requires clause in forward declaration (#486)
Browse files Browse the repository at this point in the history 
* 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]>
  • Loading branch information
@JohelEGP @MaxSagebaum @hsutter
3 people authored Jul 30, 2023
1 parent 8351bd5 commit 08806e3
Show file tree
Hide file tree
Showing 11 changed files with 128 additions and 54 deletions.
7 changes: 7 additions & 0 deletions include/cpp2util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 5 additions & 0 deletions regression-tests/pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
element: type = {
name: std::string;
operator=: (out this, forward n: std::string) = { name = n; }
}
main: () = { }
Empty file added ...test-results/gcc-13/pure2-bugfix-for-requires-clause-in-forward-declaration.cpp.execution
View file
Empty file.
Empty file added ...ts/test-results/gcc-13/pure2-bugfix-for-requires-clause-in-forward-declaration.cpp.output
View file
Empty file.
10 changes: 8 additions & 2 deletions regression-tests/test-results/mixed-forwarding.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
5 changes: 4 additions & 1 deletion regression-tests/test-results/mixed-parameter-passing-with-forward.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
55 changes: 55 additions & 0 deletions regression-tests/test-results/pure2-bugfix-for-requires-clause-in-forward-declaration.cpp
View file
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-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{}

2 changes: 2 additions & 0 deletions ...on-tests/test-results/pure2-bugfix-for-requires-clause-in-forward-declaration.cpp2.output
View file
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)

5 changes: 4 additions & 1 deletion regression-tests/test-results/pure2-function-multiple-forward-arguments.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 5 additions & 1 deletion regression-tests/test-results/pure2-requires-clauses.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
87 changes: 38 additions & 49 deletions source/cppfront.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 08806e3

Please sign in to comment.