cppfront icon indicating copy to clipboard operation
cppfront copied to clipboard

[BUG] Move on definitive last use in fold expressions

Open Velocirobtor opened this issue 1 year ago • 5 comments

Describe the bug If the definitive last use of a variable is inside a fold expression, cppfront will still emit cpp2::move(<variable>) even though this will most likely lead to multiple moves from <variable>.

To Reproduce Compile the following cpp2 code:

foo: (forward item: _) = {
    std::print(" {}", std::is_rvalue_reference_v<decltype(item)>);
}
bar_fold: <Is...: std::size_t>() = {
    test := 42;
    ((Is, foo(test)), ...);
}
int main() {
    bar_fold<0, 1>();
}

This will generate this cpp1 code (omitting forward declarations):

auto foo(auto&& item) -> void{
    std::print(" {}", std::is_rvalue_reference_v<decltype(CPP2_FORWARD(item))>);
}
template<std::size_t ...Is> auto bar_fold() -> void{
    auto test {42}; 
    ((Is, foo(cpp2::move(test))), ...);
}
int main() {
    bar_fold<0, 1>();
}

which prints " true true"

Expected Behavior I think one of the following should happen:

  • It behaves like an "equivalent" for loop -> it prints " false false"
  • It behaves like an "equivalent" manually unrolled implementation -> it prints " false true"

The first would just require not emitting cpp2::move in fold expressions. The latter could be achieved by generating code somewhere along the lines of:

namespace cpp2 {
    template<bool Condition>
    constexpr decltype(auto) move_if(auto&& val) {
        if constexpr(Condition) {
            return move(val);
        } else {
            return val;
        }
    }
}
template<std::size_t... Is>
void bar() {
    auto test{ 42 };
    [&]<std::size_t... GeneratedIndices>(std::index_sequence<GeneratedIndices...>) -> decltype(auto) {
        return ((Is, foo(cpp2::move_if<GeneratedIndices + 1 == sizeof...(Is)>(test))), ...);
    }(std::make_index_sequence<sizeof...(Is)>{});
}

Link to Godbolt with the complete example: https://cpp2.godbolt.org/z/fcjWPfhsh

P.S. How can I silence the warning left operand of comma operator has no effect using cpp2 Syntax? I tried static_cast<void>(Is) which lead to cppfront suggesting Is as void, which didn't work.

Velocirobtor avatar Sep 16 '24 17:09 Velocirobtor

P.S. How can I silence the warning left operand of comma operator has no effect using cpp2 Syntax? I tried static_cast<void>(Is) which lead to cppfront suggesting Is as void, which didn't work.

You discard it with _ = Is (https://cpp2.godbolt.org/z/foP3fMzx8), see _ — the "don't care" wildcard, including explicit discard.

JohelEGP avatar Oct 04 '24 02:10 JohelEGP

You discard it with _ = Is (https://cpp2.godbolt.org/z/foP3fMzx8), see _ — the "don't care" wildcard, including explicit discard.

Awesome, thank you! For some reason I convinced myself that I couldn't use that in an expression and just as a standalone statement and didn't even try to use it... Glad to see it was just a user error on my part :)

Velocirobtor avatar Oct 07 '24 15:10 Velocirobtor

I tried static_cast<void>(Is) which lead to cppfront suggesting Is as void, which didn't work.

Looks like there's an opportunity for improving the diagnostic here. The general diagnostic is for preferring as to cast. For the void case, suggesting _ = Is is more appropriate.

JohelEGP avatar Oct 07 '24 15:10 JohelEGP

Yeah, that would have definitely helped me and I'd guess anyone else starting out with cpp2 coming from a cpp background. Though I'm not sure if the target type is available at the point of emitting the diagnostic, as the actual message was: error: 'static_cast<T>(val)' is not supported in Cpp2 - use 'val as T' ... (notice the use of T and not void)

Velocirobtor avatar Oct 07 '24 16:10 Velocirobtor

Looks like there's an opportunity for improving the diagnostic here.

Thanks! This part is done: 07b0eeda5e89639e355678bdee137df098902a8d

hsutter avatar Oct 07 '24 17:10 hsutter