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] The meaning of operator= with multiple non-this parameters #451

Open
JohelEGP opened this issue May 11, 2023 · 4 comments
Open

[BUG] The meaning of operator= with multiple non-this parameters #451

JohelEGP opened this issue May 11, 2023 · 4 comments
Labels
bug Something isn't working

Comments

@JohelEGP
Copy link
Contributor

JohelEGP commented May 11, 2023

Title: The meaning of operator= with multiple non-this parameters.

Description:

An operator= with multiple non-this parameters
doesn't work with non definite first assignment.

Looking at #449, it seems like we expect it to work.
I think that is consistent with https://github.com/hsutter/cppfront/wiki/Cpp2:-operator=,-this-&-that#misc-notes's

More generally, writing = always invokes an operator= (in fact for a Cpp2-authored type, and semantically for a Cpp1-authored type). This avoids the Cpp1 inconsistency that "writing = calls operator=, except when it doesn't" (such as in a Cpp1 variable initialization). Conversely, operator= is always invoked by =.

Besides #449, further integration is also the objective of #368, and perhaps also of #409.
#321 is another such issue, but = (...) is intended for an std::initializer_list parameter.

Minimal reproducer (https://cpp2.godbolt.org/z/q1WvTaEnM):

t: type = {
  // operator=: (out this, x, y) = { }
  operator=: (inout this, x, y) = { }
}
main: () = { }
Commands.
cppfront -clean-cpp1 main.cpp2
clang++17 -std=c++2b -stdlib=libc++ -lc++abi -pedantic-errors -Wall -Wextra -Wconversion -I . main.cpp

Expected result:

As you can see below, cppfront currently lowers to
a Cpp1 assignment operator with multiple arguments, which is ill-formed.

In order for this to work, it'd have to be lowered to a Cpp1 cpp2_assign overload
(inspired from the C++ standard library containers assign member functions):

  public: auto cpp2_assign(auto const& x, auto const& y) -> t& ;

Here's where it gets contentious with #321.
Cpp2 a = (x, y) would have to choose between
Cpp1 a = {x, y} and
Cpp1 a.cpp2_assign(x, y).

Actual result and error:

  public: auto operator=(auto const& x, auto const& y) -> t& ;
build/_cppfront/main.cpp:9:16: error: overloaded 'operator=' must be a binary operator (has 3 parameters)
  public: auto operator=(auto const& x, auto const& y) -> t& ;
               ^
Cpp2 lowered to Cpp1.
#include "cpp2util.h"

class t;
  
class t {
  // operator=: (out this, x, y) = { }
  public: auto operator=(auto const& x, auto const& y) -> t& ;

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

  auto t::operator=(auto const& x, auto const& y) -> t& {
                                    return *this;
  }

auto main() -> int{}
@JohelEGP
Copy link
Contributor Author

In order for this to work, it'd have to be lowered to a Cpp1 cpp2_assign overload
(inspired from the C++ standard library containers assign member functions):

I considered just using assign, which means

  • Cpp2's = works for the container's assign,
  • but Cpp2 claims assign as a member.

Here's where it gets contentious with #321.
Cpp2 a = (x, y) would have to choose between
Cpp1 a = {x, y} and
Cpp1 a.cpp2_assign(x, y).

I thought about proposing to emit an assignment operator with a parameter such that a = {x, y} always works.
The use of braces would be consistent with emitted initialization,
as = would reject narrowing conversions whether it's a definite first assignment or not.
(See https://cpp2.godbolt.org/z/sarMKvnGo generate braces for initialization
and https://cpp2.godbolt.org/z/EcrnEh75v error due to narrowing braces in assignment).

However, if the user were to write both

  operator=: (out this, x: i8, y: i8) = { } // Two non-`this` parameters.
  operator=: (inout this, x: i8, y: i8) = { } // Two non-`this` parameters.

then non definite first assignment becomes ambiguous (as seen at https://cpp2.godbolt.org/z/GWsKPMzYf).

This also highlights two facts.

First, the current operator=: (out this, x: i8) = { } (single non-this parameter) narrows on non definite first assignment
(as opposed to using narrowing-preventing braces, see https://cpp2.godbolt.org/z/e8ojnYPc4).

Next, consider if #321 were fixed.
a = {x, y} would happen to work, but actually construct a t and then move-assign to a
(see https://cpp2.godbolt.org/z/4s64rnsEd, after changing the type to avoid narrowing braces).
The fact is that Cpp2 has the opportunity for its = to be both correct and efficient.
Rather than relying on a = {x, y} happening to work
(when the user writes a operator=: (out this, x: i8, y: i8) = { } [two non-this parameters]),
it could explicitly emit an operator=/cpp2_assign
like it does for operator=: (out this, x: i8) = { } (single non-this parameter).
However, the proposal to

emit an assignment operator with a parameter such that a = {x, y} always works

means it'd be ambiguous with the constructor generated from operator= with out this.
So it seems like a binary choice between

  • efficient a.cpp2_assign(x, y) and
  • narrowing-preventing braces a = {x, y}.

@JohelEGP
Copy link
Contributor Author

JohelEGP commented May 11, 2023

Here's where it gets contentious with #321.
Cpp2 a = (x, y) would have to choose between
Cpp1 a = {x, y} and
Cpp1 a.cpp2_assign(x, y).

I thought about proposing to emit an assignment operator with a parameter such that a = {x, y} always works.

So it seems like a binary choice between

  • efficient a.cpp2_assign(x, y) and
  • narrowing-preventing braces a = {x, y}.

The alternative is to combine these two monsters
to emit a.cpp2_assign({x, y}) (see https://cpp2.godbolt.org/z/e9313Ebr9).
But for a = (x, y) to also work for #321,
something else is necessary, like the UFCS macro overload resolution.
If that's acceptable, I'd learn towards preferring the more performant cpp2_assign.

@JohelEGP
Copy link
Contributor Author

The alternative is to combine these two monsters
to emit a.cpp2_assign({x, y}) (see https://cpp2.godbolt.org/z/e9313Ebr9).

Unfortunately, braces prevent deduction
(see #312 and https://cpp2.godbolt.org/z/41fqv6YqP).

So perhaps we should just lose the braces
(see https://cpp2.godbolt.org/z/fKc9Kve1r and the output warnings).

@JohelEGP
Copy link
Contributor Author

JohelEGP commented May 12, 2023

Here's an example of current vs. proposed (https://cpp2.godbolt.org/z/5vosWPW5n):

Program returned: 0
Capacity before proposed assignment: 100
Capacity  after proposed assignment: 100
Capacity before  current assignment: 100
Capacity  after  current assignment: 13

In order for this to work, it'd have to be lowered to a Cpp1 cpp2_assign overload
(inspired from the C++ standard library containers assign member functions):

I considered just using assign, which means

  • Cpp2's = works for the container's assign,
  • but Cpp2 claims assign as a member.

Unfortunately, for ranges, the containers use assign_range.

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
Development

No branches or pull requests

1 participant