Coder Social home page Coder Social logo

Comments (19)

hsutter avatar hsutter commented on June 14, 2024 2

Agreed, commit c39a94c 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.

from cppfront.

neumannt avatar neumannt commented on June 14, 2024

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)?

from cppfront.

seanbaxter avatar seanbaxter commented on June 14, 2024

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.

from cppfront.

neumannt avatar neumannt commented on June 14, 2024

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).

from cppfront.

seanbaxter avatar seanbaxter commented on June 14, 2024

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.

from cppfront.

feature-engineer avatar feature-engineer commented on June 14, 2024

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.

from cppfront.

seanbaxter avatar seanbaxter commented on June 14, 2024

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

from cppfront.

feature-engineer avatar feature-engineer commented on June 14, 2024

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?

from cppfront.

seanbaxter avatar seanbaxter commented on June 14, 2024

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

from cppfront.

feature-engineer avatar feature-engineer commented on June 14, 2024

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.

from cppfront.

seanbaxter avatar seanbaxter commented on June 14, 2024

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.

from cppfront.

feature-engineer avatar feature-engineer commented on June 14, 2024

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.

from cppfront.

ljleb avatar ljleb commented on June 14, 2024

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.

from cppfront.

hsutter avatar hsutter commented on June 14, 2024

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!

from cppfront.

seanbaxter avatar seanbaxter commented on June 14, 2024

@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.

from cppfront.

hsutter avatar hsutter commented on June 14, 2024

@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:

  • By default, the above code gen is what I intend for forward parameters. Given the two calls to discard in that code, only the last is a definite last use and so forwarded (by default). That's visible and clear for programmers to reason about. (Note that if instead of two separate function calls/uses this were rewritten as a fold-expression to expand the tuple value, then that would be a single last use and should effectively forward all the pieces. Cppfront doesn't yet support fold-expressions as I haven't considered them a priority but eventually it will.)

  • To opt-into a non-default, I've been considering forward expressions, as we have out and move expressions. I've done a simple enablement of that in the above commit 41673ef . With that, calling these two functions with an rvalue argument (and assuming discard takes a copy) results in the first default/implicit forwarding function copying from t.first and moving from t.second, and the second explicit forwarding function moving from both t.first and t.second:

// invoking each of these with an rvalue std::pair argument ...
apply_implicit_forward: (forward t: std::pair<X, X>) = {
    copy_from(t.first);             // copies
    copy_from(t.second);            // moves
}
apply_explicit_forward: (forward t: std::pair<X, X>) = {
    copy_from(forward t.first);     // moves
    copy_from(forward t.second);    // moves
}

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.

from cppfront.

hsutter avatar hsutter commented on June 14, 2024

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!

from cppfront.

seanbaxter avatar seanbaxter commented on June 14, 2024

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)

from cppfront.

JohelEGP avatar JohelEGP commented on June 14, 2024

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

from cppfront.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.