Allow unary operand types to be inferred later

[chenyukang]

View all comments

Fixes #106138

Fix the inference issue by allowing Not and Neg operand types to remain unresolved until obligations and constraints can determine them.

closes #26830

[rustbot]

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 74 candidates
• Random selection from 20 candidates

[fee1-dead]

hard for me to know if this is the right thing to do, hmm..

r? types

[jackh726]

So, this is fairly minor, but is new behavior, so I wondering if a @rust-lang/types FCP might be appropriate.

View changes since this review

[jackh726]

This definitely needs a comment. But:

1. Can we just use resolve_vars_with_obligations here for all three?
2. If not, I think it makes sense to just move these into the match arms, rather than matching twice.

[chenyukang]

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.

[dianne]

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 u8. let output = !{ input }; erroneously coerces input to the expected type of the negation as a whole, unifying its type with that of output (which is now allowed to be inferred later).

[chenyukang]

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 u8. let output = !{ input }; erroneously coerces input to the expected type of the negation as a whole, unifying its type with that of output (which is now allowed to be inferred later).

I added a test in this PR for #26830

[chenyukang]

@rustbot ready

[jackh726]

@rfcbot merge types

Minor change here. This Not and Neg unary operators use resolve_vars_with_obligations instead of structurally_resolve_type, which lets us infer the output from later information.

[rust-rfcbot]

@jackh726 has proposed to merge this. The next step is review by the rest of the tagged team members:

• @BoxyUwU
• @jackh726
• @lcnr
• @oli-obk
• @spastorino

Concerns:

• lang-decision resolved by Allow unary operand types to be inferred later #159744 (comment)

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.

[rust-rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

[jackh726]

It is probably good to have a test for behavior that we're locking into re. #159744 (comment)

Other than that, the changes themselves LGTM

View changes since this review

[rustbot]

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.

[chenyukang]

It is probably good to have a test for behavior that we're locking into re. (earlier comment)

Other than that, the changes themselves LGTM

View changes since this review

I added more test to cover it.

[jackh726]

@bors r+

[rust-bors]

📌 Commit a97890a has been approved by jackh726

It is now in the queue for this repository.