Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] No way to declare std::function that use references in cpp2 #343

Closed
filipsajdak opened this issue Apr 9, 2023 · 21 comments · Fixed by #1183
Closed

[BUG] No way to declare std::function that use references in cpp2 #343

filipsajdak opened this issue Apr 9, 2023 · 21 comments · Fixed by #1183
Labels
bug Something isn't working

Comments

@filipsajdak
Copy link
Contributor

In the current implementation of cppfront (5fa2707) in cpp2, there is no way to declare std::function that uses references.

The following examples failed to work:

f1: () -> std::function<std:string(std::string const&)> = { // error: missing = before function body (at '<')
    return :(s : std::string) -> std::string = {
        return s + " World!";
    };
}
f2: () -> std::function<(std::string) -> std::string> = { // error: missing = before function body (at '<')
    return :(s : std::string) -> std::string = {
        return s + " World!";
    };
}
f3: () -> std::function<std::string(std::string)> = {
    return :(s : std::string) -> std::string = {
        return s + " World!";
    };
}

The last example passes cppfront, but generates different types of arguments:

[[nodiscard]] auto f3() -> std::function<std::string(std::string)>{
    return [](cpp2::in<std::string> s) -> std::string{
        return s + " World!"; 
    }; 
}

I have also tried the code that was presented on the wiki (https://github.com/hsutter/cppfront/wiki/Design-note%3A-Postfix-operators#-and-)

f: (i:int) -> * (:int) -> string = {
    return :(j:int) -> std::string = {
        return std::to_string(j);
    };
}

but it also failed:

bug_std_function.cpp2(1,17): error: '*'/'const' type qualifiers must be followed by a type name or '_' wildcard (at '(')
bug_std_function.cpp2(1,19): error: missing function return after -> (at 'int')

I would like to be able use cpp2 function types as templates arguments, or function argument and return types.

From what I understand from the wiki my code should be written in the following way:

f: () -> std::function<(:std::string) -> std::string> = {
    return :(s : std::string) -> std::string = {
        return s + " World!";
    };
}
@filipsajdak filipsajdak added the bug Something isn't working label Apr 9, 2023
@JohelEGP
Copy link
Contributor

JohelEGP commented Apr 9, 2023

Seems like there are no unnamed parameters: https://godbolt.org/z/zG7xGh3Kz.

f: (:int) = { }
main.cpp2(2,10): error: invalid parameter list (at ':')
main.cpp2(2,14): error: ill-formed initializer (at ')')
main.cpp2(1,12): error: ill-formed initializer (at '{')
main.cpp2(1,1): error: unexpected text at end of Cpp2 code section (at 'main')
main.cpp2(1,0): error: parse failed for section starting here

Some test cases: https://godbolt.org/z/oxrhsecdq.

main: (args) = {
  a: * int;
  a = args.argc;

  b: * ();
  b = :() = { };

  c: * (_: int);
  c = :(_) = { };

  d: * (_: int) -> int;
  d = :(x) -> _ = x;

  e: * (_: * ());
  e = :(x) -> _ = x;
}

@orent
Copy link

orent commented Apr 11, 2023

While I generally agree with the elimination of references in favor of in/out/inout arguments there should still be a way to spell them because they remain a part of the underlying object model - just doesn’t have to be particularly short.

How about std::reference or something similar?

@JohelEGP
Copy link
Contributor

I agree. I don't think you can rewrite this in pure Cpp2: https://godbolt.org/z/csKcPTrzx.

using Int = const int&;

min: (l: Int, r: Int) -> Int = {
  if l < r {
    return l;
  } else {
    return r;
  }
}

main: () = {
  x := 0;
  y := 1;
  [[assert: min(x, y)& == x& ]]
}

@filipsajdak
Copy link
Contributor Author

@JohelEGP not the same but works: https://godbolt.org/z/zKnn7re3d

min: (forward l: int, forward r: int) -> forward int = {
  if l < r {
    return l;
  } else {
    return r;
  }
}

main: () = {
  x := 0;
  y := 1;
  [[assert: min(x, y)& == x& ]]
  y++; // this is needed to supress move from last use
}

@leejy12
Copy link

leejy12 commented Apr 13, 2023

@filipsajdak But that min implementation won't work when passing rvalues.

@filipsajdak
Copy link
Contributor Author

@leejy12 in that specific use case, it is a feature. You don't want to check an address to a temporary variable.

@leejy12
Copy link

leejy12 commented Apr 13, 2023

@filipsajdak True, but I'm still looking for a way to correctly implement std::min in pure Cpp2.
I think this comment by @AbhinavK00 is the correct analysis.

Why in parameters? The above examples make perfect sense if we function takes something with size more than 2 pointers (I think it is any UDT now).

I think the right diagnosis isn't to ban forward return of in arguments but to pass the argument by reference everytime the function forward returns it. How would we write functions such as std::min etc. with this?

When we're forward returning an in parameter, it should be passed as const T& not cpp2::in<T>.

So Cpp2's implementation of std::min could look like

min: <T> (a: T, b: T) -> forward _ = {
    if a < b {
        return a;
    }
    else {
        return b;
    }
}

which will be compiled to

template <typename T>
auto min(const T& a, const T& b) -> auto&& {
    if (a < b) {
        return a;
    }
    else {
        return b;
    }
}

This is pretty much what I already wrote in #248 (comment). What do you think?

@JohelEGP
Copy link
Contributor

Remember that the min of #343 (comment) is not a template. Although this may be similar to #193 (comment). How often do you write a non-templated function whose result is a read-only argument (so identity matters), that in parameter passing would optimize to pass by value (so it may not be future-proof or portable)?

@AbhinavK00
Copy link

AbhinavK00 commented Apr 13, 2023

How often do you write a non-templated function whose result is a read-only argument

The current design doesn't even allow us to write function templates that return an in argument.
Edit: Issue #317 which restores old behaviour of cpp2::in can also proceed with a check of whether the argument is being forward returned and then just pass it by const&.

@JohelEGP
Copy link
Contributor

The current design doesn't even allow us to write function templates that return an in argument.

I don't understand. Can you give an example?

@AbhinavK00
Copy link

From a comment in this issue only, the following isn't allowed in cpp2

min: <T> (a: T, b: T) -> forward _ = {
    if a < b {
        return a;
    }
    else {
        return b;
    }
}

1 similar comment
@AbhinavK00
Copy link

From a comment in this issue only, the following isn't allowed in cpp2

min: <T> (a: T, b: T) -> forward _ = {
    if a < b {
        return a;
    }
    else {
        return b;
    }
}

@JohelEGP
Copy link
Contributor

JohelEGP commented Apr 13, 2023

You're right: https://godbolt.org/z/MaWdqG1dM.

I changed -> forward _ to -> forward T, honest to the signature of std::min.
Then I tried adding parentheses around the returned parameters. cppfront then accepts: https://godbolt.org/z/1M9asnh1c.
The generated code returns T&.

Changing back to -> forward _ makes the declared return type be auto&&, which results in working code: https://godbolt.org/z/rra8EW8z1.

Knowing that a declared return type of auto&& or decltype(auto), and redundant parentheses, can change meaning from "type and value category of the expression" to "declared type of the data member in the data member access expression" (see this "1)" and this "2)"), I next tried with a type: https://godbolt.org/z/Kszo8EPfc.
Looks like not using T directly as the parameter's type results in cppfront wrapping the declared parameter type in cpp2::in, which results in a parameter type that can't be deduced from its argument. That's #367.

main.cpp2:10:41: note: candidate template ignored: couldn't infer template argument 'T'
template<typename T> [[nodiscard]] auto min(cpp2::in<range<T>> r) -> auto&&{
                                        ^

@AbhinavK00
Copy link

But aren't those just work-arounds? You're clearly violating a cpp2's rule, can also be thought of as a bug in transpiler, no?

@JohelEGP
Copy link
Contributor

1681415614
-- https://github.com/hsutter/708/blob/main/708.pdf

Sometimes, the "what" is a reference. How do we spell that in Cpp2?

@JohelEGP
Copy link
Contributor

I think the min example is a bit more involving than "just" a "reference". Some callers only care about the return value. Others, also about the identity of the returned reference. And being a reference to const, means being initializable from prvalues, which has its own set of implications.

@AbhinavK00
Copy link

Sometimes, the "what" is a reference. How do we spell that in Cpp2?

Do we really NEED to? I think the conventions in the paper do encompass all the use cases. A lanaguage like rust has 3 conventions and we can express everything (not really conventions but close), Val has a better model with 4 conventions and cppfront has 6. It's not like we were losing expressivity in any case until the decision was made to make in not forward returnable which I'd say was not well-thought.

@JohelEGP
Copy link
Contributor

Do we really NEED to?

I do wonder. Maybe this calls for waiting for real world use-cases. Similar to how copy is necessary.

The OP's use case is just a bug/lack of feature, because there's no way to describe a function's type, like (: std::string) -> std::string.

@JohelEGP
Copy link
Contributor

See #387 (comment) for how the grammar may be change for this.

@JohelEGP
Copy link
Contributor

JohelEGP commented Jun 22, 2023

I was going to suggest this as a workaround while we wait for actual function type support: https://cpp2.godbolt.org/z/arEsxc3Gf.

fn_t: <V: _> type == std::remove_pointer_t<decltype(V)>;
f: () -> std::function<fn_t<+ :(_: std::string) -> std::string = {}>> = {
  return :(s: std::string) -> std::string = { return s + " World!"; };
}
main: () = std::cout << f()("Hello");

But:

<source>...

example.cpp2: error: unexpected end of source file

Compiler returned: 1

JohelEGP added a commit to JohelEGP/cppfront that referenced this issue Jun 24, 2023
Uses the same grammar for functions, _function-type_.
Parameter identifiers must be named `_` and can't be omitted.
It's generally needed to specify the `throws` qualifier
so that it doesn't lower to a `noexcept` function type.
Some examples:
```Cpp2
main: () = {
  f0: * () throws = :()      = {};
  f6: * (move _: i32) throws = :(move x: i32) = {};
  [[assert: inspect f0 -> bool {
    is ()          = (std::terminate(), false);
    is * () throws = true;
    is _           = false;
  }]]
}

// Test case from hsutter#343.
f2: () -> std::function<(_: std::string) throws -> std::string> = {
  return :(s: std::string) -> std::string = { return s + " World!"; };
}

// Adapted from <https://github.com/hsutter/cppfront/wiki/Design-note%3A-Postfix-operators>.
            f:   (x: i32)        -> * (_: i32) throws -> std::string = +:(x: i32) -> std::string = "";
postfix_operators: () = {
  [[assert: f is (_: i32) throws -> * (_: i32) throws -> std::string]]
  //             /                  |  |
  //            /                   |  |
  [[assert: f(42)        is         * (_: i32) throws -> std::string]]
  //               ________________/   |
  //              /                    |
  [[assert: f(42)*         is         (_: i32) throws -> std::string]]
  //                   _______________/
  //                  /
  [[assert: (f(42)*)(1)                is                std::string]]
}
```
JohelEGP added a commit to JohelEGP/cppfront that referenced this issue Jun 27, 2023
Add _function-type_ as an alternative production of _type-id_.
A parameter's identifier must be named `_`.
`throws` needs to be specified for non Cpp1-`noexcept` function types.

Some examples:
```Cpp2
main: () = {
  f0: * () throws = :()      = {};
  f7: * (move _: i32) throws = :(move x: i32) = {};
  [[assert: inspect f0 -> bool {
    is ()          = (std::terminate(), false);
    is * () throws = true;
    is _           = false;
  }]]
}

// Test case from hsutter#343.
f2: () -> std::function<(_: std::string) throws -> std::string> = {
  return :(s: std::string) -> std::string = { return s + " World!"; };
}

// Adapted from <https://github.com/hsutter/cppfront/wiki/Design-note%3A-Postfix-operators>.
            f:   (x: i32)        -> * (_: i32) throws -> std::string = +:(x: i32) -> std::string = "";
postfix_operators: () = {
  [[assert: f is (_: i32) throws -> * (_: i32) throws -> std::string]]
  //             /                  |  |
  //            /                   |  |
  [[assert: f(42)        is         * (_: i32) throws -> std::string]]
  //               ________________/   |
  //              /                    |
  [[assert: f(42)*         is         (_: i32) throws -> std::string]]
  //                   _______________/
  //                  /
  [[assert: (f(42)*)(1)                is                std::string]]
}
```
JohelEGP added a commit to JohelEGP/cppfront that referenced this issue Jun 28, 2023
Add _function-type_ as an alternative production of _type-id_.
A parameter's identifier must be named `_`.
`throws` needs to be specified for non Cpp1-`noexcept` function types.

Some examples:
```Cpp2
main: () = {
  f0: * () throws = :()      = {};
  f7: * (move _: i32) throws = :(move x: i32) = {};
  [[assert: inspect f0 -> bool {
    is ()          = (std::terminate(), false);
    is * () throws = true;
    is _           = false;
  }]]
}

// Test case from hsutter#343.
f2: () -> std::function<(_: std::string) throws -> std::string> = {
  return :(s: std::string) -> std::string = { return s + " World!"; };
}

// Adapted from <https://github.com/hsutter/cppfront/wiki/Design-note%3A-Postfix-operators>.
            f:   (x: i32)        -> * (_: i32) throws -> std::string = +:(x: i32) -> std::string = "";
postfix_operators: () = {
  [[assert: f is (_: i32) throws -> * (_: i32) throws -> std::string]]
  //             /                  |  |
  //            /                   |  |
  [[assert: f(42)        is         * (_: i32) throws -> std::string]]
  //               ________________/   |
  //              /                    |
  [[assert: f(42)*         is         (_: i32) throws -> std::string]]
  //                   _______________/
  //                  /
  [[assert: (f(42)*)(1)                is                std::string]]
}
```
JohelEGP added a commit to JohelEGP/cppfront that referenced this issue Jun 28, 2023
Add _function-type_ as an alternative production of _type-id_.
A parameter's identifier must be named `_`.
`throws` needs to be specified for non Cpp1-`noexcept` function types.

Some examples:
```Cpp2
main: () = {
  f0: * () throws = :()      = {};
  f7: * (move _: i32) throws = :(move x: i32) = {};
  [[assert: inspect f0 -> bool {
    is ()          = (std::terminate(), false);
    is * () throws = true;
    is _           = false;
  }]]
}

// Test case from hsutter#343.
f2: () -> std::function<(_: std::string) throws -> std::string> = {
  return :(s: std::string) -> std::string = { return s + " World!"; };
}

// Adapted from <https://github.com/hsutter/cppfront/wiki/Design-note%3A-Postfix-operators>.
            f:   (x: i32)        -> * (_: i32) throws -> std::string = +:(x: i32) -> std::string = "";
postfix_operators: () = {
  [[assert: f is (_: i32) throws -> * (_: i32) throws -> std::string]]
  //             /                  |  |
  //            /                   |  |
  [[assert: f(42)        is         * (_: i32) throws -> std::string]]
  //               ________________/   |
  //              /                    |
  [[assert: f(42)*         is         (_: i32) throws -> std::string]]
  //                   _______________/
  //                  /
  [[assert: (f(42)*)(1)                is                std::string]]
}
```
JohelEGP added a commit to JohelEGP/cppfront that referenced this issue Aug 20, 2023
Add _function-type_ as an alternative production of _type-id_.
A parameter's identifier must be named `_`.
`!throws` needs to be specified for Cpp1-`noexcept` function types.

Some examples:
```Cpp2
main: () = {
  f0: * () = :()      = {};
  f7: * (move _: i32) = :(move x: i32) = {};
  [[assert: inspect f0 -> bool {
    is () !throws = (std::terminate(), false);
    is * ()       = true;
    is _          = false;
  }]]
}

// Test case from hsutter#343.
f2: () -> std::function<(_: std::string) -> std::string> = {
  return :(s: std::string) -> std::string = { return s + " World!"; };
}

// Adapted from <https://github.com/hsutter/cppfront/wiki/Design-note%3A-Postfix-operators>.
            f:   (x: i32) -> * (_: i32) -> std::string = :(x: i32) -> std::string = "";
postfix_operators: () = {
  [[assert: f is (_: i32) -> * (_: i32) -> std::string]]
  //             /           |  |
  //            /            |  |
  [[assert: f(42)     is     * (_: i32) -> std::string]]
  //               _________/   |
  //              /             |
  [[assert: f(42)*     is      (_: i32) -> std::string]]
  //                   ________/
  //                  /
  [[assert: (f(42)*)(1)         is         std::string]]
}
@hsutter
Copy link
Owner

hsutter commented Jul 27, 2024

Thanks! Catching up, the original example f2 now works with #1183, except you have to write the parameter name (i.e., it's a full function signature just as if you were writing a function, same syntax)... this now works:

f2: () -> std::function<(x: std::string) -> std::string> = { // ok with #1183
    return :(s : std::string) -> std::string = {
        return s + " World!";
    };
}

main: () = {
    std::cout << f2()("Hello");  // prints: Hello World!
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
6 participants