active_reg_inner with justActive = true incorrect with immutable types that can be incompletely initialized

[danielwe]

In the process of achieving comprehensive test coverage for #1852 I've stumbled upon the following corner case:

julia> using Enzyme

julia> struct Foo
           x::Float64
           y::Core.Box
           dummy(x) = new(x)
       end

julia> Enzyme.Compiler.active_reg_inner(Tuple{Foo}, (), nothing, Val(false)#=Val{justActive}=#)  # correct
MixedState::ActivityState = 3

julia> Enzyme.Compiler.active_reg_inner(Tuple{Foo}, (), nothing, Val(true)#=Val{justActive}=#)  # incorrect, should be ActiveState
AnyState::ActivityState = 0

As far as I understand, justActive = true is supposed to map MixedState to ActiveState and DupState to AnyState, for when you just want to know whether or not the type contains immutable active storage. This example breaks that mapping.

The problem remains if I replace dummy with Foo. I'm using dummy here to emphasize that it's the incomplete use of new in the struct definition that triggers the error, rather than something specific to inner constructor methods. The problem also remains if I add a fully initializing inner constructor next to the incomplete one.

The issue disappears with any of the following changes:

• Removing dummy(x)
• Only invoking new in a fully initializing way, e.g., new(x, Core.Box(nothing))

[wsmoses]

your understanding is correct and this is a bug needing investigation

[danielwe]

The culprit is the following conditional:

Enzyme.jl/src/compiler.jl

Lines 470 to 472 in 3c0871d

	if justActive && !allocatedinline(subT)
	return Val(AnyState)
	end

Base.allocatedinline(Foo) == false, presumably due to the possibility of incomplete initialization. Removing incomplete invocations of new makes Base.allocatedinline(Foo) == true, avoiding this branch.

Should !allocatedinline be replaced by ismutabletype here?

[danielwe]

Changing to ismutabletype does not break any tests locally. However, it's a very different condition, and it's unclear to me whether it's the correct one. For one, most abstract types are !allocatedinline but are not ismutabletype, so the set of field types that would end up in this branch is quite different.

Is this branch just a short-circuiting early return, or is it necessary for correctness? If it's the former, the condition only has to match a subset of the types that could use an early return, so ismutabletype might be an adequate solution.