Allow unary operand types to be inferred later - #159744
Conversation
|
r? @fee1-dead rustbot has assigned @fee1-dead. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
hard for me to know if this is the right thing to do, hmm.. r? types |
be7c566 to
d13857f
Compare
| let oprnd_t = match unop { | ||
| hir::UnOp::Deref => self.structurally_resolve_type(expr.span, oprnd_t), | ||
| hir::UnOp::Not | hir::UnOp::Neg => self.resolve_vars_with_obligations(oprnd_t), | ||
| }; |
There was a problem hiding this comment.
This definitely needs a comment. But:
- Can we just use
resolve_vars_with_obligationshere for all three? - If not, I think it makes sense to just move these into the match arms, rather than matching twice.
There was a problem hiding this comment.
If we also use resolve_vars_with_obligations for Deref, this code will treat a later-inferred raw pointer as an overloaded Deref, for example this code was reporting a proper E0282:
fn make<T>() -> T {
loop {}
}
fn main() {
let pointer = make();
//~^ ERROR type annotations needed
let value = unsafe { *pointer };
let _: *const u8 = pointer;
let _: u8 = value;
}error[E0282]: type annotations needed
--> src/main.rs:9:9
|
9 | let pointer = make();
| ^^^^^^^
10 | //~^ ERROR type annotations needed
11 | let value = unsafe { *pointer };
| -------- type must be known at this point
|
help: consider giving `pointer` an explicit type
|
9 | let pointer: /* Type */ = make();
| ++++++++++++
For more information about this error, try `rustc --explain E0282`.if we changed to use resolve_vars_with_obligations, the error changed to:
error[E0277]: the trait bound `*const u8: Deref` is not satisfied
--> tests/ui/inference/unary-deref-late-raw-pointer-inference-issue-106138.rs:11:26
|
11 | let value = unsafe { *pointer };
| ^^^^^^^^ the trait `Deref` is not implemented for `*const u8`
error: aborting due to 1 previous error
For more information about this error, try `rustc --explain E0277`.I'm not sure whether there is other cases, we'd better keep structurally_resolve_type for Deref, and I added this test code as a unit test.
|
This should also close #26830, I think? Also, for what it's worth, this would slightly expand the surface area of #114380 (also reported as #151202). Currently, the following fails to type-check: fn main() {
let input = Default::default();
let output = !{ input };
let _: u8 = output;
println!("{}", std::any::type_name_of_val(&input));
}Under this PR, it would compile successfully and print |
d13857f to
5fd2ec6
Compare
This comment has been minimized.
This comment has been minimized.
5fd2ec6 to
26b55d3
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
I added a test in this PR for #26830 |
|
@rustbot ready |
|
@rfcbot merge types Minor change here. This |
|
@jackh726 has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
|
This is okay because we don't do auto-derefs/coercions here, we don't do this for indexing or field projections as I do feel the argument that At this point this is a lang design tradeoff, cc @rust-lang/lang whether you want to be involved here. An alternative would be to allow |
Co-authored-by: lcnr <rust@lcnr.de>
|
this seems related to #151539 would like for whoever reviews this PR to also review that PR potentially 🤔 |
|
☔ The latest upstream changes (presumably #162229) made this pull request unmergeable. Please resolve the merge conflicts by rebasing. |
in relation to #151539, I had been considering taking advantage of how we don't allow unary operand types to be inferred later in that PR, but I'm not so sure about that approach now. as far as I was able to tell, linting on code that would break from fixing that inference bug (for an FCW or edition migration) either requires a pretty involved change or something pretty hacky. as I recall, for the hacky approach, disallowing unary operand types from being inferred later would make it easier to only lint on code that would fail to compile under the bugfix (...hopefully. I haven't tried going back to it and making it account for some missing cases). but ideally it'd be nice not to do the hacky thing at all |
Fixes #106138
Fix the inference issue by allowing
NotandNegoperand types to remain unresolved until obligations and constraints can determine them.closes #26830