Repository navigation
[BUG] in argument passing to function that uses cpp2 user defined type failed with error #270
Description
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.
Activity
filipsajdak commentedon Mar 10, 2023
@hsutter probably the easiest solution is to move definition of the class before any function declaration.
filipsajdak commentedon Mar 10, 2023
After thinking more about it - the main issue is with the forward declaration of udt. We cannot use it.
The order should be following:
hsutter commentedon Mar 11, 2023
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 commentedon Mar 20, 2023
There are two parts here:
(1) Yes,
inshould 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 forin<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_scalarinstead.(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!
Add `operator=` generation
filipsajdak commentedon Mar 25, 2023
@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
inarguments - check here: #248 (comment)hsutter commentedon Apr 1, 2023
OK, closing this as fixed. Thanks!
hsutter commentedon Apr 1, 2023
Also, I've now addressed the #248 comment, thanks for the reminder here.
Add `operator=` generation