Conversation
5b361b4 to
c7fcea9
Compare
c7fcea9 to
d7c1324
Compare
| auto r = eitherResult(in); | ||
| CHECK_ERR(r); | ||
| res.emplace_back(std::move(*r)); | ||
| res.emplace_back(*std::move(r)); |
There was a problem hiding this comment.
Why is this better or more correct?
There was a problem hiding this comment.
I based this off of Google's style guide for absl::StatusOr which recommends moving from the whole thing instead of just the value inside to indicate that the whole result type is moved-from. Basically it would likely be a mistake to do something with r after this and moving r instead of *r hints that to the reader. I got it from here (ctrl+f "Moving from the value"): https://abseil.io/tips/181
There was a problem hiding this comment.
Interesting! Makes sense, though I find it slightly less intuitive. Seems fine to introduce here.
| if (!val.type.isRef() || | ||
| !HeapType::isSubType(val.type.getHeapType(), ref->type)) { |
There was a problem hiding this comment.
We should probably check that the value is not null here, too.
| auto valLanes = val.getLanesI32x4(); | ||
| auto expLanes = v->getLanesI32x4(); |
There was a problem hiding this comment.
Can we avoid hard-coding a lane interpretation here? Maybe we should always use LaneResults for vectors if we want to be more precise about lanes in error messages?
|
|
||
| if (auto* v = std::get_if<Literal>(&expected)) { | ||
| if (val != *v) { | ||
| if (val.type.isVector() && v->type.isVector() && isAlternative) { |
There was a problem hiding this comment.
It's not clear to me why we check isAlternative here.
| << resultToString(result); | ||
| return Err{err.str()}; | ||
| } | ||
| for (Index i = 0; i < values->size(); ++i) { |
There was a problem hiding this comment.
Do you know whether either cases are supposed to handle multiple values in the case of multivalue or whether each value gets its own either clause? I would have expected the former, in which case this loop would have to be inside the either handling. (But if there are no tests that exercise this yet, it's ok to leave a TODO rather than fixing it.)

Example failure:
Example failure with SIMD from
relaxed_min_max.wast:Part of #8261 and #8315. Fixes
i16x8_relaxed_q15mulr_s.wast,i8x16_relaxed_swizzle.wast,relaxed_madd_nmadd.wastspec tests, and partially fixesrelaxed_dot_product.wast,relaxed_laneselect.wast,relaxed_min_max.wast, andthreads/thread.wast.