Support array types with dependent bounds. #4751
Conversation
| return ints().Get(insts().GetAs<IntValue>(bound_id).int_id).getZExtValue(); | ||
| auto GetArrayBoundValue(InstId bound_id) const -> std::optional<uint64_t> { | ||
| if (auto bound = insts().TryGetAs<IntValue>( | ||
| constant_values().GetConstantInstId(bound_id))) { |
There was a problem hiding this comment.
Why is bound_id in GetAs() changed to constant_values().GetConstantInstId(bound_id) in TryGetAs()? Inside they both call Get(inst_id). Does this allow some different inputs to GetArrayBoundValue? The callers are passing the same inst ids it seems.
There was a problem hiding this comment.
insts().GetAs<IntValue>(bound_id) requires bound_id to exactly be an IntValue instruction. Passing the instruction through constant_values().GetConstantInstId(bound_id) first allows any bound_id whose constant value is an IntValue. So the old formulation would fail on the bound in the array type [i32; 1 + 1] whereas the new formulation succeeds there.
I don't think this is necessary for any of the current callers of this function -- they all appear to be operating on canonical array types, which would have an IntValue as the bound_id if the bound is a constant integer. I don't remember why I made this change, but I think I was concerned about changing this from check-failing to silently returning nullopt in the case that an expression with a constant value were passed in, and the best solution seemed to be to make it return a correct bound in that case instead.
Given
N:! i32, the type[T; N]is a valid but dependent array type.