codegen_ssa: replace a Result by an Either #143291
Conversation
rustbot
added
S-waiting-on-review
|
Some changes occurred in compiler/rustc_codegen_ssa |
|
r? @scottmcm |
| @@ -648,9 +649,9 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, Result<V, abi::Scalar>> { | |||
| pub fn build(&self) -> OperandRef<'tcx, V> { | |||
| let OperandRef { val, layout } = *self; | |||
|
|
|||
| let unwrap = |r: Result<V, abi::Scalar>| match r { | |||
| Ok(v) => v, | |||
| Err(_) => bug!("OperandRef::build called while fields are missing {self:?}"), | |||
There was a problem hiding this comment.
I'd used an Err here because in build it is an error.
An Either is fine too, so I don't mind doing this, but I'd really rather land #138759 that will conflict with this first, since it's approaching 4 months :/
There was a problem hiding this comment.
I was mostly looking at the big match in insert_field, which looked like it was... finding errors and removing them? That doesn't look like a Result.
Some actual comments explaining what happens here with that sum type would have helped a lot but, not understanding the code I can't add those unfortunately. Would be great if you could. :)
An Either is fine too, so I don't mind doing this, but I'd really rather land #138759 that will conflict with this first, since it's approaching 4 months :/
Sure, this can wait.
There was a problem hiding this comment.
#138759 landed, and I rebased :)
|
☔ The latest upstream changes (presumably #138759) made this pull request unmergeable. Please resolve the merge conflicts. |
RalfJung
force-pushed
the
result-isnt-either
branch
from
5cea731 to
1628a7c
Compare
|
Thanks for rebasing! The code change looks good to me |
bors
added
S-waiting-on-bors
|
☔ The latest upstream changes (presumably #143473) made this pull request unmergeable. Please resolve the merge conflicts. |
bors
added
S-waiting-on-author
RalfJung
force-pushed
the
result-isnt-either
branch
from
1628a7c to
3fd85b3
Compare
RalfJung
force-pushed
the
result-isnt-either
branch
from
3fd85b3 to
3cd13f7
Compare
|
@bors r=scottmcm rollup |
bors
added
S-waiting-on-bors
Let `rvalue_creates_operand` return true for *all* `Rvalue::Aggregate`s Draft for now because it's built on Ralf's #143291 Inspired by #138759 (comment) where I noticed that we were nearly at this point, plus the comments I was writing in #143410 that reminded me a type-dependent `true` is fine. This PR splits the `OperandRef::builder` logic out to a separate type, with the updates needed to handle SIMD as well. In doing so, that makes the existing `Aggregate` path in `codegen_rvalue_operand` capable of handing SIMD values just fine. As a result, we no longer need to do layout calculations for aggregate result types when running the analysis to determine which things can be SSA in codegen.
Rollup merge of #143291 - RalfJung:result-isnt-either, r=scottmcm codegen_ssa: replace a Result by an Either `Err` here clearly does not indicate an "error" of any sort, so the use of `Result` is confusing. Let's use a sum type that does not come with the baggage of `Result`.
Errhere clearly does not indicate an "error" of any sort, so the use ofResultis confusing. Let's use a sum type that does not come with the baggage ofResult.