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

Fix async Fn confirmation for FnDef/FnPtr/Closure types #121654

Merged
merged 5 commits into from
Feb 29, 2024

Conversation

compiler-errors
Copy link
Member

Fixes three issues:

  1. The code in extract_tupled_inputs_and_output_from_async_callable was accidentally getting the future type and the output type (returned by the future) messed up for fnptr/fndef/closure types. :/
  2. We have a (class of) bug(s) in the old solver where we don't really support higher ranked built-in Future goals for generators. This is not possible to hit on stable code, but can be hit with unboxed_closures (Confirming built-in higher-ranked Future (and other coroutine) goals ICEs #121653).
    • I'm opting not to fix that in this PR. Instead, I just instantiate placeholders when confirming async Fn goals.
  3. Fixed a bug when generating FnPtr shims for async Fn trait goals.

r? oli-obk

@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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative labels Feb 26, 2024
@rustbot
Copy link
Collaborator

rustbot commented Feb 26, 2024

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

@oli-obk
Copy link
Contributor

oli-obk commented Feb 26, 2024

oof. and the mixup is only really noticeable for projections, not just proving that one of the async fn traits holds

@compiler-errors
Copy link
Member Author

💀

let future_trait_def_id = tcx.require_lang_item(LangItem::Future, None);
let placeholder_output_ty = self.infcx.enter_forall_and_leak_universe(sig.output());
Copy link
Contributor

Choose a reason for hiding this comment

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

so, if actually used (via unboxed closures), we'd just get odd errors about traits not being implemented even though they are?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, this change only affects async Fn trait goals, and makes them go from ICE -> PASS.

That issue I linked via unboxed closures still ICEs; I'll put up a fix like #108834 in a separate PR.

@oli-obk
Copy link
Contributor

oli-obk commented Feb 26, 2024

I guess the tests implicitly test that AsyncFnOnce::Output projections now get resolved correctly, but maybe add an explicit test?

r=me either way, and sorry for not catching these in review.

@compiler-errors
Copy link
Member Author

r=me either way, and sorry for not catching these in review.

No, it's definitely 100% my fault for writing extremely incomprehensible code lol

@compiler-errors
Copy link
Member Author

NOTE: Rebased this onto #121656, so I'll just close that one and r=oli-obk here, since you already both approved that one and this one.

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Feb 27, 2024

📌 Commit c8e3f35 has been approved by oli-obk

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 Feb 27, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 29, 2024
…llaumeGomez

Rollup of 10 pull requests

Successful merges:

 - rust-lang#118217 (Document which methods on `f64` are precise)
 - rust-lang#119748 (Increase visibility of `join_path` and `split_paths`)
 - rust-lang#121412 (platform docs: clarify hexagon-unknown-none-elf example, add hexagon-unknown-linux-musl)
 - rust-lang#121654 (Fix `async Fn` confirmation for `FnDef`/`FnPtr`/`Closure` types)
 - rust-lang#121700 (CFI: Don't compress user-defined builtin types)
 - rust-lang#121765 (add platform-specific function to get the error number for HermitOS)
 - rust-lang#121781 (bootstrap/format: send larger batches to rustfmt)
 - rust-lang#121788 (bootstrap: fix clap deprecated warnings)
 - rust-lang#121792 (Improve renaming suggestion when item starts with underscore)
 - rust-lang#121793 (Document which methods on `f32` are precise)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5978b6f into rust-lang:master Feb 29, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Feb 29, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 29, 2024
Rollup merge of rust-lang#121654 - compiler-errors:async-fn-for-fn-def, r=oli-obk

Fix `async Fn` confirmation for `FnDef`/`FnPtr`/`Closure` types

Fixes three issues:
1. The code in `extract_tupled_inputs_and_output_from_async_callable` was accidentally getting the *future* type and the *output* type (returned by the future) messed up for fnptr/fndef/closure types. :/
2. We have a (class of) bug(s) in the old solver where we don't really support higher ranked built-in `Future` goals for generators. This is not possible to hit on stable code, but [can be hit with `unboxed_closures`](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=e935de7181e37e13515ad01720bcb899) (rust-lang#121653).
    * I'm opting not to fix that in this PR. Instead, I just instantiate placeholders when confirming `async Fn` goals.
4. Fixed a bug when generating `FnPtr` shims for `async Fn` trait goals.

r? oli-obk
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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants