[BUG] in argument passing to function that uses cpp2 user defined type failed with error

[filipsajdak]

258190e introduces the possibility of defining user-defined types in cpp2. Unfortunately, it fails with a free functions that use UDT as in parameters.

The following code:

X : type = {
    i : int = 42;
}

fun: (x: X) = {
    std::cout << x.i << std::endl;
}

generates:

#define CPP2_USE_MODULES         Yes

#include "cpp2util.h"


#line 1 "../tests/bug_incomplete_type.cpp2"
class X;
#line 5 "../tests/bug_incomplete_type.cpp2"
auto fun(cpp2::in<X> x) -> void;

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

#line 1 "../tests/bug_incomplete_type.cpp2"
class X   {
    private: int i {42}; 
};

auto fun(cpp2::in<X> x) -> void{
    std::cout << x.i << std::endl;
}

It fails on cpp1 compilation with the error:

In file included from ../tests/bug_incomplete_type.cpp:4:
include/cpp2util.h:496:5: error: invalid application of 'sizeof' to an incomplete type 'X'
    sizeof(T) < 2*sizeof(void*) && std::is_trivially_copy_constructible_v<T>;
    ^~~~~~~~~
include/cpp2util.h:501:9: note: in instantiation of variable template specialization 'cpp2::prefer_pass_by_value<X>' requested here
        prefer_pass_by_value<T>,
        ^
../tests/bug_incomplete_type.cpp2:5:16: note: in instantiation of template type alias 'in' requested here
auto fun(cpp2::in<X> x) -> void;
               ^
../tests/bug_incomplete_type.cpp2:1:7: note: forward declaration of 'X'
class X;
      ^
In file included from ../tests/bug_incomplete_type.cpp:4:
include/cpp2util.h:501:9: error: non-type template argument is not a constant expression
        prefer_pass_by_value<T>,
        ^
../tests/bug_incomplete_type.cpp2:5:16: note: in instantiation of template type alias 'in' requested here
auto fun(cpp2::in<X> x) -> void;
               ^
2 errors generated.

The issue is that in argument passing is trying to figure out if auto fun(cpp2::in<X> x) -> void; passes x by reference or by value - it distinguishes that based on the size of X. Unfortunately, at that point compiler does not know the size of X.

[filipsajdak]

@hsutter probably the easiest solution is to move definition of the class before any function declaration.

[filipsajdak]

After thinking more about it - the main issue is with the forward declaration of udt. We cannot use it.

The order should be following:

1. Declarations of udt with only declaration of method and attributes,
2. Declarations of free functions,
3. Implementation of udts methods,
4. Implementation of free functions,

[hsutter]

Quick ack: Yes, that's the idea... last weekend's commit was a preliminary step, first of several, starting with just the most basic cases, UDTs are going to take me a few weeks to really rough in properly, piece by piece. Thanks for watching and for the constructive comments!

[hsutter]

There are two parts here:

(1) Yes, in should pass scalars (fundamental types) by value, but not class types.

This is an interesting learning... the key observation is that only the fundamental built-in types are always complete. This observation really helps clarify for me a long-standing split in today's "in parameters" rule that we apply by hand: Some experts teach the rule as (a) "pass fundamental types by value," and others teach it as (b) "pass small trivially copyable types by value" (which is a superset of (a) that allows some class types). But the observation here is that (b) is in general not possible to follow even for a human, unless the human knows what the complete type is, because even the English description of (b) requires knowledge of the complete type just the same as the prior in<T> implementation of (b) required it. If the human doesn't (or can't) know the complete type, pass by value isn't an option, and might not even be legal never mind efficient. So (b) isn't a problem only for in<T>, it's a problem even for a human when writing code that could work with incomplete types, which includes many templates.

This argues for the formulation (a), rather than (b). That's an interesting observation that I'll think about some more, but for now it appears that (a) is the way to go. The next commit will switch to std::is_scalar instead.

(2) Forward declarations

This is not yet in the weekend's work I'm about to push, but it's one of the next two things on my list (base classes is the other). Since forward declarations in the Cpp1 code gen was mentioned here, I'll leave this issue open until that works too.

Thanks!

[filipsajdak]

@hsutter I confirm that the original issue is solved now - thanks to switching to std::is_scalar_v.

There is still issue with forward returns of in arguments - check here: #248 (comment)

[hsutter]

OK, closing this as fixed. Thanks!

[hsutter]

Also, I've now addressed the #248 comment, thanks for the reminder here.