[OM] Ensure every sublist is finished in ListConcatOp evaluation. #8460
Conversation
We would previously check if the value corresponding to each operand of the ListConcatOp was finished evaluating, but in the case where the operand is a ReferenceValue, the reference may be done evaluating before while the underlying ListValue is not. To handle this, a check was added that the result of extractList is fully evaluated, and the evaluation of the ListConcatOp doesn't proceed until that is true. A small unit test shows this scenario, where there are object references in lists passed as field references to a ListConcatOp.
There was a problem hiding this comment.
LGTM, one comment about the test.
Although I can't see it based on the code shown here, I'm guessing the surrounding context will evaluate these values using a worklist style algorithm until everything is fully evaluated? So that makes returning the unevaluated list work since it will come back around and try again once the dependent values have been evaluated.
| auto finalList = | ||
| llvm::cast<evaluator::ListValue>(fieldValue.get())->getElements(); | ||
|
|
||
| ASSERT_EQ(2U, finalList.size()); |
There was a problem hiding this comment.
Perhaps it's worth checking the list contents are what we expect?
There was a problem hiding this comment.
Sure yeah let me do something with the object references to test that we got the two expected objects.
There was a problem hiding this comment.
Added a new field to each object so I could check the value of that field in the test.
Yep that's exactly how it works. You can return a value with the "is fully evaluated" flag set false to indicate that this needs to go back at the end of the worklist. |
…vm#8460) We would previously check if the value corresponding to each operand of the ListConcatOp was finished evaluating, but in the case where the operand is a ReferenceValue, the reference may be done evaluating before while the underlying ListValue is not. To handle this, a check was added that the result of extractList is fully evaluated, and the evaluation of the ListConcatOp doesn't proceed until that is true. A small unit test shows this scenario, where there are object references in lists passed as field references to a ListConcatOp.
We would previously check if the value corresponding to each operand of the ListConcatOp was finished evaluating, but in the case where the operand is a ReferenceValue, the reference may be done evaluating before while the underlying ListValue is not.
To handle this, a check was added that the result of extractList is fully evaluated, and the evaluation of the ListConcatOp doesn't proceed until that is true.
A small unit test shows this scenario, where there are object references in lists passed as field references to a ListConcatOp.