go through uses of `DefineOpaqueTypes::No` and either document or change them #116652

oli-obk · 2023-10-12T08:03:33Z

This issue can be resolved in individual steps (each application of all steps should be their own commit!):

flip a single usage of DefineOpauqeTypes::No to Yes
see what tests fail, document the site by mentioning the test in a comment, revert to ::No, and open a PR.
If no tests fail, still open a PR and assign to @oli-obk or @compiler-errors and leave a comment that there's no test exercising it. We'll try to come up with a test and/or run crater to find examples.

The text was updated successfully, but these errors were encountered:

goldpanth3r · 2023-10-12T12:57:23Z

Hey @oli-obk! I would like to try to contribute to this. Could you explain Step 1:flip a site to Yes? I tried to search for site and DefineOpaqueTypes::No in the codebase, but couldn't learn much.

matthiaskrgr · 2023-10-12T14:25:26Z

I think it means to just search for DefineOpaqueTypes::No for example:

compiler/rustc_trait_selection/src/traits/fulfill.rs:540:                                    .eq(DefineOpaqueTypes::No, a.args, b.args)

and change it to DefineOpaqueTypes::Yes 🙃

chansuke · 2023-10-23T13:31:58Z

@rustbot claim

change method resolution to constrain hidden types instead of rejecting method candidates Some of these are in probes and may affect inference. This is therefore a breaking change. This allows new code to compile on stable: ```rust trait Trait {} impl Trait for u32 {} struct Bar<T>(T); impl Bar<u32> { fn foo(self) {} } fn foo(x: bool) -> Bar<impl Sized> { if x { let x = foo(false); x.foo(); //^ this used to not find the `foo` method, because while we did equate `x`'s type with possible candidates, we didn't allow opaque type inference while doing so } todo!() } ``` But it is also a breaking change, since `&self` and `&mut self` method calls on recursive RPIT function calls now constrain the hidden type to `&_` and `&mut _` respectively. This is not what users really expect or want from this, but there's way around this. r? `@compiler-errors` fixes rust-lang#121404 cc rust-lang#116652

More DefineOpaqueTypes::Yes This accepts more code on stable. It is now possible to have match arms return a function item `foo::<ConcreteType>` and a function item `foo::<OpaqueTypeInDefiningScope>` in another, and that will constrain `OpaqueTypeInDefiningScope` to have the hidden type `ConcreteType`. So the following function will now compile, but on master it errors with a type mismatch on the second match arm ```rust // The function item whose generic params we want to merge. fn foo<T>(t: T) -> T { t } // Helper ensuring we can constrain `T` on `F` without explicitly specifying it fn bind<T, F: FnOnce(T) -> T>(_: T, f: F) -> F { f } fn k() -> impl Sized { let x = match true { true => { // `f` is `FnDef(foo, [infer_var])` let f = foo; // Get a value of an opaque type on stable let t = k(); // this returns `FnDef(foo, [k::return])` bind(t, f) } false => foo::<()>, }; todo!() } ``` r? `@compiler-errors` cc rust-lang#116652

More DefineOpaqueTypes::Yes This accepts more code on stable. It is now possible to have match arms return a function item `foo::<ConcreteType>` and a function item `foo::<OpaqueTypeInDefiningScope>` in another, and that will constrain `OpaqueTypeInDefiningScope` to have the hidden type `ConcreteType`. So the following function will now compile, but on master it errors with a type mismatch on the second match arm ```rust // The function item whose generic params we want to merge. fn foo<T>(t: T) -> T { t } // Helper ensuring we can constrain `T` on `F` without explicitly specifying it fn bind<T, F: FnOnce(T) -> T>(_: T, f: F) -> F { f } fn k() -> impl Sized { let x = match true { true => { // `f` is `FnDef(foo, [infer_var])` let f = foo; // Get a value of an opaque type on stable let t = k(); // this returns `FnDef(foo, [k::return])` bind(t, f) } false => foo::<()>, }; todo!() } ``` r? ``@compiler-errors`` cc rust-lang#116652

Rollup merge of rust-lang#123794 - oli-obk:define_opaque_types2, r=lcnr More DefineOpaqueTypes::Yes This accepts more code on stable. It is now possible to have match arms return a function item `foo::<ConcreteType>` and a function item `foo::<OpaqueTypeInDefiningScope>` in another, and that will constrain `OpaqueTypeInDefiningScope` to have the hidden type `ConcreteType`. So the following function will now compile, but on master it errors with a type mismatch on the second match arm ```rust // The function item whose generic params we want to merge. fn foo<T>(t: T) -> T { t } // Helper ensuring we can constrain `T` on `F` without explicitly specifying it fn bind<T, F: FnOnce(T) -> T>(_: T, f: F) -> F { f } fn k() -> impl Sized { let x = match true { true => { // `f` is `FnDef(foo, [infer_var])` let f = foo; // Get a value of an opaque type on stable let t = k(); // this returns `FnDef(foo, [k::return])` bind(t, f) } false => foo::<()>, }; todo!() } ``` r? ``@compiler-errors`` cc rust-lang#116652

More DefineOpaqueTypes::Yes This accepts more code on stable. It is now possible to have match arms return a function item `foo::<ConcreteType>` and a function item `foo::<OpaqueTypeInDefiningScope>` in another, and that will constrain `OpaqueTypeInDefiningScope` to have the hidden type `ConcreteType`. So the following function will now compile, but on master it errors with a type mismatch on the second match arm ```rust // The function item whose generic params we want to merge. fn foo<T>(t: T) -> T { t } // Helper ensuring we can constrain `T` on `F` without explicitly specifying it fn bind<T, F: FnOnce(T) -> T>(_: T, f: F) -> F { f } fn k() -> impl Sized { let x = match true { true => { // `f` is `FnDef(foo, [infer_var])` let f = foo; // Get a value of an opaque type on stable let t = k(); // this returns `FnDef(foo, [k::return])` bind(t, f) } false => foo::<()>, }; todo!() } ``` r? ``@compiler-errors`` cc rust-lang/rust#116652

…ackh726 Allow coercing functions whose signature differs in opaque types in their defining scope into a shared function pointer type r? `@compiler-errors` This accepts more code on stable. It is now possible to have match arms return a function item `foo` and a different function item `bar` in another, and that will constrain OpaqueTypeInDefiningScope to have the hidden type ConcreteType and make the type of the match arms a function pointer that matches the signature. So the following function will now compile, but on master it errors with a type mismatch on the second match arm ```rust fn foo<T>(t: T) -> T { t } fn bar<T>(t: T) -> T { t } fn k() -> impl Sized { fn bind<T, F: FnOnce(T) -> T>(_: T, f: F) -> F { f } let x = match true { true => { let f = foo; bind(k(), f) } false => bar::<()>, }; todo!() } ``` cc rust-lang#116652 This is very similar to rust-lang#123794, and with the same rationale: > this is for consistency with `-Znext-solver`. the new solver does not have the concept of "non-defining use of opaque" right now and we would like to ideally keep it that way. Moving to `DefineOpaqueTypes::Yes` in more cases removes subtlety from the type system. Right now we have to be careful when relating `Opaque` with another type as the behavior changes depending on whether we later use the `Opaque` or its hidden type directly (even though they are equal), if that later use is with `DefineOpaqueTypes::No`*

Rollup merge of rust-lang#124297 - oli-obk:define_opaque_types13, r=jackh726 Allow coercing functions whose signature differs in opaque types in their defining scope into a shared function pointer type r? `@compiler-errors` This accepts more code on stable. It is now possible to have match arms return a function item `foo` and a different function item `bar` in another, and that will constrain OpaqueTypeInDefiningScope to have the hidden type ConcreteType and make the type of the match arms a function pointer that matches the signature. So the following function will now compile, but on master it errors with a type mismatch on the second match arm ```rust fn foo<T>(t: T) -> T { t } fn bar<T>(t: T) -> T { t } fn k() -> impl Sized { fn bind<T, F: FnOnce(T) -> T>(_: T, f: F) -> F { f } let x = match true { true => { let f = foo; bind(k(), f) } false => bar::<()>, }; todo!() } ``` cc rust-lang#116652 This is very similar to rust-lang#123794, and with the same rationale: > this is for consistency with `-Znext-solver`. the new solver does not have the concept of "non-defining use of opaque" right now and we would like to ideally keep it that way. Moving to `DefineOpaqueTypes::Yes` in more cases removes subtlety from the type system. Right now we have to be careful when relating `Opaque` with another type as the behavior changes depending on whether we later use the `Opaque` or its hidden type directly (even though they are equal), if that later use is with `DefineOpaqueTypes::No`*

Allow coercing functions whose signature differs in opaque types in their defining scope into a shared function pointer type r? `@compiler-errors` This accepts more code on stable. It is now possible to have match arms return a function item `foo` and a different function item `bar` in another, and that will constrain OpaqueTypeInDefiningScope to have the hidden type ConcreteType and make the type of the match arms a function pointer that matches the signature. So the following function will now compile, but on master it errors with a type mismatch on the second match arm ```rust fn foo<T>(t: T) -> T { t } fn bar<T>(t: T) -> T { t } fn k() -> impl Sized { fn bind<T, F: FnOnce(T) -> T>(_: T, f: F) -> F { f } let x = match true { true => { let f = foo; bind(k(), f) } false => bar::<()>, }; todo!() } ``` cc rust-lang/rust#116652 This is very similar to rust-lang/rust#123794, and with the same rationale: > this is for consistency with `-Znext-solver`. the new solver does not have the concept of "non-defining use of opaque" right now and we would like to ideally keep it that way. Moving to `DefineOpaqueTypes::Yes` in more cases removes subtlety from the type system. Right now we have to be careful when relating `Opaque` with another type as the behavior changes depending on whether we later use the `Opaque` or its hidden type directly (even though they are equal), if that later use is with `DefineOpaqueTypes::No`*

…ompiler-errors Some unstable changes to where opaque types get defined None of these can be reached from stable afaict. r? `@compiler-errors` cc rust-lang#116652

…ompiler-errors Some unstable changes to where opaque types get defined None of these can be reached from stable afaict. r? ``@compiler-errors`` cc rust-lang#116652

Rollup merge of rust-lang#124080 - oli-obk:define_opaque_types10, r=compiler-errors Some unstable changes to where opaque types get defined None of these can be reached from stable afaict. r? ``@compiler-errors`` cc rust-lang#116652

change method resolution to constrain hidden types instead of rejecting method candidates Some of these are in probes and may affect inference. This is therefore a breaking change. This allows new code to compile on stable: ```rust trait Trait {} impl Trait for u32 {} struct Bar<T>(T); impl Bar<u32> { fn foo(self) {} } fn foo(x: bool) -> Bar<impl Sized> { if x { let x = foo(false); x.foo(); //^ this used to not find the `foo` method, because while we did equate `x`'s type with possible candidates, we didn't allow opaque type inference while doing so } todo!() } ``` r? `@compiler-errors` fixes rust-lang#121404 cc rust-lang#116652

change method resolution to constrain hidden types instead of rejecting method candidates Some of these are in probes and may affect inference. This is therefore a breaking change. This allows new code to compile on stable: ```rust trait Trait {} impl Trait for u32 {} struct Bar<T>(T); impl Bar<u32> { fn foo(self) {} } fn foo(x: bool) -> Bar<impl Sized> { if x { let x = foo(false); x.foo(); //^ this used to not find the `foo` method, because while we did equate `x`'s type with possible candidates, we didn't allow opaque type inference while doing so } todo!() } ``` r? ``@compiler-errors`` fixes rust-lang#121404 cc rust-lang#116652

change method resolution to constrain hidden types instead of rejecting method candidates Some of these are in probes and may affect inference. This is therefore a breaking change. This allows new code to compile on stable: ```rust trait Trait {} impl Trait for u32 {} struct Bar<T>(T); impl Bar<u32> { fn foo(self) {} } fn foo(x: bool) -> Bar<impl Sized> { if x { let x = foo(false); x.foo(); //^ this used to not find the `foo` method, because while we did equate `x`'s type with possible candidates, we didn't allow opaque type inference while doing so } todo!() } ``` r? ```@compiler-errors``` fixes rust-lang#121404 cc rust-lang#116652

change method resolution to constrain hidden types instead of rejecting method candidates Some of these are in probes and may affect inference. This is therefore a breaking change. This allows new code to compile on stable: ```rust trait Trait {} impl Trait for u32 {} struct Bar<T>(T); impl Bar<u32> { fn foo(self) {} } fn foo(x: bool) -> Bar<impl Sized> { if x { let x = foo(false); x.foo(); //^ this used to not find the `foo` method, because while we did equate `x`'s type with possible candidates, we didn't allow opaque type inference while doing so } todo!() } ``` r? ````@compiler-errors```` fixes rust-lang#121404 cc rust-lang#116652

change method resolution to constrain hidden types instead of rejecting method candidates Some of these are in probes and may affect inference. This is therefore a breaking change. This allows new code to compile on stable: ```rust trait Trait {} impl Trait for u32 {} struct Bar<T>(T); impl Bar<u32> { fn foo(self) {} } fn foo(x: bool) -> Bar<impl Sized> { if x { let x = foo(false); x.foo(); //^ this used to not find the `foo` method, because while we did equate `x`'s type with possible candidates, we didn't allow opaque type inference while doing so } todo!() } ``` r? `````@compiler-errors````` fixes rust-lang#121404 cc rust-lang#116652

change method resolution to constrain hidden types instead of rejecting method candidates Some of these are in probes and may affect inference. This is therefore a breaking change. This allows new code to compile on stable: ```rust trait Trait {} impl Trait for u32 {} struct Bar<T>(T); impl Bar<u32> { fn foo(self) {} } fn foo(x: bool) -> Bar<impl Sized> { if x { let x = foo(false); x.foo(); //^ this used to not find the `foo` method, because while we did equate `x`'s type with possible candidates, we didn't allow opaque type inference while doing so } todo!() } ``` r? ``````@compiler-errors`````` fixes rust-lang#121404 cc rust-lang#116652

change method resolution to constrain hidden types instead of rejecting method candidates Some of these are in probes and may affect inference. This is therefore a breaking change. This allows new code to compile on stable: ```rust trait Trait {} impl Trait for u32 {} struct Bar<T>(T); impl Bar<u32> { fn foo(self) {} } fn foo(x: bool) -> Bar<impl Sized> { if x { let x = foo(false); x.foo(); //^ this used to not find the `foo` method, because while we did equate `x`'s type with possible candidates, we didn't allow opaque type inference while doing so } todo!() } ``` r? ```````@compiler-errors``````` fixes rust-lang#121404 cc rust-lang#116652

Rollup merge of rust-lang#123962 - oli-obk:define_opaque_types5, r=lcnr change method resolution to constrain hidden types instead of rejecting method candidates Some of these are in probes and may affect inference. This is therefore a breaking change. This allows new code to compile on stable: ```rust trait Trait {} impl Trait for u32 {} struct Bar<T>(T); impl Bar<u32> { fn foo(self) {} } fn foo(x: bool) -> Bar<impl Sized> { if x { let x = foo(false); x.foo(); //^ this used to not find the `foo` method, because while we did equate `x`'s type with possible candidates, we didn't allow opaque type inference while doing so } todo!() } ``` r? ```````@compiler-errors``````` fixes rust-lang#121404 cc rust-lang#116652

…h726 Change a `DefineOpaqueTypes::No` to `Yes` in diagnostics code Explanation in comments of the function. r? ``@compiler-errors`` cc rust-lang#116652

…h726 Change a `DefineOpaqueTypes::No` to `Yes` in diagnostics code Explanation in comments of the function. r? ```@compiler-errors``` cc rust-lang#116652

Rollup merge of rust-lang#126675 - oli-obk:diagnostics_opaque, r=jackh726 Change a `DefineOpaqueTypes::No` to `Yes` in diagnostics code Explanation in comments of the function. r? ```@compiler-errors``` cc rust-lang#116652

…, r=compiler-errors Ensure we don't accidentally succeed when we want to report an error This also changes the `DefiningOpaqueTypes::No` to `Yes` without adding tests, as it is solely run on the error path to improve diagnostics. I was unable to provide a test that changes diagnostics, as all the tests I came up with ended up successfully constraining the opaque type and thus succeeding the coercion. r? `@compiler-errors` cc rust-lang#116652

…, r=compiler-errors Ensure we don't accidentally succeed when we want to report an error This also changes the `DefiningOpaqueTypes::No` to `Yes` without adding tests, as it is solely run on the error path to improve diagnostics. I was unable to provide a test that changes diagnostics, as all the tests I came up with ended up successfully constraining the opaque type and thus succeeding the coercion. r? ``@compiler-errors`` cc rust-lang#116652

…, r=compiler-errors Ensure we don't accidentally succeed when we want to report an error This also changes the `DefiningOpaqueTypes::No` to `Yes` without adding tests, as it is solely run on the error path to improve diagnostics. I was unable to provide a test that changes diagnostics, as all the tests I came up with ended up successfully constraining the opaque type and thus succeeding the coercion. r? ```@compiler-errors``` cc rust-lang#116652

Rollup merge of rust-lang#126673 - oli-obk:dont_rely_on_err_reporting, r=compiler-errors Ensure we don't accidentally succeed when we want to report an error This also changes the `DefiningOpaqueTypes::No` to `Yes` without adding tests, as it is solely run on the error path to improve diagnostics. I was unable to provide a test that changes diagnostics, as all the tests I came up with ended up successfully constraining the opaque type and thus succeeding the coercion. r? ```@compiler-errors``` cc rust-lang#116652

…piler-errors Allow constraining opaque types during various unsizing casts allows unsizing of tuples, arrays and Adts to constraint opaque types in their generic parameters to concrete types on either side of the unsizing cast. Also allows constraining opaque types during trait object casts that only differ in auto traits or lifetimes. cc rust-lang#116652

…rrors Some unstable changes to where opaque types get defined None of these can be reached from stable afaict. r? ``@compiler-errors`` cc rust-lang/rust#116652

oli-obk added this to type alias impl trait stabilization Oct 12, 2023

oli-obk converted this from a draft issue Oct 12, 2023

rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 12, 2023

rustbot assigned chansuke Oct 23, 2023

chansuke mentioned this issue Oct 29, 2023

Replace DefineOpaqueTypes::No => DefineOpaqueTypes::Yes #117348

Closed

lqd mentioned this issue Oct 29, 2023

Mark DefineOpaqueTypes::Yes use-sites uncovered by tests as FIXMEs #117369

Closed

oli-obk assigned oli-obk and unassigned chansuke Feb 21, 2024

oli-obk removed the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Feb 21, 2024

oli-obk mentioned this issue Apr 15, 2024

change method resolution to constrain hidden types instead of rejecting method candidates #123962

Merged

traviscross mentioned this issue Apr 15, 2024

Stabilize associated type position impl Trait (ATPIT) #120700

Open

3 tasks

This was referenced Apr 17, 2024

More DefineOpaqueTypes::Yes #123794

Merged

Some unstable changes to where opaque types get defined #124080

Merged

oli-obk mentioned this issue Apr 23, 2024

Allow coercing functions whose signature differs in opaque types in their defining scope into a shared function pointer type #124297

Merged

traviscross mentioned this issue May 28, 2024

Allow constraining opaque types during various unsizing casts #125610

Merged

oli-obk moved this from Todo to In Progress in type alias impl trait stabilization Jun 12, 2024

This was referenced Jun 19, 2024

Change a DefineOpaqueTypes::No to Yes in diagnostics code #126675

Merged

Ensure we don't accidentally succeed when we want to report an error #126673

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

go through uses of `DefineOpaqueTypes::No` and either document or change them #116652

go through uses of `DefineOpaqueTypes::No` and either document or change them #116652

oli-obk commented Oct 12, 2023 •

edited by rustbot

Loading

goldpanth3r commented Oct 12, 2023

matthiaskrgr commented Oct 12, 2023

chansuke commented Oct 23, 2023

go through uses of DefineOpaqueTypes::No and either document or change them #116652

go through uses of DefineOpaqueTypes::No and either document or change them #116652

Comments

oli-obk commented Oct 12, 2023 • edited by rustbot Loading

goldpanth3r commented Oct 12, 2023

matthiaskrgr commented Oct 12, 2023

chansuke commented Oct 23, 2023

go through uses of `DefineOpaqueTypes::No` and either document or change them #116652

go through uses of `DefineOpaqueTypes::No` and either document or change them #116652

oli-obk commented Oct 12, 2023 •

edited by rustbot

Loading