MilesCranmer / DispatchDoctor.jl

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

Better source tracking #7

Closed MilesCranmer closed 1 month ago

MilesCranmer commented 1 month ago

It seems error messages from functions will land on the outer @stable which is kind of annoying for large blocks covering entire libraries. I would like for better error tracking. Perhaps instead the codegen should be lazy, rather than done immediately.

ericphanson commented 1 month ago

I wonder if something like https://github.com/JuliaLang/julia/pull/31335 can help?

MilesCranmer commented 1 month ago

I think so? Maybe we should ping it to get merged...

thofma commented 1 month ago

Can you throw the error from inside the closure thingy? Then at least the stacktrace would be a bit more useful:

julia> OK = maximal_order(K)
ERROR: TypeInstabilityError: Instability detected in `_newton_polygon` defined at /bla/dev/Hecke/src/Hecke.jl:571 with arguments `(Vector{ZZPolyRingElem}, ZZRingElem)`. Inferred to be `Any`, which is not a concrete type.
Stacktrace:
  [1] _newton_polygon(dev::Vector{ZZPolyRingElem}, p::ZZRingElem)
    @ Hecke bla/packages/DispatchDoctor/uWnHi/src/DispatchDoctor.jl:308
  [2] ##gens_overorder_polygons_closure#645
    @ bla/dev/Hecke/src/NumFieldOrd/NfOrd/MaxOrd/Polygons.jl:411 [inlined]
  [3] gens_overorder_polygons(O::AbsSimpleNumFieldOrder, p::ZZRingElem)
    @ Hecke bla/packages/DispatchDoctor/uWnHi/src/DispatchDoctor.jl:311

So that the [1] would be ##_newton_polygon in my case, with an approximately correct source location. (Note that /bla/dev/Hecke/src/Hecke.jl:571 is the outer @stable mentioned in OP, which is not that useful.)

MilesCranmer commented 1 month ago

If we throw the error from inside the caller function then the type inference wouldn’t work (it would infer Union{} as that’s what errors result in). I wonder is there any way we can extract the LineNumberNode when parsing the function code? Since the macro’s __source__ obviously isn’t that useful here, maybe we can get a more precise location from that.

Maybe we can just locate the first LineNumberNode in the function expression and use that as source_info

MilesCranmer commented 1 month ago

Had another idea on the discourse for potentially solving this https://discourse.julialang.org/t/ann-dispatchdoctor-jl-offers-you-a-prescription-for-type-stability/114837/23?u=milescranmer

MilesCranmer commented 1 month ago

Fixed by #20