Skip to content

Add a new check-pass UI test for returning impl Fn(T) -> impl Trait #136971

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

Merged
merged 1 commit into from
Feb 15, 2025

Conversation

LFS6502
Copy link
Contributor

@LFS6502 LFS6502 commented Feb 13, 2025

This PR closes #107883 by adding a ui test.

@rustbot
Copy link
Collaborator

rustbot commented Feb 13, 2025

r? @fmease

rustbot has assigned @fmease.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 13, 2025
@LFS6502
Copy link
Contributor Author

LFS6502 commented Feb 13, 2025

r? WaffleLapkin

@rustbot rustbot assigned WaffleLapkin and unassigned fmease Feb 13, 2025
@LFS6502
Copy link
Contributor Author

LFS6502 commented Feb 13, 2025

I need some ideas for what to call this test. I thought rpit-fn-trait-rpit but I'm not convinced.

@LFS6502 LFS6502 marked this pull request as ready for review February 13, 2025 11:45
@LFS6502 LFS6502 changed the title Add a new check-pass UI test for Returning impl Fn(T) -> impl Trait Add a new check-pass UI test for returning impl Fn(T) -> impl Trait Feb 13, 2025
Copy link
Member

@fmease fmease left a comment

Choose a reason for hiding this comment

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

The test should be placed under tests/ui/impl-trait/.

Regarding naming, it would help to bisect the reprogression (e.g., with cargo-bisect-rustc) and see what PR made it compile and more importantly why (it didn't compile and now does). Moreover, looking at that PR could help us determine if we need extra regression tests at all or if it's sufficiently covered already.

@LFS6502 LFS6502 marked this pull request as draft February 13, 2025 11:56
@LFS6502
Copy link
Contributor Author

LFS6502 commented Feb 13, 2025

I ran the bisection

bisect-rustc results

cargo bisect-rustc --start=2023-02-10 --end=2024-07-20 --regress success

RESULT: nightly-2024-04-10, ===> Successfully compiled
uninstalling nightly-2024-04-10

searched toolchains nightly-2023-02-10 through nightly-2024-07-20


Regression in nightly-2024-04-10


fetching https://static.rust-lang.org/dist/2024-04-09/channel-rust-nightly-git-commit-hash.txt
nightly manifest 2024-04-09: 40 B / 40 B [================================================================================================] 100.00 % 1.48 MB/s converted 2024-04-09 to ab5bda1
fetching https://static.rust-lang.org/dist/2024-04-10/channel-rust-nightly-git-commit-hash.txt
nightly manifest 2024-04-10: 40 B / 40 B [================================================================================================] 100.00 % 1.65 MB/s converted 2024-04-10 to 8b2459c
looking for regression commit between 2024-04-09 and 2024-04-10
fetching (via remote github) commits from max(ab5bda1, 2024-04-07) to 8b2459c
ending github query because we found starting sha: ab5bda1
get_commits_between returning commits, len: 10
commit[0] 2024-04-08: Auto merge of #123645 - matthiaskrgr:rollup-yd8d7f1, r=matthiaskrgr
commit[1] 2024-04-08: Auto merge of #122077 - oli-obk:eager_opaque_checks4, r=lcnr
commit[2] 2024-04-09: Auto merge of #122387 - DianQK:re-enable-early-otherwise-branch, r=cjgillot
commit[3] 2024-04-09: Auto merge of #123099 - oli-obk:span_tcx, r=petrochenkov
commit[4] 2024-04-09: Auto merge of #123663 - matthiaskrgr:rollup-1qnj9j3, r=matthiaskrgr
commit[5] 2024-04-09: Auto merge of #123272 - saethlin:reachable-mono-cleanup, r=cjgillot
commit[6] 2024-04-09: Auto merge of #123631 - oli-obk:fail_slow, r=jieyouxu
commit[7] 2024-04-09: Auto merge of #123676 - GuillaumeGomez:rollup-1hurixy, r=GuillaumeGomez
commit[8] 2024-04-09: Auto merge of #123485 - madsmtm:use-libc-copyfile, r=joboet
commit[9] 2024-04-09: Auto merge of #123683 - pietroalbini:pa-cve-2024-24576-nightly, r=pietroalbini
ERROR: no CI builds available between ab5bda1 and 8b2459c within last 167 days

@fmease
Copy link
Member

fmease commented Feb 13, 2025

Likely #122077

@LFS6502
Copy link
Contributor Author

LFS6502 commented Feb 13, 2025

I've begun doing a manual bisect to be sure.

@LFS6502
Copy link
Contributor Author

LFS6502 commented Feb 13, 2025

Likely #122077

This is correct.

ab5bda1 fails the test
b234e44 passes the test

#122077

@LFS6502
Copy link
Contributor Author

LFS6502 commented Feb 13, 2025

Output when compiling the test with b234e44

Errors
rustc +stage1 ../bisect-test
error: concrete type differs from previous defining opaque type use
 --> ../bisect-test:9:10
  |
9 |     |_x| 15
  |          ^^ expected `impl Debug`, got `i32`
  |
note: previous use here
 --> ../bisect-test:9:5
  |
9 |     |_x| 15
  |     ^^^^^^^

error[E0720]: cannot resolve opaque type
 --> ../bisect-test:8:28
  |
8 | fn f<T>() -> impl Fn(T) -> impl Debug {
  |                            ^^^^^^^^^^ cannot resolve opaque type

error: concrete type differs from previous defining opaque type use
  --> ../bisect-test:25:10
   |
25 |     |_x| 15
   |          ^^ expected `impl Debug`, got `i32`
   |
note: previous use here
  --> ../bisect-test:25:5
   |
25 |     |_x| 15
   |     ^^^^^^^

error[E0720]: cannot resolve opaque type
  --> ../bisect-test:24:37
   |
24 | fn h<T>() -> impl Fn<(T,), Output = impl Debug> {
   |                                     ^^^^^^^^^^ cannot resolve opaque type

error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0720`.`

Now it's a matter of deciding what the test name should be.

@LFS6502 LFS6502 marked this pull request as ready for review February 13, 2025 22:51
@LFS6502
Copy link
Contributor Author

LFS6502 commented Feb 13, 2025

@WaffleLapkin It's ready

@fmease I implemented your feedback, thanks!

Copy link
Member

@WaffleLapkin WaffleLapkin left a comment

Choose a reason for hiding this comment

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

just a single nitpick and this can be merged :)

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 14, 2025
Change description from compiletest to regression test

Co-authored-by: 许杰友 Jieyou Xu (Joe) <39484203+jieyouxu@users.noreply.github.com>

Improve test name, location, and description

Update tests/ui/impl-trait/impl-fn-rpit-opaque-107883.rs

Co-authored-by: waffle <waffle.lapkin@gmail.com>
@LFS6502
Copy link
Contributor Author

LFS6502 commented Feb 14, 2025

Thanks!

@WaffleLapkin
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Feb 14, 2025

📌 Commit 7d1262a has been approved by WaffleLapkin

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Feb 14, 2025
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Feb 14, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Feb 14, 2025
Add a new check-pass UI test for returning `impl Fn(T) -> impl Trait`

This PR closes rust-lang#107883 by adding a ui test.
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 14, 2025
Rollup of 11 pull requests

Successful merges:

 - rust-lang#133312 (triagebot: automatically add more rustdoc related labels)
 - rust-lang#134016 (Stabilize `const_is_char_boundary` and `const_str_split_at`.)
 - rust-lang#135813 (CI: split i686-mingw job to three free runners)
 - rust-lang#136879 (Add safe new() to NotAllOnes)
 - rust-lang#136971 (Add a new check-pass UI test for returning `impl Fn(T) -> impl Trait`)
 - rust-lang#136983 (Prepare standard library for Rust 2024 migration)
 - rust-lang#137002 (Fix early lint check desc in query)
 - rust-lang#137006 (borrowck diagnostics cleanup: remove an unused and a barely-used field)
 - rust-lang#137026 (Stabilize (and const-stabilize) `integer_sign_cast`)
 - rust-lang#137028 (mir_build: Clarify some code for lowering `hir::PatExpr` to THIR)
 - rust-lang#137032 (Decode metadata buffer in one go)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 15, 2025
…kingjubilee

Rollup of 10 pull requests

Successful merges:

 - rust-lang#133312 (triagebot: automatically add more rustdoc related labels)
 - rust-lang#134016 (Stabilize `const_is_char_boundary` and `const_str_split_at`.)
 - rust-lang#136971 (Add a new check-pass UI test for returning `impl Fn(T) -> impl Trait`)
 - rust-lang#136983 (Prepare standard library for Rust 2024 migration)
 - rust-lang#137002 (Fix early lint check desc in query)
 - rust-lang#137006 (borrowck diagnostics cleanup: remove an unused and a barely-used field)
 - rust-lang#137032 (Decode metadata buffer in one go)
 - rust-lang#137035 (Normalize closure instance before eagerly monomorphizing it)
 - rust-lang#137037 (add x86-sse2 (32bit) ABI that requires SSE2 target feature)
 - rust-lang#137038 (llvm: Tolerate captures in tests)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2ec48fb into rust-lang:master Feb 15, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Feb 15, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 15, 2025
Rollup merge of rust-lang#136971 - HypheX:patch1, r=WaffleLapkin

Add a new check-pass UI test for returning `impl Fn(T) -> impl Trait`

This PR closes rust-lang#107883 by adding a ui test.
@LFS6502 LFS6502 deleted the patch1 branch February 15, 2025 05:04
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.

Returning impl Fn(T) -> impl Trait does not compile, unless you add an identity call
6 participants