use implied bounds when checking opaque types #106038

aliemjay · 2022-12-22T11:44:18Z

During opaque type inference, we check for the well-formedness of the hidden type in the opaque type's own environment, not the one of the defining site, which are different in the case of TAIT.

However in the case of associated-type-impl-trait, we don't use implied bounds from the impl header. This caused us to reject the following:

trait Service<Req> {
    type Output;
    fn call(req: Req) -> Self::Output;
}

impl<'a, Req> Service<&'a Req> for u8 {
    type Output= impl Sized; // we can't prove WF of hidden type  `WF(&'a Req)` although it's implied by the impl
    //~^ ERROR type parameter Req doesn't live long enough
    fn call(req: &'a Req) -> Self::Output {
        req
    }
}

although adding an explicit bound would make it pass:

- impl<'a, Req> Service<&'a Req> for u8 {
+ impl<'a, Req> Service<&'a Req> for u8  where Req: 'a, {

I believe it should pass as we already allow the concrete type to be used:

impl<'a, Req> Service<&'a Req> for u8 {
-    type Output= impl Sized;
+    type Output= &'a Req;

Fixes #95922

Builds on #105982

cc @lcnr (because implied bounds)

r? @oli-obk

rustbot · 2022-12-22T11:44:28Z

Some changes occurred in engine.rs, potentially modifying the public API of ObligationCtxt.

cc @lcnr

bors · 2022-12-24T05:55:13Z

☔ The latest upstream changes (presumably #106103) made this pull request unmergeable. Please resolve the merge conflicts.

oli-obk · 2023-01-10T08:40:13Z

the TAIT part lgtm. The implied bounds part, too, but I don't know that part well enough.

@lcnr do you want to have a look?

@aliemjay please rebase and rebless

@rustbot author

lcnr

2 nits, otherwise r=me on the second commit.

blocked on #105982

lcnr · 2023-01-13T12:11:59Z

src/test/ui/type-alias-impl-trait/wf-nested-pass.rs

+}
+
+type OuterOpaque<T> = impl Trait<&'static T, Out = impl Sized>;
+fn define<T>() -> OuterOpaque<T> {}


can you add a test which checks that this errors when define is used with a type which is not 'static

I've added a one in pass_sound revision. Note that it relies on #99217 for soundness.

lcnr · 2023-01-13T12:12:40Z

compiler/rustc_hir_analysis/src/check/check.rs

+        // We don't have to check them here because their well-formedness follows from the WF of
+        // the projection input types in the defining- and use-sites.
+        hir::OpaqueTyOrigin::TyAlias
+            if let DefKind::OpaqueTy = tcx.def_kind(tcx.parent(def_id.to_def_id())) => {}


that makes me a bit unhappy. Can you not instead add the type of the parent opaque type as an assumed wf type?

We should not only include the type of the parent opaque type in assumed_wf_types, but also all other types that appear in the trait inputs (e.g &'static T in impl Trait<&'static T, Assoc=impl Nested>). The implementation can get easily complicated if we consider multiple levels of nesting and binders like:

impl Fn<(&'static T,), Output = impl for<'a> Fn<(&'a T,), Output = impl Sized + 'a>>

I think the current implementation strikes a balance between simplicity and completeness. That being said, if you feel strongly about it, I'm happy to implement the limited version of including only the parent opaque type. Note that we would reject the test wf-nested.rs.

bors · 2023-04-13T12:22:11Z

☔ The latest upstream changes (presumably #110275) made this pull request unmergeable. Please resolve the merge conflicts.

aliemjay · 2023-05-06T22:46:45Z

no longer blocked.
@rustbot ready

oli-obk · 2023-05-11T09:31:30Z

@bors r=lcnr

bors · 2023-05-11T09:31:32Z

📌 Commit d548747 has been approved by lcnr

It is now in the queue for this repository.

use implied bounds when checking opaque types During opaque type inference, we check for the well-formedness of the hidden type in the opaque type's own environment, not the one of the defining site, which are different in the case of TAIT. However in the case of associated-type-impl-trait, we don't use implied bounds from the impl header. This caused us to reject the following: ```rust trait Service<Req> { type Output; fn call(req: Req) -> Self::Output; } impl<'a, Req> Service<&'a Req> for u8 { type Output= impl Sized; // we can't prove WF of hidden type `WF(&'a Req)` although it's implied by the impl //~^ ERROR type parameter Req doesn't live long enough fn call(req: &'a Req) -> Self::Output { req } } ``` although adding an explicit bound would make it pass: ```diff - impl<'a, Req> Service<&'a Req> for u8 { + impl<'a, Req> Service<&'a Req> for u8 where Req: 'a, { ``` I believe it should pass as we already allow the concrete type to be used: ```diff impl<'a, Req> Service<&'a Req> for u8 { - type Output= impl Sized; + type Output= &'a Req; ``` Fixes rust-lang#95922 Builds on rust-lang#105982 cc `@lcnr` (because implied bounds) r? `@oli-obk`

…mpiler-errors Rollup of 7 pull requests Successful merges: - rust-lang#106038 (use implied bounds when checking opaque types) - rust-lang#111366 (Make `NonUseContext::AscribeUserTy` carry `ty::Variance`) - rust-lang#111375 (CFI: Fix SIGILL reached via trait objects) - rust-lang#111439 (Fix backtrace normalization in ice-bug-report-url.rs) - rust-lang#111444 (Only warn single-use lifetime when the binders match.) - rust-lang#111459 (Update browser-ui-test version to 0.16.0) - rust-lang#111460 (Improve suggestion for `self: Box<self>`) Failed merges: - rust-lang#110454 (Require impl Trait in associated types to appear in method signatures) r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned oli-obk Dec 22, 2022

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 Dec 22, 2022

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 Jan 10, 2023

lcnr reviewed Jan 13, 2023

View reviewed changes

lcnr added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 13, 2023

aliemjay force-pushed the opaque-implied branch from dfcf78d to 4e0b8d5 Compare March 11, 2023 11:41

use implied bounds when checking opaque types

d548747

aliemjay force-pushed the opaque-implied branch from 4e0b8d5 to d548747 Compare May 6, 2023 22:43

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels May 6, 2023

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 May 11, 2023

Dylan-DPC mentioned this pull request May 11, 2023

Rollup of 6 pull requests #111464

Closed

compiler-errors mentioned this pull request May 12, 2023

Rollup of 7 pull requests #111489

Merged

bors merged commit 341d6df into rust-lang:master May 12, 2023

rustbot added this to the 1.71.0 milestone May 12, 2023

aliemjay deleted the opaque-implied branch August 11, 2023 14:42

lcnr mentioned this pull request Feb 27, 2024

stricter hidden type wf-check [based on #115008] #121679

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use implied bounds when checking opaque types #106038

use implied bounds when checking opaque types #106038

aliemjay commented Dec 22, 2022 •

edited

Loading

rustbot commented Dec 22, 2022

bors commented Dec 24, 2022

oli-obk commented Jan 10, 2023

lcnr left a comment

lcnr Jan 13, 2023

aliemjay Mar 11, 2023

lcnr Jan 13, 2023

aliemjay Mar 11, 2023

bors commented Apr 13, 2023

aliemjay commented May 6, 2023

oli-obk commented May 11, 2023

bors commented May 11, 2023

use implied bounds when checking opaque types #106038

use implied bounds when checking opaque types #106038

Conversation

aliemjay commented Dec 22, 2022 • edited Loading

rustbot commented Dec 22, 2022

bors commented Dec 24, 2022

oli-obk commented Jan 10, 2023

lcnr left a comment

Choose a reason for hiding this comment

lcnr Jan 13, 2023

Choose a reason for hiding this comment

aliemjay Mar 11, 2023

Choose a reason for hiding this comment

lcnr Jan 13, 2023

Choose a reason for hiding this comment

aliemjay Mar 11, 2023

Choose a reason for hiding this comment

bors commented Apr 13, 2023

aliemjay commented May 6, 2023

oli-obk commented May 11, 2023

bors commented May 11, 2023

aliemjay commented Dec 22, 2022 •

edited

Loading