[BUG] Cpp2 types can't model `std::assignable_from<SomeType>`

[JohelEGP]

Describe the bug
Cpp2 doesn't, by default, make generated operator='s return value this and return type an lvalue to the type of this.
Doing this manually results in cppfront generating a Cpp1 constructor that returns *this.

To Reproduce
Steps to reproduce the behavior:

1. Sample code x.cpp2:

myclass : type = {

    operator=: (out this, that) -> _ = {
        return this;
    }
}

2. Command lines

$ ../root/bin/cppfront -p x.cpp2 
$ ../root/clang/bin/clang++ -std=c++20 -I ../root/include/ x.cpp -o x

3. Expected result
   The generated constructor doesn't return *this.
4. Actual result/error

#define CPP2_USE_MODULES         Yes

#include "cpp2util.h"


#line 1 "x.cpp2"
class myclass;

//=== Cpp2 definitions ==========================================================

#line 1 "x.cpp2"
class myclass   {

    public: [[nodiscard]] explicit myclass(myclass const& that) -> auto{
        return (*this); 
    }public: [[nodiscard]] auto operator=(myclass const& that) -> auto{return (*this); }
};

$ ../root/bin/cppfront -p x.cpp2 
x.cpp2... ok (all Cpp2, passes safety checks)

$ ../root/clang/bin/clang++ -std=c++20 -I ../root/include/ x.cpp -o x
error: function with trailing return type must specify return type 'auto', not 'void'
x.cpp2:4:9: error: constructor 'myclass' should not return a value [-Wreturn-type]
        return (*this); 
        ^      ~~~~~~~
2 errors generated.

Additional context
I looked at 482bac8 and 4c52d2d. I found it unfortunate how no generated type of the latter was a model of std::semiregular by default, and generally of std::assignable_from<SomeType> for the former, due to the generated operator='s return type being void.

[AbhinavK00]

Sorry for the incoming rant
Modelling constructor after the assignment operator was a mistake, I don't know why Herb went with that idea for cpp2. The current model looks like we're casting some object into the respective type.
Having named contructors was much better and assignment operator could've always been synthesised from the proper contructor. That would've been one way to unify construction and assignment.
While delegation of contructors is one way, if we're gonna write them anyways, why not just write proper contructors. The problem still falls back to having to write assignment operator and then some delegated contructors. Even if one does not write any delegated contructors, we fall back to the problem of ambiguous contructors observed with vector initialization.
end
I'm sorry for this, but if I'm missing anything that is planned, please enlighten me.

[filipsajdak]

Probably you can return that as well.

[JohelEGP]

I'm sorry for this, but if I'm missing anything that is planned, please enlighten me.

I'm pretty sure Herb touches on this in https://www.youtube.com/embed/ELeZAKCN4tY. It's also most of the upper part of the roadmap.

I have this representation of an integer.

using value_type          = std::uintmax_t;
using value_limits        = std::numeric_limits<value_type>;
using vector_type         = std::vector<value_type>;
const auto max_array_size = sizeof(vector_type) / sizeof(value_type);
using array_type          = std::array<value_type, max_array_size>;

static_assert(max_array_size != 0);

export class integer;

struct integer_rep {
  using value_type = waarudo::value_type;

  bool is_negative_ : 1                                   = false;
  bool is_vector_   : 1                                   = false;
  std::size_t array_size : std::bit_width(max_array_size) = 0;
  union {
    array_type array{};
    vector_type vector;
  };

  [[nodiscard]] constexpr integer_rep() noexcept { }
  [[nodiscard]] constexpr integer_rep(const integer_rep&);
  [[nodiscard]] constexpr integer_rep(integer_rep&&) noexcept;
  constexpr ~integer_rep();
  constexpr integer_rep& operator=(const integer_rep&);
  constexpr integer_rep& operator=(integer_rep&&) noexcept;

  // ...
};

The (copy, move) constructor and assignment pairs each reuse a common function that just depends on the input's rvalueness.

template<class T> constexpr void integer_rep::copy_container(T&& r) noexcept(std::is_rvalue_reference_v<T&&>) {
  if (is_vector()) std::construct_at(&vector, std::move(r.vector));
  else std::construct_at(&array, r.array);
}

constexpr integer_rep::integer_rep(const integer_rep& r)
  : is_negative_{r.is_negative()}, is_vector_{r.is_vector()}, array_size{r.array_size} {
  copy_container(r);
}

constexpr integer_rep::integer_rep(integer_rep&& r) noexcept
  : is_negative_{r.is_negative()}, is_vector_{r.is_vector()}, array_size{r.array_size} {
  copy_container(std::move(r));
}

constexpr integer_rep::~integer_rep() {
  if (const auto s = span(); not s.empty()) expects(s.back() != 0);
  if (is_vector()) std::destroy_at(&vector);
}

template<class T> constexpr integer_rep& integer_rep::assign(T&& r) noexcept(std::is_rvalue_reference_v<T&&>) {
  if (is_vector()) {
    if (r.is_vector()) vector = std::move(r.vector);
    else vector.assign(r.array.begin(), r.array.end());
  } else {
    if (r.is_array()) array = r.array;
    else {
      if (r.vector.capacity() == 0) array = {};
      else {
        std::destroy_at(&array);
        std::construct_at(&vector, std::move(r.vector));
        is_vector_ = true;
      }
    }
    array_size = r.array_size;
  }
  is_negative_ = r.is_negative();
  return *this;
}

constexpr integer_rep& integer_rep::operator=(const integer_rep& r) { return assign(r); }

constexpr integer_rep& integer_rep::operator=(integer_rep&& r) noexcept { return assign(std::move(r)); }

C++'s this parameter can't be used to unify each pair of these special member functions. Although each pair could be a constructor template instead, which has its own tradeoffs. But I'm fairly confident that Cpp2, through the features on that upper part of the roadmap, will allow me to write this as a single function.

[AbhinavK00]

@filipsajdak, Yes what you're showing here is exactly what I was talking about

[filipsajdak]

You probably mean @JohelEGP

[AbhinavK00]

Oh, sorry for the mistake.

[jcanizales]

But I'm fairly confident that Cpp2, through the features on that upper part of the roadmap, will allow me to write this as a single function.

What (to the best of your knowledge) would that end up looking like?

[JohelEGP]

I have had more time to think about it since I posted this. Perhaps this is one of the types that can't take advantage of Cpp2's single operator= to generate C/M C/A. First of all, union is banned in Cpp2. Then, the assignment is too complicated. However, if I made the representation

struct integer_rep {
  bool is_negative_ = false;
  boost::container::short_vector<value_type, max_array_size> vector;

to contain the assignment's complexity in vector, then it might look like

  operator=: (out this, that) {
    is_negative_ = that.is_negative_;
    vector = that.vector;
  }

[hsutter]

Re the main question in this issue:

Cpp2 doesn't, by default, make generated operator='s return value this and return type an lvalue to the type of this.

Good point, this will be fixed in the next commit. Generated Cpp1 assignment operators will return *this; including on early returns. Thanks!

Other questions:

Modelling constructor after the assignment operator was a mistake

Note that's not quite what Cpp2 is doing, and this will be clearer when you see the next commit... I'm writing this up as a design note. Also, note the generation is in the other direction -- in the upcoming next commit, Cpp2 generates the Cpp1 assignment operators from the constructor (generalized operator=(out this, ...)).

First of all, union is banned in Cpp2.

You can still consume Cpp1 unions. You can also use the existing std::variant, which I think is even more convenient with Cpp2's is and as. Cpp2 will support authoring unions... not unsafe unions, but safe (tagged) union is one of the metaclasses I describe in P0707.

In the next commit you will be able to try writing the unified {copy,move} x {constructor,assignment}. You can still provide overloads if you want/need to write more specialized code for them. I finished the basic implementation last night and could push the commit now, but I think it wants to be committed with a doc page, so I'll write a doc page first today/tomorrow.

Thanks!

[AbhinavK00]

Cpp2 will support authoring unions... not unsafe unions, but safe (tagged) union is one of the metaclasses I describe in P0707.

Will those be a preference over std::variant or just a replacement to plain old unions (since std::variant was made for that).
If first case, would std::variant be effectively obsolete?
I was gonna suggest having 3 inbuilt metaclasses(?) in the language to replace 3 standard library classes. Would you consider that if I open a suggestion?

[hsutter]

Metaclasses are an aid to write custom types. So you use a metaclass when you want a custom type of a certain kind. See paper P0707, section 3, for some examples of popular metaclasses. I hope to provide most of them in-the-box, including two different variations on enum... a regular enum and a flag_enum. It will be nice to get first-class support for the latter.

Using enums as an example: My point of view here is not so much there's a whole lot wrong with built-in C enums or C++ enum classes (I'm a coauthor of the latter C++11 feature so I can't say much bad about it :) ), but if we don't have to make them be special-purpose language features hardwired into the compiler, and can write them as (metafunction) library code instead, then (a) it helps keep the language smaller and also (b) it enables people to write new powerful things (like flag-enums) as libraries without having to be a compiler developer.

As for std::variant, I like std::variant and aim to support it really well with is and as. But it has some drawbacks, such as that the members don't have names, which is a big deal for usability... we have to use get<index> or get<type> accessors instead. So P0707 section 3.10 proposes being able to write a safe_union metaclass, that could be used something like this... pasting the paper's extended-Cpp1 syntax example and changing it to maybe-future Cpp2 strawman syntax (syntax still TBD):

U: type(safe_union) = {
    i: int;
    s: string;
    document_map: map<string, vector<document>>;
}

u: U;
u = “xyzzy”; // constructs a string
[[assert: u.is_s()]]
cout << u.s() << endl; // ok
// Note I love today’s std::variant, but I wouldn’t miss writing the anonymous and pointy get<0>.
u = :map<string, vector<document>> =(); // destroys string, moves in map
[[assert: u.is_document_map()]]
use(u.document_map()); // ok
u.clear(); // destroys the map
[[assert: u.is_empty()]]

Note that that "Note" comment is directly from the paper... ;)

Also, note this proposal is older than is and as, so I would look at potentially using those instead of generating is_name() and name() accessors. But even those are a bit better than we can do with std::variant, because now the members can have names.