[BUG] in parameter optimization should be less observable

[JohelEGP]

Title: in parameter optimization should be less observable.

Description:

Some in parameters are optimized to pass-by-value.
This can result in code that inadvertently depends on the optimization or lack thereof.

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

#include <array>
f: (x: std::array<int, 42>) -> _ = x&;
f: (x: int) -> _ = x&;
main: () = {
  (arr := std::array<int, 42>())
    [[assert: f(arr) == arr&]]
  (i := 42)
    [[assert: f(i) != i&]]
}

Commands:

cppfront main.cpp2
clang++17 -std=c++23 -stdlib=libc++ -lc++abi -pedantic-errors -Wall -Wextra -Wconversion -Werror=unused-result -I . main.cpp

Expected result:

More consistent results between the operations done on in parameters regardless of optimization.
This overload f has different semantics for optimized and non-optimized in parameters.
The overload could be called address,
in which case it's very obviously wrong for int,
but very much passes for std::array<int, 42> in today's Cppfront,
even though the meaning could change for a lower number of elements.

Actual result and error:

Cpp2 lowered to Cpp1:

//=== Cpp2 type declarations ====================================================


#include "cpp2util.h"



//=== Cpp2 type definitions and function declarations ===========================

#include <array>
[[nodiscard]] auto f(cpp2::in<std::array<int,42>> x) -> auto;
[[nodiscard]] auto f(cpp2::in<int> x) -> auto;
auto main() -> int;


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


[[nodiscard]] auto f(cpp2::in<std::array<int,42>> x) -> auto { return &x;  }
[[nodiscard]] auto f(cpp2::in<int> x) -> auto { return &x;  }
auto main() -> int{
{
auto const& arr = std::array<int,42>();

    cpp2::Default.expects(f(arr) == &arr, "");
}
{
auto const& i = 42;

    cpp2::Default.expects(f(i) != &i, "");
}
}

Output:

main.cpp2:3:57: warning: address of stack memory associated with parameter 'x' returned [-Wreturn-stack-address]
    3 | [[nodiscard]] auto f(cpp2::in<int> x) -> auto { return &x;  }
      |                                                         ^
1 warning generated.

See also:

• feat: support function types #526.

[AbhinavK00]

Both in this issue, and #250 , problems arise due to functions returning a reference. What if the semantics were that optimization is only applied when result is returned by value and otherwise, a const& is passed always?
I think this would solve the problem but tracking if the returned reference depends on a parameter or not could be the challenge.

[JohelEGP]

That's just an easy way to manifest the difference in optimization.
You could depend on the in parameter's address some other way,
like pushing it back onto a vector.

[AbhinavK00]

Correct me if I'm wrong, but in that case, a copy vs reference won't make a difference, right?
But you're right, any case where a dependence on parameter's address is required, const& is mandatory.
Maybe Herb thought through this and we're overthinking?

[JohelEGP]

The difference is the validity of the address.
It's always invalid on return for optimized parameters.
Otherwise, it's the validity of the argument used to initialize the parameter.

[JohelEGP]

Correct me if I'm wrong, but in that case, a copy vs reference won't make a difference, right?

It doesn't make a difference if you store the parameter.
But I'm specifically referring to the parameter's address.

Sorry, but I don't understand the example of pushing onto a vector. Can you please explain.
-- #696 (comment).

Something like this (https://cpp2.godbolt.org/z/9Yhfc351r):

#include <vector>
game_map: @struct type = {
  entities: std::vector<* const void>;
  add_entity: (inout this, x) = entities.push_back(x&);
  add_entity: (inout this, x: long) = entities.push_back(x&);
}
main: () = {
  i := 17;
  l := 29L;
  map: game_map = ();
  map.add_entity(i);
  map.add_entity(l);
  [[assert: map.entities[0u] == i&]]
  [[assert: map.entities[1u] != l&]]
}

No warnings, but the last assertion passes.

Program returned: 0

[AbhinavK00]

Thanks for the example! While my point 2) in #696 does solve the problem, it will require rewriting the example to some other form (possibly taking in pointers as parameters) which is not desirable.

[jcanizales]

I'm struggling with that particular example.

The program is wrong, of course, but making add_entity lower to:

template <typename T>
game_map::add_entity(const T& x) {
  entities.push_back(&x);
}

instead of

template <typename T>
game_map::add_entity(T x) {
  entities.push_back(&x);
}

doesn't fix it. You can still pass a temporary to const T& and the program remains wrong. The problem here is passing the address of x somewhere that outlives it guaranteed validity.

If I want to express "pass me an object whose address outlives me", I can (and should) do:

game_map: @struct type = {
  entities: std::vector<* const void>;
  add_entity: (inout this, x: *_) = entities.push_back(x);
}

no?

[JohelEGP]

Yes.
From #696:

Like using pointers, which I might, were I writing a new interface.
But this issue will definitely come up when translating Cpp1 to Cpp2.