fix partially_inline! with isdefined

[simeonschaub]

ref #40562 (comment)

[vtjnash]

This is still not fully correct (since it first generates invalid IR, then tries to fix cases it can detect as such), but I think it may be valid for any cases that dynamically reach here?

[simeonschaub]

We could assert that the isdefined expression contains one of either a slot number, global ref, symbol or static parameter before we do the replacements, but that means the IR was already invalid before. If the original IR and replacements are valid, I don't see any way this would produce anything invalid.

[vtjnash]

Typically if there is a replacement made (unless it is a renumbering), that implies that the value was defined, which is what generally makes this valid dynamically. But it might therefore depend on the given semantics of the inlining.

[simeonschaub]

Ah, I see what you mean. I can't really think of any examples where this wouldn't be the case. The only packages using this function directly seem to be Cassette and IRTools (https://juliahub.com/ui/CodeSearch?q=partially_inline%21&u=all&t=all), so this is probably fine.

[JeffBezanson]

Just as a matter of style, I think it would be better to implement this by adding a case for isdefined expressions just like all other forms that need to be handled. That might seem like it repeats some logic for SlotNumber, but overall I think it's a better structure.

[vchuravy]

Not sure if applicable

julia> f() where T = @isdefined(T)
f (generic function with 1 method)

julia> f()
false
julia> @code_typed f()
CodeInfo(
1 ─ %1 = $(Expr(:isdefined, :($(Expr(:static_parameter, 1)))))::Bool
└──      return %1
) => Bool

So you can have a not defined static_parameter

[simeonschaub]

That's a good point, but for partially_inline! to work, you would still need to provide a value for the static parameter. Perhaps we should allow the sparam replacements to have undefined entries and only replace with true if the replacement is defined?

[simeonschaub]

Does this look good to you?

[JeffBezanson]

We typically use TypeVars to indicate an unknown static parameter, but this function doesn't seem to support that (yet) so we can leave that for now, and this looks good.

[simeonschaub]

Bump.

[simeonschaub]

Friendly bump :)

[JeffBezanson]

Needs rebase