-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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 'type annotations needed' error with opaque types #66431
Conversation
r? @davidtwco (rust_highfive has picked a reviewer for you, use r? to override) |
r? @varkor |
☔ The latest upstream changes (presumably #66449) made this pull request unmergeable. Please resolve the merge conflicts. |
} | ||
|
||
fn _produce() -> Wrapper<Foo> { | ||
Wrapper::Second |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this entirely correct. What is the final type of Foo
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Final type is bool
as determined by constrained_foo
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nits
507e3c0
to
8daaa68
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy with the implementation once the nits have been addressed and rebased into the main commits.
Related: rust-lang#66426 This commit adds handling for opaque types during inference variable fallback. Type variables generated from the instantiatino of opaque types now fallback to the opque type itself. Normally, the type variable for an instantiated opaque type is either unified with the concrete type, or with the opaque type itself (e.g when a function returns an opaque type by calling another function). However, it's possible for the type variable to be left completely unconstrained. This can occur in code like this: ```rust pub type Foo = impl Copy; fn produce() -> Option<Foo> { None } ``` Here, we'll instantatiate the `Foo` in `Option<Foo>` to a fresh type variable, but we will never unify it with anything due to the fact that we return a `None`. This results in the error message: `type annotations needed: cannot resolve `_: std::marker::Copy`` pointing at `pub type Foo = impl Copy`. This message is not only confusing, it's incorrect. When an opaque type inference variable is completely unconstrained, we can always fall back to using the opaque type itself. This effectively turns that particular use of the opaque type into a non-defining use, even if it appears in a defining scope.
42ed8b8
to
a11abe0
Compare
@varkor: Squashed |
@bors r+ |
📌 Commit a11abe0 has been approved by |
…varkor Fix 'type annotations needed' error with opaque types Related: rust-lang#66426 This commit adds handling for opaque types during inference variable fallback. Type variables generated from the instantiation of opaque types now fallback to the opaque type itself. Normally, the type variable for an instantiated opaque type is either unified with the concrete type, or with the opaque type itself (e.g when a function returns an opaque type by calling another function). However, it's possible for the type variable to be left completely unconstrained. This can occur in code like this: ```rust pub type Foo = impl Copy; fn produce() -> Option<Foo> { None } ``` Here, we'll instantatiate the `Foo` in `Option<Foo>` to a fresh type variable, but we will never unify it with anything due to the fact that we return a `None`. This results in the error message: ``` type annotations needed: cannot resolve `_: std::marker::Copy ``` pointing at `pub type Foo = impl Copy`. This message is not only confusing, it's incorrect. When an opaque type inference variable is completely unconstrained, we can always fall back to using the opaque type itself. This effectively turns that particular use of the opaque type into a non-defining use, even if it appears in a defining scope.
Rollup of 13 pull requests Successful merges: - #66090 (Misc CI improvements) - #66239 (Suggest calling async closure when needed) - #66430 ([doc] Fix the source code highlighting on source comments) - #66431 (Fix 'type annotations needed' error with opaque types) - #66461 (Add explanation message for E0641) - #66468 (Cleanup Miri SIMD intrinsics) - #66478 (rustc_plugin: Remove the compatibility shim) - #66493 (Add JohnTitor to rustc-guide toolstate notification list) - #66511 (std::error::Chain: remove Copy) - #66512 (Add unix::process::CommandExt::arg0) - #66520 (Disable gdb pretty printer global section on wasm targets) - #66529 (resolve: Give derive helpers highest priority during resolution) - #66536 (Move the definition of `QueryResult` into `plumbing.rs`.) Failed merges: r? @ghost
…varkor Fix 'type annotations needed' error with opaque types Related: rust-lang#66426 This commit adds handling for opaque types during inference variable fallback. Type variables generated from the instantiation of opaque types now fallback to the opaque type itself. Normally, the type variable for an instantiated opaque type is either unified with the concrete type, or with the opaque type itself (e.g when a function returns an opaque type by calling another function). However, it's possible for the type variable to be left completely unconstrained. This can occur in code like this: ```rust pub type Foo = impl Copy; fn produce() -> Option<Foo> { None } ``` Here, we'll instantatiate the `Foo` in `Option<Foo>` to a fresh type variable, but we will never unify it with anything due to the fact that we return a `None`. This results in the error message: ``` type annotations needed: cannot resolve `_: std::marker::Copy ``` pointing at `pub type Foo = impl Copy`. This message is not only confusing, it's incorrect. When an opaque type inference variable is completely unconstrained, we can always fall back to using the opaque type itself. This effectively turns that particular use of the opaque type into a non-defining use, even if it appears in a defining scope.
Rollup of 11 pull requests Successful merges: - #66090 (Misc CI improvements) - #66155 (Add long error explanation for E0594) - #66239 (Suggest calling async closure when needed) - #66430 ([doc] Fix the source code highlighting on source comments) - #66431 (Fix 'type annotations needed' error with opaque types) - #66461 (Add explanation message for E0641) - #66493 (Add JohnTitor to rustc-guide toolstate notification list) - #66511 (std::error::Chain: remove Copy) - #66529 (resolve: Give derive helpers highest priority during resolution) - #66536 (Move the definition of `QueryResult` into `plumbing.rs`.) - #66538 (Remove compiler_builtins_lib feature from libstd) Failed merges: r? @ghost
☔ The latest upstream changes (presumably #66545) made this pull request unmergeable. Please resolve the merge conflicts. |
Related: #66426
This commit adds handling for opaque types during inference variable
fallback. Type variables generated from the instantiation of opaque
types now fallback to the opaque type itself.
Normally, the type variable for an instantiated opaque type is either
unified with the concrete type, or with the opaque type itself (e.g when
a function returns an opaque type by calling another function).
However, it's possible for the type variable to be left completely
unconstrained. This can occur in code like this:
Here, we'll instantatiate the
Foo
inOption<Foo>
to a fresh typevariable, but we will never unify it with anything due to the fact
that we return a
None
.This results in the error message:
pointing at
pub type Foo = impl Copy
.This message is not only confusing, it's incorrect. When an opaque type
inference variable is completely unconstrained, we can always fall back
to using the opaque type itself. This effectively turns that particular
use of the opaque type into a non-defining use, even if it appears in a
defining scope.