Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Move/forward on last use doesn't apply to generated code #869

Closed
JohelEGP opened this issue Dec 4, 2023 · 0 comments · Fixed by #887
Closed

[BUG] Move/forward on last use doesn't apply to generated code #869

JohelEGP opened this issue Dec 4, 2023 · 0 comments · Fixed by #887
Labels
bug Something isn't working

Comments

@JohelEGP
Copy link
Contributor

JohelEGP commented Dec 4, 2023

Title: Move/forward on last use doesn't apply to generated code.

Description:

@union generates set functions with a forward parameter pack.
But the lowered code doesn't forward the pack on its last use.

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

v: @union @print type = {
  i: int;
}
main: () = { }
Commands:
cppfront main.cpp2
clang++18 -std=c++23 -stdlib=libc++ -lc++abi -pedantic-errors -Wall -Wextra -Wconversion -Werror=unused-result -Werror=unused-value -Werror=unused-parameter -I . main.cpp

Expected result:

Same as when you copy the @printed code to the Cpp2 source (https://cpp2.godbolt.org/z/fa4T55hGE):

    auto v::set_i(

        auto&& ..._args
    ) & -> void
    {
        if (!(is_i()))
        {
            _destroy();
            std::construct_at(reinterpret_cast<int*>(&_storage), CPP2_FORWARD(_args)...);
        }
        else
        {
            *cpp2::assert_not_null(reinterpret_cast<int*>(&_storage)) = int{CPP2_FORWARD(_args)...};
        }
        _discriminator = 0;
    }

Actual result and error:

No implicit forward on last use in the Cpp1 code lowered from generated Cpp2 code.

auto v::set_i(auto&& ..._args) & -> void{if (!(is_i())) {_destroy();std::construct_at(reinterpret_cast<int*>(&_storage), _args...);}else {*cpp2::assert_not_null(reinterpret_cast<int*>(&_storage)) = int{_args...};}_discriminator = 0;}
Cpp2 lowered to Cpp1:
//=== Cpp2 type declarations ====================================================


#include "cpp2util.h"

#line 1 "/app/example.cpp2"
class v;
#line 2 "/app/example.cpp2"


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

#line 1 "/app/example.cpp2"
class v {
private: std::aligned_storage_t<cpp2::max(sizeof(int))> _storage {}; private: cpp2::i8 _discriminator {-1}; public: [[nodiscard]] auto is_i() const& -> bool;
public: [[nodiscard]] auto i() const& -> int const&;
public: [[nodiscard]] auto i() & -> int&;
public: auto set_i(cpp2::in<int> _value) & -> void;
public: auto set_i(auto&& ..._args) & -> void;
private: auto _destroy() & -> void;
public: ~v() noexcept;
public: explicit v();
public: v(v const& that);

public: v(v&& that) noexcept;
public: auto operator=(v const& that) -> v& ;
public: auto operator=(v&& that) noexcept -> v& ;

#line 3 "/app/example.cpp2"
};
auto main() -> int;

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

#line 1 "/app/example.cpp2"


#line 1 "/app/example.cpp2"
[[nodiscard]] auto v::is_i() const& -> bool { return _discriminator == 0; }
[[nodiscard]] auto v::i() const& -> int const& {
                                           cpp2::Default.expects(is_i(), "");return *cpp2::assert_not_null(reinterpret_cast<int const*>(&_storage)); }
[[nodiscard]] auto v::i() & -> int& {
                                                 cpp2::Default.expects(is_i(), "");return *cpp2::assert_not_null(reinterpret_cast<int*>(&_storage)); }
auto v::set_i(cpp2::in<int> _value) & -> void{if (!(is_i())) {_destroy();std::construct_at(reinterpret_cast<int*>(&_storage), _value);}else {*cpp2::assert_not_null(reinterpret_cast<int*>(&_storage)) = _value;}_discriminator = 0;}
auto v::set_i(auto&& ..._args) & -> void{if (!(is_i())) {_destroy();std::construct_at(reinterpret_cast<int*>(&_storage), _args...);}else {*cpp2::assert_not_null(reinterpret_cast<int*>(&_storage)) = int{_args...};}_discriminator = 0;}
auto v::_destroy() & -> void{
if (_discriminator == 0) {std::destroy_at(reinterpret_cast<int*>(&_storage));}
_discriminator = -1;
}

v::~v() noexcept{_destroy();}
v::v(){}
v::v(v const& that)
        : _storage{  }
        , _discriminator{ -1 }{
if (CPP2_UFCS(is_i)(that)) {set_i(CPP2_UFCS(i)(that));}
}

v::v(v&& that) noexcept
        : _storage{  }
        , _discriminator{ -1 }{
if (CPP2_UFCS(is_i)(std::move(that))) {set_i(CPP2_UFCS(i)(std::move(that)));}
}

auto v::operator=(v const& that) -> v& {
if (CPP2_UFCS(is_i)(that)) {set_i(CPP2_UFCS(i)(that));}
        return *this;
}

auto v::operator=(v&& that) noexcept -> v& {
if (CPP2_UFCS(is_i)(std::move(that))) {set_i(CPP2_UFCS(i)(std::move(that)));}
        return *this;
}
#line 4 "/app/example.cpp2"
auto main() -> int{}
Output:
Step build returned: 0
[1/3] Generating main.cpp
main.cpp2...

v: type =
{
    _storage: std::aligned_storage_t<cpp2::max(sizeof(int))> = ();

    _discriminator: i8 = -1;

    is_i:(in this) -> move bool = _discriminator == 0;

    i:(in this) -> forward int
        pre( is_i() ) = reinterpret_cast<* const int>(_storage&)*;

    i:(inout this) -> forward int
        pre( is_i() ) = reinterpret_cast<* int>(_storage&)*;

    set_i:(
        inout this,
        in _value: int
    ) =
    {
        if !is_i()
        {
            _destroy();
            std::construct_at(reinterpret_cast<* int>(_storage&), _value);
        }
        else
        {
            reinterpret_cast<* int>(_storage&)* = _value;
        }
        _discriminator = 0;
    }

    set_i:(
        inout this,
        forward _args...:
    ) =
    {
        if !is_i()
        {
            _destroy();
            std::construct_at(reinterpret_cast<* int>(_storage&), _args...);
        }
        else
        {
            reinterpret_cast<* int>(_storage&)* = : int = (_args...);
        }
        _discriminator = 0;
    }

    private _destroy:(inout this) =
    {
        if _discriminator == 0
        {
            std::destroy_at(reinterpret_cast<* int>(_storage&));
        }
        _discriminator = -1;
    }

    operator=:(move this) =
    {
        _destroy();
    }

    operator=:(out this) =
    {
    }

    operator=:(
        out this,
        in that
    ) =
    {
        _storage = ();
        _discriminator = -1;
        if that.is_i()
        {
            set_i(that.i());
        }
    }

    operator=:(
        inout this,
        in that
    ) =
    {
        _storage = _;
        _discriminator = _;
        if that.is_i()
        {
            set_i(that.i());
        }
    }
}
 ok (all Cpp2, passes safety checks)

[2/3] Building CXX object CMakeFiles/main.dir/main.cpp.o
[3/3] Linking CXX executable main
Program returned: 0

See also:

Other bugs in this area:

@JohelEGP JohelEGP added the bug Something isn't working label Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant