-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Add tests for static async functions in traits #102642
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) soon. Please see the contribution instructions for more information. |
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.
Some initial comments, probably not comprehensive but wanted to do a first pass. I will take another look at it in a few days.
where | ||
Self: 'a; | ||
|
||
async fn foo(&self) -> Self::Fut<'a>; |
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.
Can you fix this? This should probably be &'a self
(and adding 'a
generic lifetime) or Self::Fut<'_>
.
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.
async fn foo(&self) -> Self::Fut<'a>; | |
fn foo(&self) -> Self::Fut<'a>; |
I'm not 100% sure what this test is trying to test, but the name "desugared" suggests to me that it's trying to write out explicitly the equivalent of an async fn
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.
Can you fix this? This should probably be
&'a self
(and adding'a
generic lifetime) orSelf::Fut<'_>
.
oh, yeah that makes sense. I'll update 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.
@spastorino what did you have in mind for this one (example 10 in your HackMD)? I know what desugared means in a general sense, not sure what that would look like for this though.
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.
Actually, I understand now, after re-reading the static AFIT RFC
This comment has been minimized.
This comment has been minimized.
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.
Gave a quick skim, though not over everything. Some questions!
where | ||
Self: 'a; | ||
|
||
async fn foo(&self) -> Self::Fut<'a>; |
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.
async fn foo(&self) -> Self::Fut<'a>; | |
fn foo(&self) -> Self::Fut<'a>; |
I'm not 100% sure what this test is trying to test, but the name "desugared" suggests to me that it's trying to write out explicitly the equivalent of an async fn
Added some additional tests |
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.
Left some more feedback.
This patch adds test cases for AFIT, the majority of which are currently expected to run as `check-fail`.
Co-authored-by: Michael Goulet <michael@errs.io>
- Add comment to some tests that will break when rust-lang#102745 is implemented - Mark a test with known-bug - Delete duplicate test
c212812
to
bfdefdb
Compare
Thanks! I updated it now |
This looks good for now. @bryangarza, make sure to note what has test coverage and what doesn't for future AFIT/RPITIT test writers. Not sure if there's a hackmd already for that. @bors r+ rollup |
…errors Add tests for static async functions in traits This patch adds test cases for AFIT, the majority of which are currently expected to run as `check-fail`. --- Note: I grabbed the cases from https://hackmd.io/SwRcXCiWQV-WRJ4BYs53fA Also, I'm not sure if the `async-associated-types2` and `async-associated-types2-desugared` are correct, I modified them a bit from the examples in the HackMD.
…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#102642 (Add tests for static async functions in traits) - rust-lang#103283 (Add suggestions for unsafe impl error codes) - rust-lang#103523 (Fix unwanted merge of inline doc comments for impl blocks) - rust-lang#103550 (diagnostics: do not suggest static candidates as traits to import) - rust-lang#103641 (Don't carry MIR location in `ConstraintCategory::CallArgument`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This patch adds test cases for AFIT, the majority of which are currently expected to run as
check-fail
.Note: I grabbed the cases from https://hackmd.io/SwRcXCiWQV-WRJ4BYs53fA
Also, I'm not sure if the
async-associated-types2
andasync-associated-types2-desugared
are correct, I modified them a bit from the examples in the HackMD.