[BUG] Converting assignment generated from non-converting constructor

[JohelEGP]

Title: Converting assignment generated from non-converting constructor.

Description:

Relevant extracts from https://github.com/hsutter/cppfront/wiki/Cpp2:-operator=,-this-&-that
always talk about generating converting assignment from a converting constructor,
but cppfront currently generate converting assignment from an explicit constructor.

• implicit: On a two-parameter operator= function, writing operator=: (implicit out this, /*...*/) defines a conversion function that can be used to perform implicit conversions to this type.

operator= is the generalized name for all value-setting functions, including the six combinations of { copy, move, converting } x { constructors, assignment operators }.

Note: The concept of a converting assignment operator is first-class in Cpp2, and makes this 3x2 matrix of options symmetric. In Cpp1, it's common to write a converting constructor from some other type, but not an assignment operator from that other type, which leads to asymmetries like mytype var = other; working but var = other; not working.

• (A)ssignment, A1, A2, A3: If you write a copy or move or converting constructor, but not a corresponding copy or move or converting assignment operator, the latter is generated.

Note: Generating inout this (assignment) from out this also generates converting assignment from converting construction, which is a new thing. Today in Cpp1, if you write a converting constructor from another type X, you may or may not write the corresponding assignment from X; in Cpp2 you will get that by default, and it sets the object to the same state as the converting constructor from X does.

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

quantity: type = {
  value: int = ();
  operator=: (out this, val: int) = { value = val; }
}

main: () = {
  x := quantity(0);
  x = 1;
}

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:

An error at:

  x = 1;

Actual result and error:

Cpp2 lowered to Cpp1.

#include "cpp2util.h"

class quantity;
  
class quantity {
  private: int value {}; 
  public: explicit quantity(cpp2::in<int> val);
  public: auto operator=(cpp2::in<int> val) -> quantity& ;

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

auto main() -> int;
  

  quantity::quantity(cpp2::in<int> val)
                                      : value{ val }
  {}
  auto quantity::operator=(cpp2::in<int> val) -> quantity& {
                                      value = val;
                                      return *this;
  }

auto main() -> int{
  auto x {quantity(0)}; 
  x = 1;
}

Output:

Program returned: 0

[realgdman]

As a side note, I find that standalone {} suspicious, like it's failed generation of something operator= related.
I have ran grep '^\s*{\s*}' *.cpp on my cpp2 dir and see it it in several files.

[JohelEGP]

Do you mean in this one?

  quantity::quantity(cpp2::in<int> val)
                                      : value{ val }
  {}

That's the constructor's function body.

[hsutter]

@realgdman

As a side note, I find that standalone {} suspicious, like it's failed generation of something operator= related.
I have ran grep '^\s*{\s*}' *.cpp on my cpp2 dir and see it it in several files.

Besides the function body cases, in my own code I regularly use {} to mean a default value. For example, for a function that returns a pointer, return {}; and return nullptr; are equivalent. For a function that returns a string, return {}; and return ""; are equivalent. For many standard types, it also happens work today for "default assignment" (e.g., my_vector = {};) to reset to essentially the default-constructed state, and so parallel "default construction" (e.g., vector<int> my_vector{};). Maybe those are what you're seeing?

[hsutter]

@JohelEGP - thanks! I think this is intended behavior though... given your example type:

quantity: type = {
  value: int = ();
  operator=: (out this, val: int) = { value = val; }
}

The first example is:

main: () = {
  x: quantity = 0; // explicit construction
  x = 1;           // explicit assignment

This is what I intended... I considered both lines to be equivalently "explicit" syntaxes. My view is that both are explicitly asking x to be set to 0 or 1. Note that with delayed initialization the above can be written equivalently like this:

  y: quantity;
  y = 0;          // explicit construction (first use)
  y = 1;          // explicit assignment
}

and with this version the = equivalence is even more direct.

My understanding of the danger of implicit conversions is when Cpp1 silently creates a temporary quantity object without any clue in the source that it's happening. In this case there are only explicit quantity objects being constructed/assigned.

Does that make sense?

[JohelEGP]

Yes!
It definitely makes much more sense when portrayed with Cpp2,
especially

  y: quantity;
  y = 0;          // explicit construction (first use)
  y = 1;          // explicit assignment

The use of the C++ standard term converting constructor in the wiki
made me think this should only apply to operator= generated from implicit this.

Now, how does one opt-out of this generation?
Types representing quantities don't want assignment from numbers, as that's type unsafe.
For example, the physical meaning of x = 1 changes silently under a refactor of x's type.

[hsutter]

Opt-out is via implicit.

But I think what you're asking is maybe what if the user writes y = Z(); where Z has a conversion to int and then the assignment to y works? But then the implicit conversion is the conversion from Z to int, so it's the responsibility of the author of Z to make that conversion explicit. (Which, if they're writing it in Cpp2, is the default...)

[JohelEGP]

That doesn't work: https://cpp2.godbolt.org/z/zTbj843v7.
I want the explicit assignment x = 1 to be an error.

[hsutter]

Oh, I see -- you want an opt-out of even generating the assignment?

Assuming you want to disable all mutation, not just assignment, one way is to make a const quantity object. Or to make the value member be const.

If you want to allow mutating operations, and allow construction from int but not allow assignment from int, this should work (but currently doesn't, that's just a bug I should fix):

quantity: type = {
  value: int = ();
  operator=: (out this, val: int) = { value = val; }
  private operator=: (inout this, val: int) = { value = val; } // opt out with private assignment (doesn't currently work)
}

There's a bug here because I should not generate the converting assignment when it's user-written, but this would be the way to do it.

But let me ask: Given x: quantity;, can you elaborate on why x = 0; x = 1; should be an error only on the second statement? And x = quantity(1); would still work? That is, I think you're asking for this:

x: quantity;
x = 0; // ok
x = 1; // I think you want this to be an error, even though...
x = quantity(1); // still ok

I'm trying to imagine a situation where that would be desirable... the reason why Cpp2 generates converting assignment from converting construction is that usually they should be symmetric and it's a surprise when Cpp1 class authors support construction from a different type but not assignment from that same type. Is there a use case you came across?

[JohelEGP]

But let me ask: Given x: quantity;, can you elaborate on why x = 0; x = 1; should be an error only on the second statement? And x = quantity(1); would still work? That is, I think you're asking for this:

x: quantity;
x = 0; // ok
x = 1; // I think you want this to be an error, even though...
x = quantity(1); // still ok

I'm trying to imagine a situation where that would be desirable... the reason why Cpp2 generates converting assignment from converting construction is that usually they should be symmetric and it's a surprise when Cpp1 class authors support construction from a different type but not assignment from that same type. Is there a use case you came across?

The use case is unit/quantity types.
x: metres = 1; defines x to be 1 m.
If x = 1; were allowed, that would set x to 1 m.
But if the type of x were to change to
x: millimetres = 1;.
Suddenly, any following x = 1 changes from setting x to 1 m to setting x to 1 mm.
So for added safety, unit libraries require the assignment from a quantity type and not a number:
x = metres(1); // OK, meaning doesn't change under a refactor of the type of `x`.

[hsutter]

Suddenly, any following x = 1 changes from setting x to 1 m to setting x to 1 mm.

Thanks. OK, so I see why you want to suppress assignment, and you'll soon be able to do that by writing a private converting assignment operator (that will work as soon as I fix the bug to not generate it if the user wrote it).

FWIW, this works nicely if you want the UDL-like suffix syntax, which in Cpp2 works for both literals and non-literals (below, an int object named forty_two)... the following already works now:

meters: @value type = {
  value: int = ();
  operator=: (out this, val: int) = { value = val; }
}

main: () = {
  forty_two := 42;

  x := 0.meters();
  x = 1.meters();
  x = forty_two.meters();
}

Note this way all the cases can use the same consistent syntax (though x: quantity = 0; would still work, I'm just saying it's possible to use a single consistent style).

I'll look at fixing the double generation bug so you can opt out and make x = 1; assignment an error... thanks for the feedback and helping me notice the bug.

[JohelEGP]

Thank you.
What about clarifying or removing the uses of "converting"
from the quotes of the wiki article in the opening comment?