hsutter / cppfront

A personal experimental C++ Syntax 2 -> Syntax 1 compiler
Other
5.48k stars 242 forks source link

How do you forward non-last-use argument? #77

Closed seanbaxter closed 1 year ago

seanbaxter commented 2 years ago

How do you forward a non-last-use argument? It has to be done when destructuring aggregates. std::apply does this variadically.

#include <string>

void g(auto&& x);

struct pair_t {
  std::string x, y;
};

f: (forward pair : pair_t) = {
  g(pair.x);  // Not a forward. How do I forward?
  g(pair.y);  // OK
}
neumannt commented 2 years ago

The grammar already allows for move annotations in calls (e.g., g(move pair.x)), perhaps we also have to support forward annotations as in g(forward pair.x)?

seanbaxter commented 2 years ago

What is the precedence on those? I think forward may have to be a postfix operator to attach at higher precedence. It needs to work on the parameter pair, not the data member .x.

neumannt commented 2 years ago

Ah, that does not work then. These call annotations are soft keywords of the call site (expression_list in the parser), not part of the expression hierarchy. Then you have to explicitly use something similar to std::forward(pair).

seanbaxter commented 2 years ago

I don't think you can use std::forward or any library call, as the parameter directive hides from you the true type of the parameter. You name the parameter and you just get an lvalue.

feature-engineer commented 2 years ago

Why support forward at all? I thought auto in, auto inout and auto out were there to let us forget about &, &&, const etc. It should just do the right thing automatically.

seanbaxter commented 2 years ago

Same reason it exists in C++, so you don't have to write the same function twice (for each forwarded parameter).

feature-engineer commented 2 years ago

Same reason it exists in C++, so you don't have to write the same function twice (for each forwarded parameter).

I may be missing something.

copy already automatically deduces l-value and r-value, in could do it too, when would you need forward?

seanbaxter commented 2 years ago

copy doesn't deduce anything. It's just pass-by-value.

feature-engineer commented 2 years ago

copy doesn't deduce anything. It's just pass-by-value.

Not according to the talk Herb gave. When there's a definite last use of a variable, it's passed as r-value as a copy argument. Watch at 1:27.

seanbaxter commented 2 years ago

When there's a definite last use of a variable, it's passed as r-value as a copy argument.

That's not argument deduction, that's just changing the value category of an argument.

Anyways, definite last use is not a sufficient treatment for these semantics, as this ticket shows.

feature-engineer commented 2 years ago

That's not argument deduction, that's just changing the value category of an argument.

So by argument deduction you also mean deducing constness and whether it's a ref or a value? I may be too inexperienced to know when this is necessary, can you give an example?

Anyways, definite last use is not a sufficient treatment for these semantics, as this ticket shows.

It could be, if we change the granularity to fields instead of whole objects.

ljleb commented 1 year ago

Anyways, definite last use is not a sufficient treatment for these semantics, as this ticket shows.

It could be, if we change the granularity to fields instead of whole objects.

Correct me if I'm wrong, but I don't think fine-grain semantics for forward parameters makes sense, depending on the design goals of the language. Let me clarify:

According to cpp core guideline F.19, any forward parameter object used more than or less than exactly once in any static code path should be brought to the attention of the human in charge. (these usages should be "flag[ged]" as they put it)

If we are to enforce guideline F.19, it seems like using the forward parameter annotation syntax for this use case could work if forward parameters have two distinct interpretations depending on usage -- so that on any static code path, either:

  1. the whole forward parameter object is used exactly once
  2. all subobjects of the forward parameter object are used exactly once (and the forward parameter object is not used anywhere for other purposes than for subobject access (i.e. pair.x))

This feels like we're over-saturating forward parameter annotations semantics with more than what they are intended to be used for (i.e. we are putting single object forwarding and aggregate subobjects forwarding in the same bucket). Unless we resort to verifying that every subobject of a forward parameter (xor the parameter object itself) is used exactly once on every static code path to ensure mutually exclusive subobject forwarding, I think using a forward parameter annotation in this way should be a compile-time error. And even if we implemented it like that, it seems to me that using fine-grain semantics for forward parameter objects in this way would make the language harder to teach and reason about.

Looking into how std::apply is implemented, its std::tuple parameter has a variadic template argument list. The type of each element of the tuple knows its own value category. When called, all tuple members are forwarded to (and in-place, at the call site of) std::invoke, which takes variadic template forward parameters.

I don't know how frequently aggregate subobjects forwarding should be used in c++ for any purpose, or whether there are better alternatives in general (for example, by calling standard functions that hide the use of such forwarding instead of defining your own).

If this kind of forwarding is a narrow feature for users in general, then (in light of how std::tuple carries the value category for each of its element types) I'd be inclined to prefer being able to wrap aggregate subobjects into something like a template<typename A> struct cpp2::forward type.

The point of cpp2::forward here would be to encode forward semantics into the type system, similarly to how value categories are an integral part of type signatures. For an incomplete example:

namespace cpp2 {
  template<typename A>
  class forward {
    A _a;

  public:
    forward(auto&& a):
      _a(std::forward<A>(a))
    {}

    ... // (deleted) constructors and operator= omitted

    ~forward() {
      ... // ensure parameter was consumed exactly once here?
      // iiuc this can and should be done at compile time (not necessarily implemented here)
    }

    A&& get() {
      return std::forward<A>(_a);
    }

    ... // more members if needed
  };

  forward(auto&& a) -> forward<decltype(a)>;
}

And then:

#include <cpp2util.h>
#include <string>

void g(auto&& x);

struct pair_t {
  cpp2::forward<std::string> x, y;
};

f: (pair : pair_t) = {
  g(pair.x);  // forwards
  g(pair.y);  // and forwards too!
  g(pair);    // forwards as a copy in this case (pair is implicitly annotated with "in")
              //   if we are to enforce guideline F.19, this code should not be allowed
              //   to compile. Ideally, the constructor of `cpp2::forward` doesn't allow
              //   an object to be forwarded more than once. Since `pair` is an in parameter,
              //   it should use the copy constructors of `x` and `y`, which should in turn
              //   attempt to forward. Since both `x` and `y` have already been forwarded,
              //   according to guideline F.19, this should result in a compile-time error
}

My intention here is that the code generator would take care of calling .get() on the cpp2::forward objects. The name forward is just used as an example handle here to convey my intent. It could also be implemented using syntax instead of just being a helper class, but as I said above (in the case where the current parameter annotation facilities are reused without altering the syntax in any way) I'm not convinced this is not working against the design goals of the language.

As a side note, of course my point of view is highly dependent on whether guideline F.19 is intended to be strictly enforced at compile time.

hsutter commented 1 year ago

I think @ljleb said it all. I don't currently aim to allow individual forwarding of members in separate non-fold expressions, so I'll close this. If there is demand for it we can always revisit it in the future. Thanks!

seanbaxter commented 1 year ago

@ljleb is wrong about how tuple works and is misreading cpp core guideline F.19 (which is also broken). You have to forward the function parameter, not a subobject of the function parameter, using the forwarding template parameter. The std::apply implementation is clear on this.

  template <typename _Fn, typename _Tuple, size_t... _Idx>
    constexpr decltype(auto)
    __apply_impl(_Fn&& __f, _Tuple&& __t, index_sequence<_Idx...>)
    {
      return std::__invoke(std::forward<_Fn>(__f),
               std::get<_Idx>(std::forward<_Tuple>(__t))...);
    }

The parameter __t is forwarded one time for each element, and then get extracts the member, keeping the same value category. The Cpp2 translator is generating broken code because the forward directive semantics are wrong.

https://godbolt.org/z/d46n8EMrs

apply: (forward t: std::tuple<int, double>) = {
  discard(t.x);
  discard(t.y);
}
auto apply(auto&& t) -> void
requires std::is_same_v<CPP2_TYPEOF(t), std::tuple<int,double>>
{ discard(t.x);
  discard(CPP2_FORWARD(t).y);
}

The forward language in D0708, which indicates definite last use, is not correct.

hsutter commented 1 year ago

@seanbaxter Thanks for the notes! Some thoughts:

For the C++ Core Guidelines: We'd love to know if F.19 is broken... can you please open an issue in the Guidelines repo to tell us how (and if possible suggest an improvement)? Thanks!

For paper d0708: I'd like to fix bugs in d0708... if the explanation of my intent below doesn't address your concerns, please let me know the forward language that you think isn't correct, and how the example is broken. Thanks!

For Cpp2/cppfront:

The second function is new functionality that needs this commit, it wouldn't compile before.

See the new regression test mixed-forwarding.cpp2 for a full working example.

Bug reports and constructive feedback are always welcome! Thanks.

hsutter commented 1 year ago

BTW, I guess your final comment turned this from a wontfix to actually supporting the example, well played! :)

Your question at top was:

f: (forward pair : pair_t) = {
  g(pair.x);  // Not a forward. How do I forward?
  g(pair.y);  // OK
}

The above is still the default. To opt into to forwarding both pieces, the answer in this commit would be to forward the first (and it doesn't hurt to do the second too if you like):

f: (forward pair : pair_t) = {
  g(  forward   pair.x);
  g(/*forward*/ pair.y);
}

At least that's the direction I had in mind, that this commit starts to explore. Thanks for the example!

seanbaxter commented 1 year ago

Now it's broken in a different way. 41673ef translates:

apply_explicit_forward: (forward t: std::pair<X, X>) = {
    discard(forward t.first);
    discard(forward t.second);
}

into

auto apply_explicit_forward(auto&& t) -> void
requires std::is_same_v<CPP2_TYPEOF(t), std::pair<X,X>>
#line 19 "mixed-forwarding.cpp2"
{   discard(CPP2_FORWARD(t.first));
    discard(CPP2_FORWARD(CPP2_FORWARD(t).second));
}

apply_explicit_forward is broken when you pass an lvalue, because that gets forwarded to discard as an xvalue. This is an illegal use of decltype--CPP2_FOWARD(t.first) will strip the reference and discard then reference collapses && back on. You have to forward the parameter, and then do member-access. The CPP2_FORWARD macro is a footgun and should not be used because it compiles illegal uses like this. https://godbolt.org/z/5f3P6WEnc shows the successful rvalue case and broken lvalue case.

The forward operator needs to be highest precedence--higher than postfix-expression, even though it's where a unary-expression would go. It needs to apply to the parameter, not to a member-access of the parameter. Both subobjects should be forwarded like: (forward t).first (forward t).second. Right now it's like: forward (t.first) (forward t).second. (due to last-definite-use)

JohelEGP commented 1 year ago

An alternative to precedence is the forwarding operator, t>>.first, which might be more in-line with the postfix operator philosophy of Cpp2.

hsutter commented 1 year ago

Agreed, commit c39a94cb7451bd55188d87c1c00848bfae2e6cc2 should address this. Now it translates

apply_explicit_forward: (forward t: std::pair<X, X>) = {
    discard(forward t.first);
    discard(forward t.second);
}

to

auto apply_explicit_forward(auto&& t) -> void
requires std::is_same_v<CPP2_TYPEOF(t), std::pair<X,X>>
#line 2 "demo.cpp2"
{   discard(CPP2_FORWARD(t).first);
    discard(CPP2_FORWARD(t).second);
}

See also the commit comments re grammar. I'm currently running with out, move, and forward all being an adjective on the entire argument (grammatically), though the implementation of forward applies std::forward specifically to the forwarded parameter... that's enough to get experience with the experiment, and then if experience shows it's used and the difference matters then I could parse forward differently. Thanks!

@JohelEGP True, and Barry is a source of good ideas. For Cpp2 though, one of my stakes in the ground is that declaration syntax is consistent with use syntax, so for Cpp2 I'd want to change the syntax in both places (declaration and use) or neither rather than introduce an asymmetry there.