MilesCranmer / DispatchDoctor.jl

The dispatch doctor prescribes type stability
Apache License 2.0
128 stars 6 forks source link

Cannot register macro with same name #43

Open jakobjpeters opened 6 days ago

jakobjpeters commented 6 days ago

The current design limits the extensibility of the package, and I argue that no macro should be given special treatment. While I'm not sure if this is an issue when a user depends on a package registering the same name, the predefined behavior intended for Distributed.@everywhere, Turing.@model, and MacroTools.@capture prevents anyone else from registering their own macro under one of those names. This applies to the Base and Core macros too, but one could argue that users shouldn't be shadowing those names anyways. While admittedly an edge case, this issue could increase in severity as DispatchDoctor.jl becomes more widely used or even moved to Core #40.

Possible solutions

I am personally in favor of #1 or #2, maybe #4, but not #3.

1. Compare macro equality at run-time (EDIT: or compile-time?)

I think that this can be implemented by storing both the module and name of each macro in MACRO_BEHAVIOR and returning an expression from get_macro_behavior and _stabilize_all. The expression would contain an if-statement for the union of behaviors of a given name in MACRO_BEHAVIOR and the default behavior.

EDIT: This might be able to be done at compile-time (at least, in the same way that LLVM can optimize it away), since macros have unique types.

Pros:

Cons:

2. Treat all macros as incompatible

Pros:

Cons:

3. Make macro behavior opt-in only

The MACRO_BEHAVIOR dictionary would be empty by default, requiring users to register macros as needed.

Pros:

Cons:

4. Make macro behavior toggle-able or customizable with preferences

jakobjpeters commented 6 days ago

I just realized #2 wouldn't support docstrings, oops!

@stable begin
""" foo() """
foo() = 1
end
MilesCranmer commented 5 days ago

For 1, would this mean having an if statement over all macros — at every occurrence of a macro — which only gets evaluated at runtime?

MilesCranmer commented 5 days ago

Maybe another option is to allow a per-module register_macro!? This is done in DynamicQuantities to allow per-module unit symbol mapping

jakobjpeters commented 5 days ago

For 1, would this mean having an if statement over all macros — at every occurrence of a macro — which only gets evaluated at runtime?

Unfortunately it might, but I'm not sure and am hoping to get it to compile away. I'm still working on a prototype, which is admittedly difficult. Returning an expression like I mentioned doesn't work. My next idea is to use downward callback functions to simulate modifying the expression without actually touching it so that the branch requiring the run-time macro can do it's thing without interfering. I believe this approach is similar to how Haskell handles impure functions by pretending to do work but evaluating it later.

Maybe another option is to allow a per-module register_macro!? This is done in DynamicQuantities to allow per-module unit symbol mapping

So the preferences file would additionally store the module and the macro would pass in __module__? Seems like a good idea!

jakobjpeters commented 5 days ago

Assuming the modules can be recognized and we store the macro in MACRO_BEHAVIORS with getproperty(parent_module, name), we might be able to dispatch on the macro with get_behavior(macro) itself, which is unique per module, and then I think the compiler will just toss out the unneeded methods?

EDIT: The methods would be generated from the table, whereas the dispatch might be able to occur at compile-time since macros can be returned as values from the expressions

julia> macro f() Symbol("@doc") end;

julia> typeof(@f)
Core.var"#@doc"
MilesCranmer commented 5 days ago

By the way, I’m not sure Preferences.jl makes sense here, because Preferences is more for compiler flag-like stuff. But declaring incompatible macros seems like it should be a constant and thus live in the code.

MilesCranmer commented 5 days ago

@ven-k how is @register_unit in DynamicQuantities able to only affect the local module's unit registry? I never really understood why that works. I'd like to do something similar here with a @register_macro.

jakobjpeters commented 4 days ago

Am I doing this incorrectly? Maybe it could be done by having the macro check whether the table already exists in the current module and define it if not?

julia> module A
       using DynamicQuantities
       @register_unit MyVolt 1u"V"
       end;

julia> module B
       using DynamicQuantities
       @register_unit MyVolt 2u"V"
       end;
ERROR: LoadError: Unit `MyVolt` is already defined as `1.0 m² kg s⁻³ A⁻¹`

I'm working on #1; still trying to fix tests but it might be viable. EDIT: The DontPropagateMacro method is incorrect here, but it shows the concept.

julia> Base.remove_linenums!(@macroexpand @stable @show 1)
┌ Warning: `@stable` found no compatible functions to stabilize
│   source_info = :(#= REPL[30]:1 =#)
│   calling_module = Main
└ @ DispatchDoctor._Stabilization ~/Documents/programming/forks/DispatchDoctor.jl/src/stabilization.jl:40
quote
    var"##435"(::DispatchDoctor._Interactions.IncompatibleMacro) = begin
            begin
                Base.println("1 = ", Base.repr(begin
                            local var"#196#value" = 1
                        end))
                var"#196#value"
            end
        end
    var"##435"(::DispatchDoctor._Interactions.CompatibleMacro) = begin
            begin
                Base.println("1 = ", Base.repr(begin
                            local var"#197#value" = 1
                        end))
                var"#197#value"
            end
        end
    var"##435"(::DispatchDoctor._Interactions.DontPropagateMacro) = begin
            1
        end
    var"##435"(DispatchDoctor._Interactions.CompatibleMacro())
end
jakobjpeters commented 3 days ago

Option #1 only works for the return value of expressions, which is a pretty big limitation. But it was fun to try it out at least