Fix support for function multiple forward arguments - close #279 #293
Conversation
The current implementation correctly handle one forward argument
- it produces correct requires clause.
When there are more forward arguments like in following code:
```cpp
fun: (forward s1 : std::string, forward s2 : std::string, forward s3 : std::string) = {
// ...
}
```
generates:
```cpp
requires std::is_same_v<CPP2_TYPEOF(s1), std::string>requires std::is_same_v<CPP2_TYPEOF(s2), std::string>requires std::is_same_v<CPP2_TYPEOF(s3), std::string>
```
This change correct this behaviour and creates requires clause
that joins multiple clauses with `&&`:
```cpp
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>)
```
filipsajdak
changed the title
|
Looks good, thanks! |
|
This has also spurred me to push 3e5a83a to get around to adding requires-clause's to Cpp2 UDTs and functions, which will get merged with any generated Also: I think the requires-clauses should actually go out on the function declarations too, not only on the definitions as it's been doing all this time now even though that seemed to work. However, my first attempt to emit them also on the declarations generated code that worked on Clang and MSVC, but caused 'ambiguous call' errors on GCC which seemed to be treating the declaration and definition as two functions... for now I've committed just the basic requires-clause support, but I'll take another look at moving them up to function declarations once I look into what was happening with GCC. |
|
Ah, I found the answer... it was a bug in GCC 10.x that was fixed in GCC 11.0 (March 2021). So I can easily go and emit the requires-clauses also on the function declarations (just move the branch up a little from line 4734 to line 4646). But that would be at the cost of breaking concrete-type forward parameters on GCC 10... which happens to be my main GCC test compiler. Sigh. I'd probably do it if I could easily update my GCC environment (I'm using Ubuntu 20.04 LTS) but it was a pain the last time I tried and I'm loath to spend time on it. |
|
Documented in c31320d |
|
@hsutter I played a little with the custom requires for functions, and they work even for things like: fun: <X : type, Y : type, Z : type> (forward x : X, forward s1 : std::string, forward y : Y, forward s2 : std::string, forward z : Z, forward s3 : std::string)
requires std::is_same_v<X, int>
&& (sizeof(Z) > 2u + 2u || sizeof(X) + sizeof(Y) > (1u + 1u) * 2)
= {
std::cout << x << s1 << y << s2 << z << s3 << std::endl;
}
main: () = {
a : std::string = "a";
b : std::string = "b";
fun(42, a, "_string_literal_", b, 24L, std::string("b"));
b = "";
}Generates: #define CPP2_USE_MODULES Yes
#include "cpp2util.h"
#line 1 "../tests/bug_multiple_forward_arguments.cpp2"
template<typename X, typename Y, typename Z> auto fun(X&& x, auto&& s1, Y&& y, auto&& s2, Z&& z, auto&& s3) -> void;
#line 8 "../tests/bug_multiple_forward_arguments.cpp2"
auto main() -> int;
//=== Cpp2 definitions ==========================================================
#line 1 "../tests/bug_multiple_forward_arguments.cpp2"
template<typename X, typename Y, typename Z> auto fun(X&& x, auto&& s1, Y&& y, auto&& s2, Z&& z, auto&& s3) -> void
requires (std::is_same_v<X,int> && (cpp2::cmp_greater(sizeof(Z),2u + 2u) || cpp2::cmp_greater(sizeof(X) + sizeof(Y),(1u + 1u) * 2)) && 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 4 "../tests/bug_multiple_forward_arguments.cpp2"
{
std::cout << CPP2_FORWARD(x) << CPP2_FORWARD(s1) << CPP2_FORWARD(y) << CPP2_FORWARD(s2) << CPP2_FORWARD(z) << CPP2_FORWARD(s3) << std::endl;
}
auto main() -> int{
std::string a {"a"};
std::string b {"b"};
fun(42, std::move(a), "_string_literal_", b, 24L, std::string("b"));
b = "";
}Compiles and returns: |
|
@hsutter Regarding gcc-11 I have try to create a Dockerfile that has Ubuntu 20.04 with gcc-11: # Use the official Ubuntu 20.04 base image
FROM ubuntu:20.04
# Set environment variables to prevent tzdata from prompting during installation
ENV DEBIAN_FRONTEND=noninteractive
# Update and upgrade the system packages
RUN apt-get update && apt-get upgrade -y
# Install dependencies required for adding the repository and installing gcc-11
RUN apt-get install -y --no-install-recommends \
software-properties-common \
build-essential \
wget \
ca-certificates \
gnupg \
lsb-release
# Add the repository for gcc-11 and install it
RUN add-apt-repository -y ppa:ubuntu-toolchain-r/test && \
apt-get update && \
apt-get install -y --no-install-recommends gcc-11 g++-11
# Set gcc-11 and g++-11 as default compilers
RUN update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-11 100 && \
update-alternatives --install /usr/bin/g++ g++ /usr/bin/g++-11 100
# Clean up cache and temporary files
RUN apt-get clean && \
rm -rf /var/lib/apt/lists/*
# Set the working directory
WORKDIR /app
# Set the default command to run when starting the container
CMD ["/bin/bash"]Build with: docker build -t cppfront-ubuntu-20.04-gcc11 .Then I was able to easily compile cppfront: docker run --rm -v "$(pwd)":/app -w /app cppfront-ubuntu-20.04-gcc11 g++ -std=c++20 -o cppfront-gcc11 source/cppfront.cppAnd later can run it with: docker run --rm -v "$(pwd)":/app -w /app cppfront-ubuntu-20.04-gcc11 ./cppfront-gcc11 -hIf you are not willing to use docker then you can install gcc-11 with: add-apt-repository -y ppa:ubuntu-toolchain-r/test && apt-get update && apt-get install -y --no-install-recommends gcc-11 g++-11 |
… (hsutter#293) * Add support for function multiple forward arguments The current implementation correctly handle one forward argument - it produces correct requires clause. When there are more forward arguments like in following code: ```cpp fun: (forward s1 : std::string, forward s2 : std::string, forward s3 : std::string) = { // ... } ``` generates: ```cpp requires std::is_same_v<CPP2_TYPEOF(s1), std::string>requires std::is_same_v<CPP2_TYPEOF(s2), std::string>requires std::is_same_v<CPP2_TYPEOF(s3), std::string> ``` This change correct this behaviour and creates requires clause that joins multiple clauses with `&&`: ```cpp 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>) ``` * Add regression test for multiple forward arguments
The current implementation correctly handles one forward argument - only then produces the correct requires clause.
When there are more forward arguments, like in the following code:
fun: (forward s1 : std::string, forward s2 : std::string, forward s3 : std::string) = { // ... }Generates:
This change corrects this behavior and creates requires clause
that joins multiple clauses with
&&surrounded by parentheses: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>)The change maintains the old behavior of not adding parentheses for only one requires clause - all regression tests pass.
Closes #279