GUFA: Fix string.encode#8812
Conversation
| auto* set = | ||
| builder.makeArraySet(curr->ref, curr->index, get, MemoryOrder::Unordered); | ||
| // The index does not matter, as we do not track array indexes yet TODO | ||
| Expression* index = builder.makeNop(); |
There was a problem hiding this comment.
It seems like it would be easy to change some other code that would then trip over a none-typed index expression. Should we make this an i32.const 0 instead?
There was a problem hiding this comment.
0 would be incorrect, though, which could lead to a subtle bug. a Nop doesn't even have the right type, so if this is actually used, it should error loudly.
There was a problem hiding this comment.
I see. I'm sympathetic to failing quickly and loudly on errors, but I don't think a type error will be super obvious. Maybe some other sentinel would be better, like -2? (IIRC, -1 is used for descriptors.) I don't feel too strongly about this, though. I guess this ArraySet will only be visible in call stacks that include this function?
There was a problem hiding this comment.
-1 or -2 are valid values, though - if we improve the analysis, it would be like we are actually writing there, which seems risky for correctness.
If we really want to make valid IR here, we could generate imported functions and put calls to them here. Then it would even be valid to print the module in the middle of the pass here. I think if we ever run into annoying issues here, we can consider that.
| (func $read (export "read") (result i32) | ||
| ;; We could infer what the value is here, since there is only one write. TODO | ||
| ;; Meanwhile, we should not infer a wrong value, even in closed world. | ||
| (array.atomic.get_s $array |
There was a problem hiding this comment.
Does it matter that this is atomic? If not, we should make it non-atomic to simplify the test.
There was a problem hiding this comment.
Oops, thanks, this was just in the reduced testcase and I didn't notice it. I made it unatomic.
2 participants
This writes to an array. Without noticing the write, we might think it
contains the wrong thing.