[SUGGESTION] Improve in parameter passing (restore old behavior, exclude problem cases only)

[JohelEGP]

Restore in optimization for class types

At first, in parameters were optimized for class types.
Cppfront's optimization of in parameters was faithful to
F.16: For “in” parameters, pass cheaply-copied types by value and others by reference to const.

Then, #270 brought to light that the optimization didn't play well with Cpp2's order independence.
After #317, the net effect is that Cppfront never optimizes in parameters of class type.

I still lament the fact that we lost this optimization to Cpp2 itself.
Similar to #317, here I propose to "improve in parameter passing (restore old behavior, exclude problem cases only)".

#270 (comment) describes the general problem.
However, it generally doesn't apply to Cpp2.
Cpp2 generally requires parameters of complete type.
That's because most functions are initialized and will have a Cpp1 definition emitted.
So we can generally try to optimize in parameters of class types to pass-by-value.
I'd love to have this optimization restored for the general case.

The conditions when applying cpp2::in is wrong are two only (AFAIK).

• The case of [BUG] in argument passing to function that uses cpp2 user defined type failed with error #270.
  In the current source order,
  it only applies to parameters of type that come later in source order.
• Virtual functions.
  An uninitialized virtual this function can have parameters of incomplete type.
  When declaring an override this or final this function in a different TU,
  we can't know if its in parameters of class type had opted-into the optimization with the current Cppfront.

I propose that we identify these conditions,
and only when at least one of these conditions is true,
do we continue emit a version of cpp2::in that doesn't optimize class types.
Otherwise, we should emit a version of cpp2::in that does try to optimize class types.

Identified problems:

• In templates, the optimization is observable.
  This is evident with function types (e.g., see feat: support function types #526 (comment)).
  With this suggestion, the effect would, once again, extend to optimized class types.
• Similar to fix(to_cpp1): improve recognition of dependent types and deducible parameters #533, we might need to do lexical analysis
  to determine that a parameter's type indeed names a type that shouldn't be optimized.

References:

• [BUG] in argument passing to function that uses cpp2 user defined type failed with error #270.
• [BUG] in argument passing to function that uses cpp2 user defined type failed with error #270 (comment).
• [SUGGESTION] Add support for defining type aliases. #273 (comment).
• [BUG] Improve cpp2::in (restore old behavior, exclude problem cases only) #317.
• feat: support function types #526 (comment).
• fix(to_cpp1): improve recognition of dependent types and deducible parameters #533.
• [SUGGESTION] Independent order and type data members #641 (comment).
• https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-in.

Related links:

• Parameter pass-by-value heuristic: trivial copyability issue, and incomplete type issue isocpp/CppCoreGuidelines#2112.

Will your feature suggestion eliminate X% of security vulnerabilities of a given kind in current C++ code?
No.

Will your feature suggestion automate or eliminate X% of current C++ guidance literature?
Yes.
F.16: For “in” parameters, pass cheaply-copied types by value and others by reference to const.

Describe alternatives you've considered.
None.

[JohelEGP]

Identified problems:

• In templates, the optimization is observable.
  This is evident with function types (e.g., see feat: support function types #526 (comment)).
  With this suggestion, the effect would, once again, extend to optimized class types.

Here's an example with the main branch: https://cpp2.godbolt.org/z/b8s5vqsEG.

• Similar to fix(to_cpp1): improve recognition of dependent types and deducible parameters #533, we might need to do lexical analysis
  to determine that a parameter's type indeed names a type that shouldn't be optimized.

Here's an example that should continue using a version of cpp2::in that doesn't optimize class types:
https://cpp2.godbolt.org/z/sE6b5z8Kx.

my_type_alias: type == my_type;
f: (_: my_type_alias) = { }
my_type: @struct type = { }
main: () = { }

my_type_alias doesn't meet the condition to never optimize, but the type it names does.

[JohelEGP]

The conditions when applying cpp2::in is wrong are two only (AFAIK).

• The case of [BUG] in argument passing to function that uses cpp2 user defined type failed with error #270.
  In the current source order,
  it only applies to parameters of type that come later in source order.

The pedantic condition is
parameters of type that come later in source order
emitted in Phase 2 "Cpp2 type definitions and function declarations".
I.e., when a parameter is part of the declaration of all those things emitted in Phase 2.
This would include cases like:

cppfront/regression-tests/pure2-bugfix-for-non-local-function-expression.cpp2

Lines 5 to 7 in a92fdc8

	v: <T> concept = :() -> bool = true;();
	u: type == std::type_identity_t<decltype(:() = {})>;

[JohelEGP]

Here's my proposed version of a cpp2::in that optimized class types: https://cpp2.godbolt.org/z/W9cP8a64h.

template<class T, bool V = (requires { sizeof(T); })> constexpr bool is_complete{V};

template<typename T>
constexpr bool prefer_pass_by_value_optimized =
    sizeof(T) <= 2*sizeof(void*)
    && std::is_trivially_copy_constructible_v<T>;

template<typename T>
    requires std::is_array_v<T> || std::is_function_v<T>
constexpr bool prefer_pass_by_value_optimized<T> = false;

template<typename T>
    requires is_complete<T>
using in_optimized =
    std::conditional_t <
        prefer_pass_by_value_optimized<T>,
        T const,
        T const&
    >;

Compared to main's cpp2::in:

• It removes std::is_class_v<T> || std::is_union_v<T> to enable optimizing those.
• It replaces (!std::is_void_v<T>) with is_complete<T>
  for the same reasons as the former ([BUG] Improve cpp2::in (restore old behavior, exclude problem cases only) #317 (comment)).
  This gives a clear error message about is_complete<C> being false
  rather than about using sizeof on an incomplete type C.

[JohelEGP]

• Similar to fix(to_cpp1): improve recognition of dependent types and deducible parameters #533, we might need to do lexical analysis
  to determine that a parameter's type indeed names a type that shouldn't be optimized.

Here's an example that should continue using a version of cpp2::in that doesn't optimize class types:
https://cpp2.godbolt.org/z/sE6b5z8Kx.

my_type_alias: type == my_type;
f: (_: my_type_alias) = { }
my_type: @struct type = { }
main: () = { }

my_type_alias doesn't meet the condition to never optimize, but the type it names does.

This can still result in a missed optimization due to a false positive.

// header.h2
ns: namespace = {
  my_type: @struct type = { }
}

// main.cpp2
#include "header.h2"
ns: namespace = {
  my_type_alias: type == my_type;
  f: (_: my_type_alias) = { }
}
my_type: @struct type = { }
main: () = { }

The truth is that my_type_alias names the complete type ns::my_type.
But lexical lookup suggests it names ::my_type.
The result is a non-optimized parameter that could have been optimized.

[JohelEGP]

Afterwards, that parameters outside special member functions should be optimized.
In the distant future, this parameters should be optimized (with an explicit object parameter).

[JohelEGP]

#270 (comment) describes the general problem.
However, it generally doesn't apply to Cpp2.
Cpp2 generally requires parameters of complete type.
That's because most functions are initialized and will have a Cpp1 definition emitted.
So we can generally try to optimize in parameters of class types to pass-by-value.
I'd love to have this optimization restored for the general case.

This might not be true anymore given that
.h2 headers permit splitting the interface and implementation to different TUs
(see #705 (comment)).

Working around such an issue might be another use case for inout _: const _ parameters (#696).
Opting out of the in optimization is certainly a use case for inout _: const _.
Certain things in https://wg21.link/temp are specified with lexical analysis (AFAIU).
If the same kind of analysis to not optimize in parameters falls short, reach out for inout _: const _.

[JohelEGP]

AFAIK, optimizing shouldn't be a problem in modules.
The strong ownership model means you can't move the definitions anywhere outside that module.

[JohelEGP]

#270 (comment) describes the general problem.
However, it generally doesn't apply to Cpp2.
Cpp2 generally requires parameters of complete type.
That's because most functions are initialized and will have a Cpp1 definition emitted.
So we can generally try to optimize in parameters of class types to pass-by-value.
I'd love to have this optimization restored for the general case.

This might not be true anymore given that
.h2 headers permit splitting the interface and implementation to different TUs
(see #705 (comment)).

Let me clarify.

Compiling an .h2 header with -pure-cpp2 lowers it to two Cpp1 headers.
An .h header with the interface and an .hpp header with the implementation (see #594 (comment)).
This merely adds another condition when optimizing in parameters is wrong.

Why `in` parameters can generally be optimized in Cpp2 (from the opening comment):

#270 (comment) describes the general problem.
However, it generally doesn't apply to Cpp2.
Cpp2 generally requires parameters of complete type.
That's because most functions are initialized and will have a Cpp1 definition emitted.
So we can generally try to optimize in parameters of class types to pass-by-value.
I'd love to have this optimization restored for the general case.

The conditions when applying cpp2::in is wrong are two only (AFAIK).

• The case of [BUG] in argument passing to function that uses cpp2 user defined type failed with error #270.
  In the current [source order][],
  it only applies to parameters of type that come later in [source order][].
• Virtual functions.
  An [uninitialized virtual this function][] can have parameters of incomplete type.
  When declaring an override this or final this function in a different TU,
  we can't know if its in parameters of class type had opted-into the optimization with the current Cppfront.

This is because users can consume the interface without completing its types.
Only the TU that provides the implementation needs to complete them all.

However, we can still optimize in parameters when this new condition doesn't hold:

• When compiling an .h2 header without -pure-cpp2 (everything lowers to the .h header).
• When compiling a module, as mentioned in the comment right above ([SUGGESTION] Improve in parameter passing (restore old behavior, exclude problem cases only) #666 (comment)).

[JohelEGP]

Here's an example that should continue using a version of cpp2::in that doesn't optimize class types:
https://cpp2.godbolt.org/z/sE6b5z8Kx.

my_type_alias: type == my_type;
f: (_: my_type_alias) = { }
my_type: @struct type = { }
main: () = { }

my_type_alias doesn't meet the condition to never optimize, but the type it names does.

You can replace my_type_alias: type == my_type; with Cpp1 struct my_type_alias;
and my_type: @struct type = { } with Cpp1 struct my_type_alias { }; to the same effect (https://cpp2.godbolt.org/z/nn9vvz3z9):

struct my_type_alias;
f: (_: my_type_alias) = { }
struct my_type_alias { };
main: () = { }

So perhaps only pure Cpp2 .cpp2 sources can always benefit from optimized in parameters.

[JohelEGP]

There's been many changes since this issue was opened.
Particularly, now we have in_ref and the ability to write more code that is not order independent.
I think it'd be the simplest to just diagnose incomplete types, suggesting to use in_ref instead.
Here's my proposed solution, superseding #666 (comment) (but its points still stand),
as a diff with the main branch:

-template<typename T>
-constexpr bool prefer_pass_by_value =
-    sizeof(T) <= 2*sizeof(void*)
-    && std::is_trivially_copy_constructible_v<T>;
-
-template<typename T>
-    requires std::is_class_v<T> || std::is_union_v<T> || std::is_array_v<T> || std::is_function_v<T>
-constexpr bool prefer_pass_by_value<T> = false;
+template<class T, bool V = (requires { sizeof(T); })> constexpr bool is_complete{V};
+
+template<typename T>
+constexpr bool prefer_pass_by_value() {
+  if constexpr (std::is_array_v<T> || std::is_function_v<T>) {
+    return false;
+  } else {
+    static_assert(is_complete<T>, "an 'in' parameter must be a complete type - use 'in_ref' instead");
+    return sizeof(T) <= 2*sizeof(void*)
+           && std::is_trivially_copy_constructible_v<T>;
+  }
+}

 template<typename T>
     requires (!std::is_void_v<T>)
 using in =
     std::conditional_t <
-        prefer_pass_by_value<T>,
+        prefer_pass_by_value<T>(),
         T const,
         T const&
     >;

[gregmarr]

@JohelEGP How does this not result in ODR violations because whether or not a type is complete is not a property of the type but whether or not you've seen the full definition or just a forward declaration?

[JohelEGP]

If ODR is an issue despite the static_assert,
we can move the assertion to the requires-clause of cpp2::impl::in,
as did the previous attempt (#666 (comment)).

+template<class T, bool V = (requires { sizeof(T); })> constexpr bool is_complete{V};
+
+template<typename T, bool IsComplete = is_complete<T>>
+constexpr bool assert_valid_in_type() {
+    static_assert(!std::is_void_v<T>, "a parameter type can't be 'void'");
+    static_assert(!std::is_void_v<T> && IsComplete, "an 'in' parameter must be a complete type - use 'in_ref' instead");
+    return !std::is_void_v<T> && IsComplete;
+}
+
 template<typename T>
 constexpr bool prefer_pass_by_value =
     sizeof(T) <= 2*sizeof(void*)
     && std::is_trivially_copy_constructible_v<T>;

 template<typename T>
-    requires std::is_class_v<T> || std::is_union_v<T> || std::is_array_v<T> || std::is_function_v<T>
+    requires std::is_array_v<T> || std::is_function_v<T>
 constexpr bool prefer_pass_by_value<T> = false;

 template<typename T>
-    requires (!std::is_void_v<T>)
+    requires assert_valid_in_type<T>()
 using in =
     std::conditional_t <
         prefer_pass_by_value<T>,
         T const,
         T const&
     >;

[gregmarr]

Okay, so with this change:

class ForwardDeclared;

f: (obj: ForwardDeclared) = {} // won't work, because it's incomplete
g: (obj: in_ref ForwardDeclared) = {} // OK

Because f will fail to compile when it's incomplete, there is no possibility of an ODR violation.
Makes sense.

[JohelEGP]

ODR relates to the consistency of definitions across TUs.
Maybe you meant the equivalent for template instantiations within a TU.

[gregmarr]

This is the case I'm referring to with the is_complete in the prefer_pass_by_value function.

TU 1:

class Foo
{
};
f: (obj: Foo) = {}

TU 2:

class Foo;
f: (obj: Foo) = {}

That gives using in<Foo> = Foo const in TU1 and using in<Foo> = Foo const & in TU2.

I was thinking it made an actual in<Foo> class definition, not an alias. I'm not sure if different using aliases in different TUs is an ODR violation, but it would at least give a redeclaration error here as it would have both usings from above:

class Foo;
f: (obj: Foo) = {}
class Foo
{
};
g: (obj: Foo) = {}

That won't happen with the later change, the TU 2 would just be an error.

[JohelEGP]

My latest suggestion makes the f in TU 2 an error.
It would suggest using in_ref instead.
If you did that, I think you'd simply get two different f functions.
Legal, I think, even if you intended them to be the same.

[gregmarr]

Yes, that's exactly what I'm saying.