-
Notifications
You must be signed in to change notification settings - Fork 163
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
Split async and const function qualifiers #2751
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
P-E-P
force-pushed
the
async_const_functions
branch
2 times, most recently
from
November 22, 2023 09:15
8847069
to
bec3971
Compare
A function cannot be both async and const, however this should not be handled in the parser but rather at a later stage in the compiler. This commit change the AsyncConstStatus in the AST and the HIR to allows a function to be both async and const. gcc/rust/ChangeLog: * ast/rust-ast-builder.cc (AstBuilder::fn_qualifiers): Change constructor to match the new arguments. * ast/rust-ast-collector.cc (TokenCollector::visit): Change behavior to handle both const and async specifiers at the same time. * ast/rust-ast.cc (FunctionQualifiers::as_string): Likewise. * ast/rust-item.h (class FunctionQualifiers): Remove AsyncConstStatus and replace it with both Async and Const status. Also change the safety arguments to use an enum instead of a boolean. * hir/rust-ast-lower-base.cc (ASTLoweringBase::lower_qualifiers): Update constructor call. * hir/tree/rust-hir-item.h: Add Const and Async status, remove AsyncConstStatus, update the constructor. * hir/tree/rust-hir.cc (FunctionQualifiers::as_string): Update with the new status. * parse/rust-parse-impl.h (Parser::parse_function_qualifiers): Update constructor call. * util/rust-common.h (enum Mutability): Make an enum class. (enum class): Add Async and Const enum class to avoid booleans. (enum Unsafety): Change to an enum class. Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
We need to account for const specifiers in async parsing as const can be used in the syntax before the async keyword. gcc/rust/ChangeLog: * parse/rust-parse-impl.h (Parser::parse_vis_item): Allow parsing async items in const. (Parser::parse_async_item): Account for const offset during async lookahead. Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
Add a check during AST validation pass to ensure functions are either const or async but not both. gcc/rust/ChangeLog: * checks/errors/rust-ast-validation.cc (ASTValidation::visit): Add async const check. Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
Functions that are both async and const shall be rejected during the AST validation pass. This new test highlight this behavior. gcc/testsuite/ChangeLog: * rust/compile/const_async_function.rs: New test. Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
P-E-P
force-pushed
the
async_const_functions
branch
from
November 22, 2023 10:13
1ce07b4
to
d097d19
Compare
CohenArthur
approved these changes
Dec 1, 2023
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.
LGTM
Comment on lines
+40
to
+50
enum class Const | ||
{ | ||
Yes, | ||
No, | ||
}; | ||
|
||
enum class Async | ||
{ | ||
Yes, | ||
No | ||
}; |
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.
Great stuff, these enums are much better than booleans for use cases like these
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Function cannot be both async and const but this needs to be rejected at a later stage than parsing, during the ast validation pass.
Probably requires #2736