-
Notifications
You must be signed in to change notification settings - Fork 242
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
Allow inspect
statements
#738
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (!is_expression) { | ||
errors.emplace_back( | ||
curr().position(), | ||
"(temporary alpha limitation) cppfront is still learning 'inspect' - only inspect expressions are currently supported" | ||
); | ||
return {}; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the correct implementation?
I don't think there's code to lower inspect statements.
What happens with this code?
main: () = {
inspect 0 {
is 0 = std::cout << "0\n";
is _ = std::cout << "?\n";
}
}
In the test case, the inspect
is actually the expression of an expression-statement.
Could it be that is_expression
should have been true
in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh that doesn't work. So yes it seems the test case is doing return inspect v {
.
It does work actually!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there's code to lower inspect statements.
There is, in emit(inspect_expression_node, bool)
. If is_expression
is false, the lambda parameters and inline call are not emitted, but the braces still are emitted. So it just generates a block. You can see it in the diff here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While your example does work, the following does not:
inspect 0 {
is 0 = { std::cout << "0\n"; }
is _ = std::cout << "?\n";
}
The Cpp1 code has a semi-colon before the else
clause, causing an error. I'll try to fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you should scope your PR
to treating inspect
as an expression rather than as a statement
when it is a declaration's statement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now pushed a simple fix.
No, it gives:
|
Trailing spaces should not trigger that.
I'm not sure why they were disabled, the test below works.
Update: A small fix was needed to avoid emitting
}; else
Cpp1 when statement is a compound statement.