undo breaking change of removing parent field from CodeInfo#53393
undo breaking change of removing parent field from CodeInfo#53393
parent field from CodeInfo#53393Conversation
|
I still think this is semantic nonsense. Cthulhu needs to be fixed anyway, since it's just making bad assumptions about the relationships of CodeInfo and MethodInstance. So 👎 from me on this change, but if you really want it, I guess I won't stand in the way. |
The loss provided no value, as it is easy to provide this info, and has downstream users as well as being documented for use.
pre_13 would fail to read the max_world field, resulting in the stream getting desynchronized and corrupted. Add some type-asserts to help detect that error earlier.
ed42c2b to
2126b67
Compare
|
I agree that this value should already be known to the caller, so it should commonly be redundant here. However, our public codegen interface ( There are also some cases in codegen where the parent value would not be quite the same as the contextual info in the caller, because of a particular rare case of codegen despecialization that means the |
…ang#53393) This drops the unnecessary breaking change from JuliaLang#53219 by re-adding the optional `parent` field to CodeInfo. A few months ago, I had actually already put together a branch that also fixed Keno's complaints about how it was not set reliably, so I copied that code here also, so that it should get set more reliably whenever a CodeInfo is associated with a MethodInstance (either because it called `retrieve_code_info` to get IR from the Method or `uncompress_ir` to get it from the inference cache). This does not entirely fix Cthulhu's test errors, since I don't see any particular reason to re-introduce the other fields (min_world, max_world, inferred, edges, method_for_inference_limit_heuristics) that got removed or are now set incorrectly, and most errors appear to be instead related to the `Expr(:boundscheck, true/false)` change. However, they could be trivially re-added back as placeholders, if someone encounters a broken package that still needs them.
This drops the unnecessary breaking change from #53219 by re-adding the optional
parentfield to CodeInfo. A few months ago, I had actually already put together a branch that also fixed Keno's complaints about how it was not set reliably, so I copied that code here also, so that it should get set more reliably whenever a CodeInfo is associated with a MethodInstance (either because it calledretrieve_code_infoto get IR from the Method oruncompress_irto get it from the inference cache). This does not entirely fix Cthulhu's test errors, since I don't see any particular reason to re-introduce the other fields (min_world, max_world, inferred, edges, method_for_inference_limit_heuristics) that got removed or are now set incorrectly, and most errors appear to be instead related to theExpr(:boundscheck, true/false)change. However, they could be trivially re-added back as placeholders, if someone encounters a broken package that still needs them.