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

Add tests for static async functions in traits #102642

Merged
merged 6 commits into from
Oct 28, 2022

Conversation

bryangarza
Copy link
Contributor

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.

@rust-highfive
Copy link
Collaborator

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.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 4, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 4, 2022
@bryangarza
Copy link
Contributor Author

r? @compiler-errors

Copy link
Member

@compiler-errors compiler-errors left a 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>;
Copy link
Member

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<'_>.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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

Copy link
Contributor Author

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<'_>.

oh, yeah that makes sense. I'll update it

Copy link
Contributor Author

@bryangarza bryangarza Oct 4, 2022

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.

Copy link
Contributor Author

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

@rust-log-analyzer

This comment has been minimized.

Copy link
Contributor

@nikomatsakis nikomatsakis left a 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!

src/test/ui/async-await/in-trait/async-associated-types.rs Outdated Show resolved Hide resolved
where
Self: 'a;

async fn foo(&self) -> Self::Fut<'a>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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

@bryangarza
Copy link
Contributor Author

Added some additional tests

Copy link
Member

@compiler-errors compiler-errors left a 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.

bryangarza and others added 6 commits October 27, 2022 22:58
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
@bryangarza
Copy link
Contributor Author

Thanks! I updated it now

@compiler-errors
Copy link
Member

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

@bors
Copy link
Contributor

bors commented Oct 28, 2022

📌 Commit bfdefdb has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 28, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 28, 2022
…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.
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 28, 2022
…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
@bors bors merged commit c404092 into rust-lang:master Oct 28, 2022
@rustbot rustbot added this to the 1.66.0 milestone Oct 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants