Generator for staged function should be in .source

[Keno]

julia> @generated function generatedfoo(T)
           :(return $T)
       end
generatedfoo (generic function with 1 method)

julia> (@which generatedfoo(1)).source
ERROR: UndefRefError: access to undefined reference

It doesn't make much sense to me to not have this here. It's what the user wrote, after all, even if you have to look at isstaged to properly interpret it.

[JeffBezanson]

I agree. Also, in theory it would be nice to have an actual unspecialized implementation of a generated function in the unspecialized field.

[andyferris]

I was a little excited to see .source here but I realized it's not that at all... it's Julia IR. Shouldn't "source code" mean code from the source, i.e. user-level Julia syntax?

(btw if I could insert AST/IR into new methods or get the actual toplevel source from a method, I would find that useful - e.g. we could make Traitor.jl actually useful).

[JeffBezanson]

Understood --- the key feature of this field is that it's what was originally written, albeit converted to IR. We could call the field ir, but (1) that's more obscure, and (2) it's not just any IR, but the original IR (as opposed to that of a specialization). I welcome any name suggestions here.

[StefanKarpinski]

.ir0, .ir_original? tbh, anyone who's using this should probably know what IR means.

[JeffBezanson]

cc @vtjnash

[JeffBezanson]

Not directly related to this, but there is a questionable property of the CodeInfo type I didn't notice before: there is no way to uncompress it without having its Method, but it doesn't reference its Method any more. This undermines the status of CodeInfo as an independent type, since having one by itself is basically useless.

Of course, the solution to this might involve eliminating the compressed representation. However, even then we still might want per-method constant tables, in which case you'd still need a Method.

[JeffBezanson]

It's also a bit odd that a CodeInfo can tell you the type of absolutely everything inside a function, except the inferred return type of the function overall. Of course, I was the one who set that in motion by removing the Expr corresponding to the function body, but it really doesn't make sense to wrap every function's IR in an Expr. This leads to, for example, needing to return a CodeInfo=>Type pair from code_typed, which is not terribly natural. So maybe we should just move the rettype field into CodeInfo.

[vtjnash]

This undermines the status of CodeInfo as an independent type, since having one by itself is basically useless.

This is already true for other reasons, but we perhaps should still add the backlink. It would be redundant with its lookup, but so are a lot of the fields of Method.

So maybe we should just move copy the rettype field into CodeInfo.

It wouldn't be a terrible idea to duplicate it, since MethodInstance->rettype ≭ CodeInfo->rettype (in general), but I'm not sure whether that is just more confusing for other developers.

Please let's wait for #17057 first though, since that can complicate these questions quite significantly. And separating the CodeInfo fields out into their own type is one mechanism I've been using to partition the complexity better. (because it means that type inference doesn't change any fields of MethodInstance)

Also, in theory it would be nice to have an actual unspecialized implementation of a generated function in the unspecialized field.

Yes. In theory too, I think this means the ->source field should be a MethodInstance in the isstaged case. The current setup is mostly a lazy way to help cause faster errors when the user forgets to check ->isstaged before using ->source.

[StefanKarpinski]

@Keno: is this really release blocking?

[Keno]

Yes, this whole data structure needs another look. Right now it may not have enough information for the debugger anymore (the second issue Jeff mentioned above).

[tkelman]

it may not have enough information for the debugger anymore

Will fixing this be breaking? If so, this needs to be worked on ASAP

[josefsachsconning]

From my perspective, not fixing this would be breaking, since it is necessary for Gallium to work in v0.6 (stopped working around September 14.)

[JeffBezanson]

@Keno I believe the debugger should be getting a MethodInstance, which should have all the needed info. Do you happen to know of anything else the debugger needs here?

[Keno]

I'll have to revisit this. I remember when we tried porting the debugger over initially, there was something missing, but since we thought this would be refactored anyway, I don't think we went into too much detail.