cppfront icon indicating copy to clipboard operation
cppfront copied to clipboard

[BUG] cppfront generates illegal code in an inspect returning a struct

Open threeifbyair opened this issue 1 year ago • 3 comments

If I declare a struct, and then use it as the result of an inspect, cppfront generates code constructing the struct with no arguments -- and there's no way to declare an operator= with no arguments in a struct!

Steps to reproduce the behavior:

https://cpp2.godbolt.org/z/K6f9h9qjx

action: @struct type = {
    dx: int;
    dy: int;
}

testfn: (x: int, y: int) -> action = {
    return inspect (x,y) -> action {
        is (0, 0) = action(1, 0);
        is (0, 1) = action(0, 1);
        is _      = action(1, 1);
    };
}

cppfront generates this as the definition of action:

class action {
    public: int dx; 
    public: int dy; 
    public: action(auto&& dx_, auto&& dy_)
CPP2_REQUIRES_ (std::is_convertible_v<CPP2_TYPEOF(dx_), std::add_const_t<int>&> && std::is_convertible_v<CPP2_TYPEOF(dy_), std::add_const_t<int>&>) ;

};

and this as the definition of testfn:

[[nodiscard]] auto testfn(cpp2::impl::in<int> x, cpp2::impl::in<int> y) -> action{
    return [&] () -> action { auto&& _expr = (x, y);
        if (cpp2::impl::is(_expr, (0, 0))) { if constexpr( requires{action(1, 0);} ) if constexpr( std::is_convertible_v<CPP2_TYPEOF((action(1, 0))),action> ) return action(1, 0); else return action{}; else return action{}; }
        else if (cpp2::impl::is(_expr, 0, 1)) { if constexpr( requires{action(0, 1);} ) if constexpr( std::is_convertible_v<CPP2_TYPEOF((action(0, 1))),action> ) return action(0, 1); else return action{}; else return action{}; }
        else return action(1, 1); }
    (); 
}

Obviously action{} is a zero-parameter constructor, and action doesn't have any zero-argument constructors. (And even if I ensure that action{} is on a code path never taken, both gcc 14 and Clang 19 try to compile it and throw an error.)

threeifbyair avatar Dec 07 '24 01:12 threeifbyair

For it to be default constructible, all the members must have default initializers.

However, I'm not sure this should really require it to be default constructible. Need to ponder a bit more.

gregmarr avatar Dec 07 '24 04:12 gregmarr

There is a syntax error in the generation here:

cpp2

        is (0, 1) = action(0, 1);

cpp1

else if (cpp2::impl::is(_expr, 0, 1))

Missing parens:

else if (cpp2::impl::is(_expr, (0, 1)))

I fixed that and it compiled properly, as all the if constexpr else branches were unused.

gregmarr avatar Dec 07 '24 05:12 gregmarr

So I finally got to looking at this again, and here's the thing. if constexpr else branches are used if the function is not templated (or the constexpr does not depend on the template parameters). So just putting things in parentheses doesn't help.

Not only that, but those parentheses aren't what was intended anyway. What you actually need is std::tuple<int, int>(0, 1) instead.

So... now that I have official permission to sign a CLA, I'll come up with something!

threeifbyair avatar Mar 19 '25 02:03 threeifbyair