fix: use ExprIsRead::Yes for rhs of binary operators#21654
fix: use ExprIsRead::Yes for rhs of binary operators#21654ChayimFriedman2 merged 5 commits intorust-lang:masterfrom
ExprIsRead::Yes for rhs of binary operators#21654Conversation
rustbot
added
the
S-waiting-on-review
Albab-Hasan
force-pushed
the
fix/binop-rhs-never-coercion
branch
from
d91ef9c to
0bed4fb
Compare
both operands of binary operators (including compound assignments like `+=`) were inferred with `ExprIsRead::No`, which prevented divergence detection for place expressions of type `!`. this caused false type mismatches when a function's return type depended on the block diverging after something like `x += *never_ptr` or `let _ = *never_ptr + 1`. both operands of any binary operator are always consumed (read), so this uses `ExprIsRead::Yes` to be consistent with the ordinary assignment and let binding paths.
Albab-Hasan
force-pushed
the
fix/binop-rhs-never-coercion
branch
from
0bed4fb to
b3feb5f
Compare
| } | ||
|
|
||
| #[test] | ||
| fn binop_lhs_never_place_diverges() { |
There was a problem hiding this comment.
This... does not check what it declares?
There was a problem hiding this comment.
my bad. one sec, ill fix it
There was a problem hiding this comment.
Did you fix it?
yes
There was a problem hiding this comment.
How? It still does not test coercion in the LHS.
The other comment was my mistake.
There was a problem hiding this comment.
sorry i just switched to check_infer_with_mismatches thinking that showingthe inferred types was enough.
There was a problem hiding this comment.
It seems there is a misunderstanding. The LHS in the assignment a = b is a. Asserting that a never coercion happens in the LHS means something like *p += 1 where *p is *const !.
There was a problem hiding this comment.
This test is still not correct as there is no assignment now.
Albab-Hasan
force-pushed
the
fix/binop-rhs-never-coercion
branch
from
8006f04 to
2f12839
Compare
|
@ChayimFriedman2 hey, so im really confused rn.
I get this. i pushed the fix. but im not sure what ur trying to say in the first comment. the changes on lines 181 and 183 are in the _ => branch thats regular binops like + not assignments. both operands of + are consumed by |
|
@ChayimFriedman2 can i get some instructions or should i close this pr and work on one liners again? |
|
@ChayimFriedman2 sorry for wasting your time |
|
The change is good, only the test is problematic. A good test will be something like (not verified): let a: *const ! = 0 as _;
*a += 1; |
|
@ChayimFriedman2 ok ill try to fix the test |
|
@ChayimFriedman2 can you review the changes? the test is fixed |
|
@ShoyuVanilla hey can you check this one out too, i want to know if im missing anything? |
There was a problem hiding this comment.
Well, this still isn't an assignment as Chayim said and more importantly, both tests aren't compiled with rustc.
I think the test code should be the valid Rust code which is compiled with rustc, unless the behavior we'd like to test is diagnostics or error resilience that not panicking or completing/inferencing the partially broken code.
I'd suggest trying out other code that compiled with rustc to test this PR.
|
@ChayimFriedman2 its done. let me know if i missed anything |
rustbot
removed
the
S-waiting-on-review
4 participants
the rhs of binary operators (including compound assignments like
+=) was inferred withExprIsRead::No, which prevented divergence detection for place expressions of type!. this caused false type mismatches when a function's return type depended on the block diverging after something likex += *never_ptr. the rhs of any binary operator is always consumed (read), so this usesExprIsRead::Yesto be consistent with the ordinary assignment and let binding paths.