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] cppfront confused by function expression in for's (next) expression #432

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

[BUG] cppfront confused by function expression in for's (next) expression #432

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

Comments

@JohelEGP
Copy link
Contributor

JohelEGP commented May 7, 2023

Title: Confused by function expression in for's (next) expression.

Description:

It seems something enters a bad state.
Unparenthesized, it complains about a missing do.
Parenthesized, lifetime analysis says the loop variable is uninitialized.

Minimal reproducer (https://cpp2.godbolt.org/z/Er81zc91E, https://cpp2.godbolt.org/z/a75sW77bs):

main: (args) = {
  for args next :() = 0 do (arg) std::cout << arg;
}
main: (args) = {
  for args next (:() = 0) do (arg) std::cout << arg;
}

Commands:

cppfront -clean-cpp1 main.cpp2

Actual result and error:

main.cpp2(2,25): error: 'for range' must be followed by 'do ( parameter )' (at 'do')
main.cpp2(2,49): error: local variable arg is used before it was initialized
  ==> program violates initialization safety guarantee - see previous errors
@JohelEGP JohelEGP added the bug Something isn't working label May 7, 2023
@filipsajdak
Copy link
Contributor

:() = 0 unnamed function should end with ; (semicolon). There is a bug that I have introduced by adding a check for double semicolons after short function syntax. Probably can be fixed by the following patch:

diff --git a/source/parse.h b/source/parse.h
index f716925..3165ec0 100644
--- a/source/parse.h
+++ b/source/parse.h
@@ -3606,6 +3606,7 @@ 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().type() != lexeme::Keyword                 // not as next expression
                 ) {
                     // this is a fix for a short function syntax that should have double semicolon used
                     // (check comment in expression_statement(bool semicolon_required))

Additionally, arg is not treated as initialized... don't know why.

Also, shall we allow statements as for-body? I was sure that it require a compound statement as ifs.

@JohelEGP
Copy link
Contributor Author

JohelEGP commented May 7, 2023

:() = 0 unnamed function should end with ; (semicolon).

When? Always? When not an expression-statement (the only case that would require double ;s, AFAIK)?

@filipsajdak
Copy link
Contributor

Ha! You are right, and I am mistaken. I am pretty sure it was required before. Maybe my memory failed here.

The following code explains it (https://cpp2.godbolt.org/z/o11cYnKj6):

main: () -> int = {

    l := :() = std::cout << 42;                         // require one semicolon
    l();

    :() = std::cout << " >>instantly called<< ";();     // require one semicolon before ()

    return fun(:() -> int = 0+1, :() = std::cout << 1); // semicolon is not required in both cases

}

fun: (f, g) -> int = {
    g();
    return f();
}

@JohelEGP
Copy link
Contributor Author

JohelEGP commented May 8, 2023

By the way, I did try adding ;s before reporting.
The results were the same, so I chose to not mention that.

I was just wondering if you had the answer about ; after a simple function expression.

I've thought about it some more, and I think the answer lies in this grammar:

//G expression-statement:
//G     expression ';'
//G     expression

So the ; is optional when parsing the function expression's statement.
Except when the FE is a statement itself
(rejected during parse, probably left-shifted from semantic analysis):

main: () = {
  :() = 0 // `f` can't be part of the FE's statement, but still requires `;`.
  f();
}

When the FE is in an expression-list,
,, ), and ], can end the it (; or not).
As they would any other primary-expression (the , thanks to no operator,).

See https://cpp2.godbolt.org/z/6j9rq1bvb.

@JohelEGP JohelEGP changed the title [BUG] Confused by function expression in for's (next) expression [BUG] cppfront confused by function expression in for's (next) expression May 8, 2023
@realgdman
Copy link

realgdman commented May 8, 2023

I've been exploring missing ; in short-body lambdas and tried to invent dangerously looking example, and want to share my attempt so far

foo: (x: int) -> int = { return x; }
a : std::function<int(int)> = foo&; //don't know another ways to express ptr to fn
	
main: () = {
	s1 := :(x) ->_ =a;(1);
	s2 := :(x) ->_ =a(2);   //note missing ;
	
	std::cout << s1(3) << std::endl; //prints 3
	std::cout << s2(4) << std::endl; //prints 2

	supress_last_use(s1, s2);
}

supress_last_use: (x,y) = {}

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