[BUG] Certain combination of `inout` member func, `as` and non-last non-moved use doesn't compile UFCS

[realgdman]

Cpp1 compilation fails on exact combination: inout this member function, cast with as, and non-last use without std::move.

Code to Reproduce

S: type = {
	const_foo: (this) = { std::cout << "const foo \n"; }
	mut_foo: (inout this) = { std::cout << "foo \n"; }
}

test_const_foo: () = {
	s: S =();
	(s as S).const_foo(); //ok, CPP2_UFCS_0(const_foo, (cpp2::as_<S>(s)));
	(s as S).const_foo(); //ok, CPP2_UFCS_0(const_foo, (cpp2::as_<S>(std::move(s))));
}

test_mut_foo: () = {
	s: S =();
	(s as S).mut_foo(); //BAD CPP2_UFCS_0(mut_foo, (cpp2::as_<S>(s)));
	(s as S).mut_foo(); //ok, CPP2_UFCS_0(mut_foo, (cpp2::as_<S>(std::move(s))));
}

Command lines
cppfront/cppfront $1.cpp2 -p
clang++-15 -Icppfront/include $1.cpp -std=c++20 -o $1

Expected result - what you expected to happen
Compiled code or diagnostic on wrong usage

Actual result/error

derivedufcs.cpp2:14:14: error: use of undeclared identifier 'mut_foo'
 CPP2_UFCS_0(mut_foo, (cpp2::as_<S>(s)));
             ^
derivedufcs.cpp2:14:2: note: in instantiation of function template specialization 'test_mut_foo()::(anonymous class)::operator()<const S &>' requested here
 CPP2_UFCS_0(mut_foo, (cpp2::as_<S>(s)));
 ^
cppfront/include/cpp2util.h:695:2: note: expanded from macro 'CPP2_UFCS_0'                                            
}(PARAM1)
 ^
1 error generated. 

Additional context
I encountered this writing bigger example, so didn't figured out root cause yet.
But I reduced it to combination of 3 elements. Removing any 1 of them make it work.

1. Using s.foo() instead of (s as S).foo(), but that's what I was testing in first place. But cpp2::as_ template could be first suspect.
2. Removing inout on member function (proved by const_foo compiling without problem)
3. Removing one use. It is working on last use move, but failing on regular use.
   Last one I found important, since last use getting special treatment, and tests usually do something only once, so they usually test last-use move, and not regular one.

Also it's totally possible that I just wrote something silly, and this is expected for this combination.

[JohelEGP]

That's because s as S is returning a reference to const s: https://cpp2.godbolt.org/z/KdYMn39MM.

template< typename C, typename X >
    requires std::is_same_v<C, X>
auto as( X const& x ) -> decltype(auto) {
    return x;
}

// Missing overload:
template< typename C, typename X >
    requires std::is_same_v<C, X>
auto as( X& x ) -> decltype(auto) {
    return x;
}

template< typename C, typename X >
    requires std::is_base_of_v<C, X>
auto as( X&& x ) -> C&& {
    return CPP2_FORWARD(x);
}

template< typename C, typename X >
    requires (std::is_base_of_v<X, C> && !std::is_same_v<C,X>)
auto as( X& x ) -> C& {
    return Dynamic_cast<C&>(x);
}

template< typename C, typename X >
    requires (std::is_base_of_v<X, C> && !std::is_same_v<C,X>)
auto as( X const& x ) -> C const& {
    return Dynamic_cast<C const&>(x);
}

[MaxSagebaum]

There seems to be also other problems with down casting with 'operator as'.

Example: https://cpp2.godbolt.org/z/qz4cqj93d

A: type = {
  public i: int = 0;
  public i2: int = 0;
  public i3: int = 0;
  public i4: int = 0;
  public i5: int = 0;
  public i6: int = 0;
  public i7: int = 0;
  public in8: int = 0;
}

B: type = {
  this: A = ();
  public d: double = 0.0;
  public d2: double = 0.0;
  public d3: double = 0.0;
  public d4: double = 0.0;
}

func_mut: (inout a: A) -> void  = {
  std::cout << "Call A mut: (a.i)$" << std::endl;
}

func_const: (a: A) -> void  = {
  std::cout << "Call A const: (a.i)$" << std::endl;
}

func_mut: (inout b: B) -> void  = {
  std::cout << "Call B mut: (b.d)$" << std::endl;
}

func_const: (b: B) -> void  = {
  std::cout << "Call B const: (b.d)$" << std::endl;
}

main: () -> int = {
  b: B = ();

  func_const(b);  // Call 1
  func_const(b as A);  // Call 2
  func_mut(b);  // Call 3
  func_mut(b as A);  // Call 4

  return 0;
}

Call 1, 2 and 3 give no problems. Call 4 has problems with the move func_mut(cpp2::as_<A>(std::move(b)));:

build/_cppfront/main.cpp:74:3: error: no matching function for call to 'func_mut'
   74 |   func_mut(cpp2::as_<A>(std::move(b)));
      |   ^~~~~~~~

The move basically makes this a const& call. Adding inout on the calling side did not work func_mut(inout b as A); or func_mut((inout b) as A);
Is there a qualifier that removes the move?

If we hack this without the move: https://cpp2.godbolt.org/z/ssa9473vs

[[nodiscard]] auto main() -> int {
  B b {}; 

  func_mut(cpp2::as_<A>(b));
  
  return 0; 
}

we get

cpp2util.h:1692:13: error: static assertion failed due to requirement 'program_violates_type_safety_guarantee<A, B>': No safe 'as' cast available - please check your cast 

So it has now problems with the as cast.
The version which should match is:

template< typename C, typename X >
    requires std::is_base_of_v<C, X>
auto as( X&& x ) -> C&& {
    return CPP2_FORWARD(x);
}

X is matched as B& and C as A so std::is_base_of_v<A, B&> fails. If we remove the reference from X it starts matching.

template< typename C, typename X >
    requires std::is_base_of_v<C, std::remove_cvref_t<X>>
auto as( X&& x ) -> C&& {
    return CPP2_FORWARD(x);
}

But then it tries to return a non reference value. A version that works is:

template< typename C, typename X >
    requires std::is_base_of_v<C, std::remove_cvref_t<X>>
auto as( X&& x ) -> C& {
    return std::forward<X>(x);
}

We would need to define the return type C based on the type X. Is there a function in the standard that does this?

[JohelEGP]

Is there a qualifier that removes the move?

See #466 (comment).

If we hack this without the move: https://cpp2.godbolt.org/z/ssa9473vs

See #396 (comment) for more hacks.

We would need to define the return type C based on the type X. Is there a function in the standard that does this?

There's C++23's std::forward_like.

Call 1, 2 and 3 give no problems. Call 4 has problems with the move func_mut(cpp2::as_<A>(std::move(b)));:

According to https://cpp2.godbolt.org/z/8rPdrhnWs,
call 2 also fails:

build/_cppfront/main.cpp:72:3: error: no matching function for call to 'func_const'
   72 |   func_const(cpp2::as_<A>(b));// Call 2
      |   ^~~~~~~~~~
build/_cppfront/main.cpp:56:6: note: candidate function not viable: no known conversion from 'nonesuch_' to 'const A' for 1st argument
   56 | auto func_const(cpp2::in<A> a) -> void{
      |      ^
build/_cppfront/main.cpp:64:6: note: candidate function not viable: no known conversion from 'nonesuch_' to 'const B' for 1st argument
   64 | auto func_const(cpp2::in<B> b) -> void{
      |      ^

[MaxSagebaum]

I took a look at std::forward_like but it does not work in our case. Having an implicit cast in there breaks it for our case. It is simple to provide a & and const& version. I extended my test case and I think that I have now a working version #501

It might be possible to remove the versions with is_same and remove the is_same from the is_base methods.

This compiles now with the version in #501.

template <typename T>
using AddRef = T&;

template <typename T>
using AddConstRef = T const&;

A: type = {
  public i: int = 0;

	const_foo: (virtual this) = { std::cout << "const foo \n"; }
	mut_foo: (inout this) = { std::cout << "foo \n"; }

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

B: type = {
  this: A = ();
  public d: double = 0.0;

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

func_mut: (inout a: A) -> void  = { std::cout << "Call A mut: (a.i)$" << std::endl; }
func_mut: (inout b: B) -> void  = { std::cout << "Call B mut: (b.d)$" << std::endl; }
func_const: (a: A) -> void  = { std::cout << "Call A const: (a.i)$" << std::endl; }
func_const: (b: B) -> void  = { std::cout << "Call B const: (b.d)$" << std::endl; }

test_const_foo: () = {
	s: A =();
  sC: AddConstRef<A> = s;
  s.const_foo();
  sC.const_foo();
	(s as A).const_foo();
  (sC as A).const_foo();
  s = s;
}

test_mut_foo: () = {
	s: A =();
  s.mut_foo();
	(s as A).mut_foo();
  s = s;
}

test_down: () = {
  b: B = ();
  bC: AddConstRef<B> = b;

  func_const(b);
  func_const(b as B);
  func_const(b as A);
  func_const(bC);
  func_const(bC as B);
  func_const(bC as A);
  func_mut(b);
  func_mut(b as B);
  func_mut(b as A);

  b = b;
  //bC = bC;
}

test_up: () = {
  b: B = ();
  bC: AddConstRef<B> = b;
  a: AddRef<A> = b as A;
  aC: AddConstRef<A> = b as A;

  func_const(a);
  func_const(a as B);
  func_const(a as A);
  func_const(aC);
  func_const(aC as B);
  func_const(aC as A);
  func_mut(a);
  func_mut(a as B);
  func_mut(a as A);

  a = a;
}

main: () -> int = {

  test_const_foo();
  test_mut_foo();
  test_down();
  test_up();

  return 0;
}

[MaxSagebaum]

With the newest update, I took another look at the example I provided. I removed the References. I also shortened it a little bit.

A: type = {
  public i: int = 0;

	const_foo: (virtual this) = { std::cout << "const foo \n"; }
	mut_foo: (inout this) = { std::cout << "foo \n"; }
}

B: type = {
  this: A = ();
  public d: double = 0.0;
}

func_mut: (inout a: A) -> void  = { std::cout << "Call A mut: (a.i)$" << std::endl; }
func_mut: (inout b: B) -> void  = { std::cout << "Call B mut: (b.d)$" << std::endl; }
func_const: (a: A) -> void  = { std::cout << "Call A const: (a.i)$" << std::endl; }
func_const: (b: B) -> void  = { std::cout << "Call B const: (b.d)$" << std::endl; }

test_const_foo: () = {
	s: A =();
  sC: *const A = s&;
  s.const_foo();
  sC*.const_foo();
	(s as A).const_foo();
  (sC* as A).const_foo();
  _ = s;
  _ = sC;
}

test_mut_foo: () = {
	s: A =();
  s.mut_foo();
	(s as A).mut_foo();
  _ = s;
}

test_down: () = {
  b: B = ();
  bC: *const B = b&;

  func_const(b);
  func_const(b as B);
  func_const(b as A);
  func_const(bC*);
  func_const(bC* as B);
  func_const(bC* as A);

  func_mut(b);
  func_mut(b as B);
  func_mut(b as A);

  _ = b;
  _ = bC;
}

test_up: () = {
  b: B = ();
  bC: *const B = b&;
  a: *A = (b as A)&;
  aC: *const  A = (b as A)&;

  func_const(a*);
  func_const(a* as B);
  func_const(a* as A);
  func_const(aC*);
  func_const(aC* as B);
  func_const(aC* as A);
  func_mut(a*);
  func_mut(a* as B);
  func_mut(a* as A);

  _ = b;
  _ = bC;
  _ = a;
  _ = aC;
}

main: () -> int = {

  test_const_foo();
  test_mut_foo();
  test_down();
  test_up();

  return 0;
}

Should I put this in a regression test?

[JohelEGP]

Should I put this in a regression test?

Definitely!

[MaxSagebaum]

What is the best way to generate the result files for the regression tests? I added the file in regression-tests/pure2-types-down-upcast.cpp2. Your cmake project pics this up and runs the generation. I added then the result files under regression-tests/test-results. Now the test system tries to compare it and build it but I get a failure without any specifics:

[msagebaum]regression-tests> ctest --output-on-failure -R pure2-types-down-upcast
Test project cppfrontCMake/build/regression-tests
    Start 215: codegen/pure2-types-down-upcast
1/3 Test #215: codegen/pure2-types-down-upcast .........   Passed    0.08 sec
    Start 216: codegen/check/pure2-types-down-upcast
2/3 Test #216: codegen/check/pure2-types-down-upcast ...***Failed    0.01 sec

    Start 217: build/pure2-types-down-upcast
3/3 Test #217: build/pure2-types-down-upcast ...........   Passed    2.21 sec

67% tests passed, 1 tests failed out of 3

Total Test time (real) =   2.32 sec

The following tests FAILED:
	216 - codegen/check/pure2-types-down-upcast (Failed)

My next question would be how to generate the outputs for the compilers and how to force the comparison with gcc-10 (I have gcc-12 installed.)

[JohelEGP]

Now the test system tries to compare it and build it but I get a failure without any specifics:

That means the code generated by codegen/pure2-types-down-upcast
differs from the one under regression-tests/test-results.
Which is weird, considering it should be the same.

I added then the result files under regression-tests/test-results.

See https://github.com/modern-cmake/cppfront#developers.
Build with --target cppfront_update_test_results so you don't have to update it manually.

My next question would be how to generate the outputs for the compilers and how to force the comparison with gcc-10 (I have gcc-12 installed.)

You would need to use modern-cmake/cppfront#88.
This one exchanges building with --target cppfront_update_test_results
for configuring with -DCPPFRONT_DEVELOPING=TRUE.

[MaxSagebaum]

Thanks, I actually forgot to look at the cmake repository description. I updated the merge request with the regression test.