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] Function expression can't be the left argument to is/as #435

Closed
JohelEGP opened this issue May 8, 2023 · 8 comments · Fixed by #479
Closed

[BUG] Function expression can't be the left argument to is/as #435

JohelEGP opened this issue May 8, 2023 · 8 comments · Fixed by #479
Labels
bug Something isn't working

Comments

@JohelEGP
Copy link
Contributor

JohelEGP commented May 8, 2023

Title: Function expression can't be the left argument to is/as.

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

main: () = {
  :() = 0; is int;
}

Commands:

cppfront -clean-cpp1 main.cpp2

Expected result:

Same as (https://cpp2.godbolt.org/z/8nszP1Wo4):

main: () = {
  cp := :(x) -> _ = x;
  cp(:() = 0) is int;
}

Actual result and error:

main.cpp2(1,12): error: ill-formed initializer (at '{')
main.cpp2(1,1): error: unexpected text at end of Cpp2 code section (at 'main')
main.cpp2(1,0): error: parse failed for section starting here
@JohelEGP JohelEGP added the bug Something isn't working label May 8, 2023
@msadeqhe
Copy link

msadeqhe commented May 9, 2023

I think your example code is wrong, because:

  • If the return type of an unnamed function is omitted, it will be void.
  • Use parenthesis instead of ;, otherwise the statement will be ended before is.

This is the correct example code (it needs parenthesis around the unnamed function):

main: () = {
    x: = (: () -> int = 0) is int; // true
}

Or this one (it doesn't need parenthesis around the unnamed function):

main: () = {
    x: = : () = {} is int; // false
}

@filipsajdak
Copy link
Contributor

I have a fix for that. @JohelEGP found an actual bug.

At the same time, I agree with you, @msadeqhe, that this code should rise an error as it discards the result of the is expression.

@filipsajdak
Copy link
Contributor

I need to correct my build system to handle self-compilation by the previous version of cppfront. I will deliver the patch soon.

@filipsajdak
Copy link
Contributor

By writing above messages I just realize that is and as functions has discardable results - probably they should be all marked with [[nodiscard]]

@filipsajdak
Copy link
Contributor

Just in case you would like to test the fix by yourself, or if I am not able to deliver fast, I am attaching a diff:

diff --git a/source/parse.h b/source/parse.h
index f716925..65d2fb0 100644
--- a/source/parse.h
+++ b/source/parse.h
@@ -3606,6 +3606,10 @@ private:
                     && curr().type() != lexeme::LeftParen               // not imediatelly called
                     && curr().type() != lexeme::RightParen              // not as a last argument to function
                     && curr().type() != lexeme::Comma                   // not as first or in-the-middle, function argument
+                    && curr() != "is"
+                    && curr() != "as"
+                    && curr() != "do"
+                    && curr() != "next"
                 ) {
                     // this is a fix for a short function syntax that should have double semicolon used
                     // (check comment in expression_statement(bool semicolon_required))

@JohelEGP
Copy link
Contributor Author

JohelEGP commented May 9, 2023

If the return type of an unnamed function is omitted, it will be void.

It's still valid code. Not practically useful, but slide-ware/example-ware useful. Like for reporting bugs.

Use parenthesis instead of ;, otherwise the statement will be ended before is.

I think this should work to disambiguate when you want the is outside the function expression's statement.
Seems like a natural fit, considering the other cases you can add ;.
So I think it's a bug that it doesn't work.

I need to correct my build system to handle self-compilation by the previous version of cppfront. I will deliver the patch soon.

I didn't notice because the generated Cpp1 source files are checked in.
I just launched my usual command to compile cppfront and it worked.

@filipsajdak
Copy link
Contributor

I didn't notice because the generated Cpp1 source files are checked in.
I just launched my usual command to compile cppfront and it worked.

My usual command worked as well but after I have added reflect.h2 to dependencies I have created recurrence in dependencies.

Nevertheless, I want my cmake to be able to build cppfront from scratch.

@filipsajdak
Copy link
Contributor

The original code is not working, but the following is:

main: () = {
  (:() = 0;) is int;
}

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.

3 participants