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

Statements before and between member initializers in special member functions #518

Closed
wants to merge 12 commits into from
Closed

Statements before and between member initializers in special member functions #518

wants to merge 12 commits into from

Conversation

jbatez
Copy link
Contributor

@jbatez jbatez commented Jun 18, 2023

This is a proposed solution for Issue #475

Instead of allowing only out parameter assignments before and between member initializers in special member functions, this change allows all statements. When generating constructors, it uses IIFE's and the comma operator to insert statements in the member initializer list. And when generating assignment operators, it does exactly what you'd expect.

Since statements can now go before or after member initializers, it leads to the question: where do statements go in a function with only implicit member initializers? I came up with the following rules:

  1. By default, statements are inserted as late as possible while still maintaining the given order relative to explicit member initializers. This means a function without any explicit member initializers will have its statements inserted after all the implicit initializers.
  2. To insert statements before the implicit initializers, add a special this = that statement after the leading statements. Note: this only works for functions that include that as a parameter. If we want something similiar for functions without that, we could add a this = {} or this = default syntax, but I have a harder time imagining real-world use-cases for that.

This change also makes implicit this.member = that.member initialization opt-in. Without an explicit this = that statement in the body, that functions default to the same default initialization behavior as all the other special member functions. I personally find this more consistent with the rest of the language and less surprising, but I'm open to other ideas. I've updated the copyable metatype accordingly.

I've also gotten rid of implicit member assignment in explicit assignment (i.e. inout this) operators. The way I see it, if you're writing custom assignment operators, it's because there's something special about your type that can't be described by defaults. Implicit assignment operators (i.e. the ones that get generated alongside constructors when you write out this) still assign to every member to match the corresponding constructor, and you can still use this = that to opt-in to implicit memberwise copy or move.

This change is a work-in-progress. I'm still working on cleaning up the existing regression tests and adding new tests and will likely find a few more bugs or edge cases I hadn't considered. And if you accept this approach, I need to update the documentation.

One obvious design hole is: what should we do about declarations between initializers? They can't escape the IIFE's in the cpp1 initializer lists, but they're definitely useful in assignment operators (see the example in the linked issue).

Edit: return statements before and between member initialzers also raise some interesting questions.

jbatez added 2 commits June 17, 2023 20:53
…izer in a constructor, return the initializer from the lambda instead of using the comma operator
@jbatez
Copy link
Contributor Author

jbatez commented Jun 18, 2023

I just realized I don't need to use the comma operator, I can just return the initializer from the lambda. That way, variables declared before a member initializer can be used in that member initializer. E.g.:

t: type = {
    m: i16;
    operator=: (out this, a: i16) = {
        x: i16 = a;
        m = x;
    }
}

generates:

    t::t(cpp2::in<cpp2::i16> a)
        : m{ [&]() -> cpp2::i16 {
              cpp2::i16 x {a};
              return std::move(x);
          }() }
    {
    }

    auto t::operator=(cpp2::in<cpp2::i16> a) -> t& {
        cpp2::i16 x {a}; 
        m = std::move(x);
        return *this;
    }

@JohelEGP
Copy link
Contributor

JohelEGP commented Jun 18, 2023

One obvious design hole is: what should we do about declarations between initializers? They can't escape the IIFE's in the cpp1 initializer lists, but they're definitely useful in assignment operators (see the example in the linked issue).

My understanding it that permitting that just comes naturally to Cpp2.
But what Cppfront can do is limited, given that it has to lower to Cpp1.

What if you delegated to a generated constructor to tap into the power of the stack?
Have this:

t: type = {
  public x: i32;
  operator=: (out this, y: i32) = {
    z := y;
    x = z;
  }
}

Generate this:

class t {
  public: cpp2::i32 x;
  private: struct cpp2_stack_3_2 { };
  public: explicit t(cpp2::in<cpp2::i32> y);
  private: explicit t(cpp2::in<cpp2::i32> y, cpp2_stack_3_2, auto);
  public: auto operator=(cpp2::in<cpp2::i32> y) -> t& ;

  public: t(t const&) = delete; /* No 'that' constructor, suppress copy */
  public: auto operator=(t const&) -> void = delete;
};

  t::t(cpp2::in<cpp2::i32> y)
                                    : t{ y, cpp2_stack_3_2{}, [] {
    struct cpp2_stack_t {
      cpp2::deferred_init<decltype(auto{y})> z;
    };
    return cpp2_stack_t{};
                                    }() }
  {}
  t::t(cpp2::in<cpp2::i32> y, t::cpp2_stack_3_2, auto cpp2_stack)
                                    : x{ (cpp2_stack.z.construct(y), cpp2_stack.z.value()) }
  {}
  auto t::operator=(cpp2::in<cpp2::i32> y) -> t& {
                                    auto z{y};
                                    x = z;
                                    return *this;
  }

See https://cpp2.godbolt.org/z/nf1x4svbh.

@jbatez
Copy link
Contributor Author

jbatez commented Jun 18, 2023

That's a clever trick. It gets complicated pretty quickly, though; you have to track down all the references to z and replace with cpp2_stack.z.value(). E.g.

t: type = {
  public x: i32;
  operator=: (out this, y: i32) = {
    z := y;
    std::cout << z << std::endl;
    w := z;
    x = w;
  }
}

Generate:

class t {
  public: cpp2::i32 x;
  private: struct cpp2_stack_3_2 { };
  public: explicit t(cpp2::in<cpp2::i32> y);
  private: explicit t(cpp2::in<cpp2::i32> y, cpp2_stack_3_2, auto);
  public: auto operator=(cpp2::in<cpp2::i32> y) -> t& ;

  public: t(t const&) = delete; /* No 'that' constructor, suppress copy */
  public: auto operator=(t const&) -> void = delete;
};

  t::t(cpp2::in<cpp2::i32> y)
                                    : t{ y, cpp2_stack_3_2{}, [] {
    struct cpp2_stack_t {
      cpp2::deferred_init<decltype(auto{y})> z;
      cpp2::deferred_init<decltype(auto{z.value()})> w;
    };
    return cpp2_stack_t{};
                                    }() }
  {}
  t::t(cpp2::in<cpp2::i32> y, t::cpp2_stack_3_2, auto cpp2_stack)
                                    : x{ [&]() -> cpp2::i32 {
                                          cpp2_stack.z.construct(y);
                                          std::cout << cpp2_stack.z.value() << std::endl;
                                          cpp2_stack.w.construct(cpp2_stack.z.value());
                                          return cpp2_stack.w.value();
                                      }() }
  {}
  auto t::operator=(cpp2::in<cpp2::i32> y) -> t& {
                                    auto z{y};
                                    x = z;
                                    return *this;
  }

See https://cpp2.godbolt.org/z/89EsnhMxM.

I'll give it a try, but it might be more work than I bargained for at this time.

source/cppfront.cpp Outdated Show resolved Hide resolved
source/cppfront.cpp Outdated Show resolved Hide resolved
@jbatez jbatez changed the title [WIP] Statements before and between member initializers in special member functions Statements before and between member initializers in special member functions Jun 24, 2023
@jbatez
Copy link
Contributor Author

jbatez commented Jun 24, 2023

I think I'm done with this for now. I didn't do the work to handle things declared before a member initializer and used after. The more I thought about it, the more complicated it got. I think it'd be awesome if the feature was there, but I unfortunately don't have the time to implement it myself.

source/cppfront.cpp Outdated Show resolved Hide resolved
@JohelEGP
Copy link
Contributor

Here's another suggestion by example.

This employee's operator=
doesn't start with assignment expression statements to all uninitialized members
and has some other statement that is not an assignment to an initialized member or an out parameter.

employee: type = {
  public w: std::string;
  public v: std::string;
  operator=: (out this, b: bool) = {
    w = foo();
    stuff();
    if b { v = bar(); }
    else { v = baz(); }
  }
}

So lower it as follows.

class employee {
  public: union { std::string w; };
  public: union { std::string v; };
  public: explicit employee(cpp2::in<bool> b);

  public: auto operator=(cpp2::in<bool> b) -> employee& ;

  public: ~employee() noexcept;

  public: employee(employee const&) = delete; /* No 'that' constructor, suppress copy */
  public: auto operator=(employee const&) -> void = delete;
};
  employee::employee(cpp2::in<bool> b)
  {
    auto _w = cpp2::out_member<std::string>{&w};
    auto _v = cpp2::out_member<std::string>{&v};
    _w.construct(foo());
    stuff();
    if (b) {_v.construct(bar()); }
    else {_v.construct(baz()); }
  }
  employee::~employee() noexcept{
    std::destroy_at(&v);
    std::destroy_at(&w);
  }

For details, see #540 (comment).

@jbatez
Copy link
Contributor Author

jbatez commented Nov 9, 2023

The original issue has been addressed by cdf71bd

@jbatez jbatez closed this Nov 9, 2023
@hsutter
Copy link
Owner

hsutter commented Nov 10, 2023

Thanks, Jo!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants