`@stable` doesn't accept `@propagate_inbounds`

[matthias314]

julia> @stable Base.@propagate_inbounds f(v, i) = v[i]
ERROR: LoadError: begin
    #= /usr/local/julia/packages/DispatchDoctor/OveAl/src/DispatchDoctor.jl:223 =#
    function var"##f_closure#225"(v, i; )
        #= REPL[2]:1 =#
        v[i]
    end
    #= /usr/local/julia/packages/DispatchDoctor/OveAl/src/DispatchDoctor.jl:224 =#
    #= /usr/local/julia/packages/DispatchDoctor/OveAl/src/DispatchDoctor.jl:224 =# (Base).@__doc__ function f(v, i; )
            #= /usr/local/julia/packages/DispatchDoctor/OveAl/src/DispatchDoctor.jl:214 =#
            var"##f_return_type#226" = (Base).promote_op(var"##f_closure#225", map(DispatchDoctor.specializing_typeof, (v, i))...)
            #= /usr/local/julia/packages/DispatchDoctor/OveAl/src/DispatchDoctor.jl:215 =#
            if (DispatchDoctor.type_instability)(var"##f_return_type#226") && (!((DispatchDoctor.is_precompiling)()) && (DispatchDoctor.checking_enabled)())
                #= /usr/local/julia/packages/DispatchDoctor/OveAl/src/DispatchDoctor.jl:216 =#
                throw((TypeInstabilityError)("`f`", "REPL[2]:1", (v, i), (;), () .=> (), var"##f_return_type#226"))
            end
            #= /usr/local/julia/packages/DispatchDoctor/OveAl/src/DispatchDoctor.jl:219 =#
            return var"##f_closure#225"(v, i)
        end
end is not a function expression

Same for

@stable module M

Base.@propagate_inbounds f(v, i) = v[i]

end

which might be more realistic.

I'm using the current version 9cc1040 of DispatchDoctor.

[MilesCranmer]

Base.@propagate_inbounds @stable f(v, i) = v[i]

works. You can put that within the module as well, and it will cause the outermost @stable to ignore that block of code.

Not sure what else I can do; @stable returns two functions... If a macro can't work with that, then that macro needs to be evaluated first. @stable is more flexible so can be evaluated last.

@stable returns the following:

    return quote
        $(combinedef(func_with_body))
        $(Base).@__doc__ $(combinedef(func))
    end

The $(Base).@__doc__ is for this exact purpose, to say which function the docs should map to.

If there were a $(Base).@__forward_macro__ then that could work here. But not sure how else to fix this other than having people write it manually.

[MilesCranmer]

Alternatively, Base.@propagate_inbounds should apply to the first function in the expression, rather than expecting it to be the first expression it sees

[matthias314]

What should the result be when @stable encounters @propagate_inbounds or some other macro? I see two possible answers:

1) The effect of the other macro should be preserved. This is tricky to implement. For example, @propagate_inbounds would have to be applied to both the original function and the new wrapper functions.

2) The result should be working code, but the effect of the other macro may be lost. This would probably be easy to implement because most macros (@propagate_inbounds, @inline) could simply be dropped. (I am not sure about @assume_effects.)

I think the answer depends on how DispatchDoctor is envisaged to be used. If it is supposed to be a permanent part of other packages, then you want to have (1). On the other hand, if it is just a diagnostic tool used during development, then (2) would be enough.

[MilesCranmer]

I guess I could just skip @propagate_inbounds for now. I do have a list of macros known to be incompatible:

https://github.com/MilesCranmer/DispatchDoctor.jl/blob/e2a7558db510d6b1f0fe17b018b78b9e567cd82e/src/DispatchDoctor.jl#L8

[MilesCranmer]

Realistically there's no way to accomplish this:

The effect of the other macro should be preserved. This is tricky to implement. For example, @propagate_inbounds would have to be applied to both the original function and the new wrapper functions.

for all current and future macros. Maybe I can just make things more modular so someone can inject certain behavior for these if they really need it.

Or, I could have interaction with macros be opt-in rather than opt-out?

[matthias314]

I'm still unsure how you expect DispatchDoctor to be used. As I said, if it's a diagnostic tool, then dropping macros like @propagate_inbounds seems reasonable to me.

As a default approach for unknown macros, can't @stable collect the macros it sees and apply them to the original function? For example,

@stable @macro1 @macro2 f(x) = 2*x

would be transformed to something like

begin
    @macro1 @macro2 original_f(x) = 2*x
    f(x) = wrapper function calling original_f(x)
end

[MilesCranmer]

Fixed by 29fc39e47492bc84e0c4f16753577725c86e30dc

[MilesCranmer]

@matthias314 It's really tricky because macros are so flexible. I don't think there is a sensible general approach. Sometimes it makes sense to apply to the original function definition only, but sometimes it does not. For example, for @inline, you would actually want to apply to both functions. For macros which register a function in some global list, you would not want it to register both the original function symbol and the generated function.

Maybe the best approach is just let the user register such behavior somehow?

I'm still unsure how you expect DispatchDoctor to be used.

I mean I could see it being useful in both contexts depending on use-case:

1. In performance critical applications you would definitely want to tag important kernels with @stable so that you know immediately if there's any type stability.
2. When using StaticCompiler.jl, you can't compile type instabilities, so DispatchDoctor.jl would give you better and more localized error messages.
3. For general usage which has no such constraints, I could see @stable warnonly=true being generally useful to put into a package. At that point it's really subjective whether a user wants to have it always on or just for testing or not.
4. For libraries which have very wide audiences and no such constraints, perhaps its only useful as a diagnostic tool, but perhaps you wouldn't want to leave it on all the time in case there are necessary type instabilities.

[MilesCranmer]

As a default approach for unknown macros, can't @stable collect the macros it sees and apply them to the original function?

I have no idea how to do this, but if you can figure something out, let me know.

[MilesCranmer]

You also have macros like

@macro1 prefix1=1 prefix2=2 function f(x)
    x
end postfix1=1 postfix2=2

This is totally valid macro syntax! You can type it into an expression and see that it just becomes a list of args in an Expr.

You can also have

@put_inside (x,) -> x^2 (x, y) -> x * y

where it wouldn't be clear what function to actually operate on.

This is why a general approach to solving @macro1 @macro2 f(x) = x is basically impossible (without introducing some hard-to-debug errors).

I'm inclined to just make custom macros opt-in, and the user has to explicitly state the desired behavior.

[MilesCranmer]

18 handles this properly.