Skip to content

[FIX] multi return values handling when lambda is used inside the function #113

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

Closed

Conversation

filipsajdak
Copy link
Contributor

Closes #112

The current implementation fails to handle cases when there is a lambda in the function where multi-return values are used. The issue is with the get_declaration_of() function that tries not to go beyond the current function but is misled by a lambda function

//  Don't look beyond the current function
assert(decl.declaration);
if (decl.declaration->type.index() == declaration_node::function) {
    return nullptr;
}

It makes it impossible to compile below code:

fun: () -> (ri : int) = {
    ri = 0;

    pred := :(e:_) -> bool = { return e == 1; };

    ri = 42;

    return;
}

as the second call to ri variable is not getting a proper declaration from the get_declaration_of() function and it makes that cppfront generates:

#line 1 "tests/get_declarations_return_vals.cpp2"
[[nodiscard]] auto fun() -> fun__ret{
        cpp2::deferred_init<int> ri;
#line 2 "tests/get_declarations_return_vals.cpp2"
    ri.construct(0);

    auto pred { [](auto const& e) -> bool{return e == 1; } }; 

    ri = 42; // <--- lack of call to .value() method

    return  { std::move(ri.value()) }; 
}

This fix makes cppfront compile the above code to:

#line 1 "tests/get_declarations_return_vals.cpp2"
[[nodiscard]] auto fun() -> fun__ret{
        cpp2::deferred_init<int> ri;
#line 2 "tests/get_declarations_return_vals.cpp2"
    ri.construct(0);

    auto pred { [](auto const& e) -> bool{return e == 1; } }; 

    ri.value() = 42;

    return  { std::move(ri.value()) }; 
}

Also, this change fixes other issues when the multi-return values are initialized in the declaration place. The below code:

fun: () -> (ri : int = 0) = {
    pred := :(e:_) -> bool = { return e == 1; };

    ri = 42;

    return;
}

now compiles to:

#line 1 "/tests/get_declarations_return_vals.cpp2"
[[nodiscard]] auto fun() -> fun__ret{
    int ri = 0;
#line 2 "tests/get_declarations_return_vals.cpp2"
    auto pred { [](auto const& e) -> bool{return e == 1; } }; 

    ri = 42;

    return  { std::move(ri) }; // previously: return  { std::move(ri.value()) };
}

@filipsajdak
Copy link
Contributor Author

I have a thought about what will happen when lambda itself will return multiple values. I have checked the code:

fun: () -> (ri : int) = {
    ri = 0;

    pred := :(e:_) -> (f:bool) = { f = e == 1; return; };

    ri = 42;

    return;
}

And I have an error message from cppfront that this is not a supported feature, yet.

get_declarations_return_vals.cpp2(4,57): error: an unnamed function at expression scope currently cannot return multiple values (at ';')

Unnamed functions also do not support contracts.

an unnamed function at expression scope currently cannot have contracts

cppfront also checks if there is no identifier set for an unnamed function - that makes my solution a good way of deducing if the function is indeed an unnamed function.

@filipsajdak filipsajdak force-pushed the fsajdak-fix-multireturn-handling branch from 1a05b5e to 4e6d147 Compare November 26, 2022 16:22
@filipsajdak filipsajdak force-pushed the fsajdak-fix-multireturn-handling branch from 4e6d147 to a08dabb Compare November 29, 2022 21:48
@filipsajdak filipsajdak changed the title [FIX] multi return values handling [FIX] multi return values handling when lambda is used inside the function Nov 30, 2022
@hsutter hsutter closed this in 3faeae3 Dec 3, 2022
@hsutter
Copy link
Owner

hsutter commented Dec 3, 2022

Looks good, thanks!

I introduced conflicts so I applied essentially the same change as a separate commit instead of asking you to rebase. Please let me know if you spot a mistake, but it passes regression and your two test cases above.

@filipsajdak filipsajdak deleted the fsajdak-fix-multireturn-handling branch December 22, 2022 10:44
Azmah-Bad pushed a commit to Azmah-Bad/cppfront that referenced this pull request Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Multiple return values failed to work in presence of lambda
2 participants