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] Return-by broken by "return expression list" support #408

Closed
JohelEGP opened this issue May 3, 2023 · 5 comments
Closed

[BUG] Return-by broken by "return expression list" support #408

JohelEGP opened this issue May 3, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@JohelEGP
Copy link
Contributor

JohelEGP commented May 3, 2023

Title: Return-by broken by ab29f19.

That's my guess.

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

f: (forward x) -> forward _ = (forward x);
g: (forward x) -> forward _ = { return (forward x); }
main: () = { }

Commands:

cppfront -clean-cpp1 main.cpp2
clang++17 -std=c++2b -stdlib=libc++ -lc++abi -I . main.cpp

Expected result:

[[nodiscard]] auto f(auto&& x) -> auto&& { return CPP2_FORWARD(x);  }
[[nodiscard]] auto g(auto&& x) -> auto&&{return CPP2_FORWARD(x); }

Actual result and error:

[[nodiscard]] auto f(auto&& x) -> auto&& { return { CPP2_FORWARD(x) };  }
[[nodiscard]] auto g(auto&& x) -> auto&&{return { CPP2_FORWARD(x) }; }
Cpp2 lowered to Cpp1.
#include "cpp2util.h"


[[nodiscard]] auto f(auto&& x) -> auto&&;
[[nodiscard]] auto g(auto&& x) -> auto&&;
auto main() -> int;

[[nodiscard]] auto f(auto&& x) -> auto&& { return { CPP2_FORWARD(x) };  }
[[nodiscard]] auto g(auto&& x) -> auto&&{return { CPP2_FORWARD(x) }; }
auto main() -> int{}
main.cpp2:1:51: error: cannot deduce return type from initializer list
[[nodiscard]] auto f(auto&& x) -> auto&& { return { CPP2_FORWARD(x) };  }
                                                  ^~~~~~~~~~~~~~~~~~~
main.cpp2:2:49: error: cannot deduce return type from initializer list
[[nodiscard]] auto g(auto&& x) -> auto&&{return { CPP2_FORWARD(x) }; }
                                                ^~~~~~~~~~~~~~~~~~~
2 errors generated.
@JohelEGP JohelEGP added the bug Something isn't working label May 3, 2023
@JohelEGP JohelEGP changed the title [BUG] Return-by broken by ab29f19d84985594569d27e11db8190bb54bf019 [BUG] Return-by broken by "return expression list" support May 3, 2023
@JohelEGP

This comment was marked as outdated.

@JohelEGP
Copy link
Contributor Author

JohelEGP commented May 3, 2023

I think that'd make the parameter-directions in expression-list redundant.

 //G expression-list:
-//G     parameter-direction? expression
-//G     expression-list ',' parameter-direction? expression
+//G     expression
+//G     expression-list ',' expression

This change would also make it so that
parentheses are required around a passed-by binary expression.
That's a plus, as a conventional binary expression should require no pass-by.
E.g., f(move a + b); -> f(move (a + b));.

This is probably necessary, anyways.

EDIT: Rereading #77 (comment), forward already works like a prefix operator: https://cpp2.godbolt.org/z/a1eeMvWhK.

@JohelEGP
Copy link
Contributor Author

JohelEGP commented May 3, 2023

I just noticed this, to replace #408 (comment).

 //G prefix-operator:
 //G     one of  '!' '-' '+'
-//GT     parameter-direction
+//G     parameter-direction

@JohelEGP
Copy link
Contributor Author

JohelEGP commented May 22, 2023

An alternative to the general solution above:

 //G return-statement:
-//G     return expression? ';'
+//G     return parameter-direction? expression? ';'

But the general solution is more useful.
There are more unparenthesized arguments in Cpp1 and Cpp2, like in a = b.

@hsutter
Copy link
Owner

hsutter commented Aug 10, 2023

Thanks! Got here from #487 -- for now I think the right fix is to not emit { } on deduced returns, which can change the type. We still want the { } on non-deduced returns I think. Anyway, the coming commit should fix this problem, and if there's more we can look at it again with new examples.

zaucy pushed a commit to zaucy/cppfront that referenced this issue Dec 5, 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

No branches or pull requests

2 participants