Correct inference of primitive operand type behind binary operation

[varkor]

Fixes #57447.

r? @nikomatsakis

[nikomatsakis]

The code seems fine, but I think we probably ought to document the behavior of this part of the type-checker better. This effects the lang spec in the end. Perhaps the reference is the right place? I don't think we have a lot of "framework" for doing so, but I think it's probably a good idea for us to start collecting these sorts of things in the reference.

This is tricky, though, as we don't have any sort of "framework", and it's hard to describe the structure of the rules without that. Still, I wouldn't want to get too hung up on (e.g) figuring out the best way to write typing judgements.

Maybe we should start by documenting in rustc-guide or in an open issue to start?

[varkor]

I've written a comment on a relevant issue in the Rust reference repository.

[varkor]

I find it slightly surprising that x + y and &x + &y work, but &&x + &&y (and similar) do not. This is due to https://github.com/rust-lang/rust/blob/056d48a1bc454dec2186f9242db77af69e493524/src/libcore/internal_macros.rs#L21-L23
which seems a little ad hoc (added in #21227).

[nikomatsakis]

It seems a bit surprising, but then those are the impls that exist, right? (i.e., we don't have a general impl for &T, just for specific types?)

[varkor]

Yes, that's right. It's related, but only tangentially in that the cases look similar from a user's perspective.

[nikomatsakis]

@rfcbot fcp merge

Discussed in today's @rust-lang/lang meeting. The consensus of the meeting was that we should make this change, but we're doing an fcp merge to make it official.

We did think it would be good to add some more tests:

• To cover other sorts of operators, like <<
• To test cases where the integer is "indirect", like this one
• To test cases where there is no expected type (which we expect to error)

[rfcbot]

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

• @Centril
• @cramertj
• @eddyb
• @joshtriplett
• @nikomatsakis
• @pnkfelix
• @scottmcm
• @withoutboats

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.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[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.

The RFC will be merged soon.

[nikomatsakis]

@bors r+

[bors]

📌 Commit 056d48a1bc454dec2186f9242db77af69e493524 has been approved by nikomatsakis

[nikomatsakis]

@bors r-

Wait, @varkor, did you add the extra tests we mentioned in the meeting...?

[varkor]

@nikomatsakis: no, I was waiting for the FCP to complete. I'll do so soon.

[varkor]

To test cases where there is no expected type (which we expect to error)

Just to clarify, do you mean cases like the following?

let _ = 0 + &0;

This currently passes, as it defaults to i32. Or was a different case supposed to error?

I've added the suggested test cases.

[nikomatsakis]

Good question @varkor, what did I mean... I'm not sure, but the tests seem good.

@bors r+

[bors]

📌 Commit 0276d7a has been approved by nikomatsakis

[MightyPork]

I hoped this would fix the ever-present "you need a *" nag, but e.g. this still does not work - is that split to a separate issue? It seems to still be broken for all comparison operators

fn main() {
    let a = 1u16;
    let b = 0u16;
    
    let a_borrowed = &a;
    
    if a_borrowed > b { // expected `&u16`, found `u16`
        println!("hello");
    }
}

[varkor]

This would be a separate issue, but I'm not sure whether or not it's covered by an existing issue.
The reason this doesn't currently work is that we don't callforward_ref_binop https://github.com/rust-lang/rust/blob/a59264b01247836c70e24217e0d346b868387525/src/libcore/internal_macros.rs#L21-L23
for the comparison operators, unlike the arithmetic operators. I'm not sure if this is intentional or not. You're welcome to open an issue: if this discussion already exists, someone else will know about it.