Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dispatch failure or failure to throw error regression #44732

Closed
KristofferC opened this issue Mar 24, 2022 · 2 comments
Closed

Dispatch failure or failure to throw error regression #44732

KristofferC opened this issue Mar 24, 2022 · 2 comments
Labels
regression Regression in behavior compared to a previous version
Milestone

Comments

@KristofferC
Copy link
Sponsor Member

Code to repro the issue:

julia> using Dash

julia> app = dash(assets_folder = "assets_compressed", compress = true);

julia> make_handler(app)

In 1.7:

julia> make_handler(app)
ERROR: The layout must be a component, tree of components, or a function which returns a component.
Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:33
 [2] validate_layout(layout::Nothing)
   @ Dash ~/PkgEval/Dash.jl/src/handler/make_handler.jl:44
 [3] make_handler(app::Dash.DashApp, registry::DashBase.ResourcesRegistry; check_layout::Bool)
   @ Dash ~/PkgEval/Dash.jl/src/handler/make_handler.jl:123
 [4] make_handler(app::Dash.DashApp)
   @ Dash ~/PkgEval/Dash.jl/src/handler/make_handler.jl:158

In 1.8:

julia> make_handler(app); # No error

Now, this should cleary error because what ends up getting called is

https://github.com/plotly/Dash.jl/blob/f0606e07d2479fd8b5be1da4a6a59656e24acfa3/src/handler/make_handler.jl#L123

which has these three definitions:

https://github.com/plotly/Dash.jl/blob/f0606e07d2479fd8b5be1da4a6a59656e24acfa3/src/handler/make_handler.jl#L40-L44

and app.layout is nothing:

julia> app.layout === nothing
true

In fact, it errors if we execute it manually:

julia> Dash.validate_layout(Dash.get_layout(app))
ERROR: The layout must be a component, tree of components, or a function which returns a component.
Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:35
 [2] validate_layout(layout::Nothing)
   @ Dash ~/PkgEval/Dash.jl/src/handler/make_handler.jl:44

and also when running it through the debugger:

julia> using Debugger

julia> @run make_handler(app)
ERROR: The layout must be a component, tree of components, or a function which returns a component.

So something here makes the dispatch go to the wrong function or the error to be swallowed.

@KristofferC KristofferC added the regression Regression in behavior compared to a previous version label Mar 24, 2022
@KristofferC KristofferC added this to the 1.8 milestone Mar 24, 2022
@vtjnash
Copy link
Sponsor Member

vtjnash commented Mar 24, 2022

Fascinating. It declares it called consteval on nothing and confidently concluded (during inlining) that the method has no side-effects and returns nothing.

    ┌ @ /home/vtjnash/.julia/packages/Dash/yscRy/src/app/dashapp.jl:69 within `get_layout`
    │┌ @ Base.jl:37 within `getproperty`
6 ──││ %17  = Base.getfield(app, :layout)::Union{Nothing, Function, Component}
│   └└
│    %18  = (isa)(%17, Nothing)::Bool
└───        goto #8 if not %18
7 ──        goto #11
8 ── %21  = (isa)(%17, Component)::Bool
└───        goto #10 if not %21
9 ── %23  = π (%17, Component)
│   ┌ @ /home/vtjnash/.julia/packages/Dash/yscRy/src/handler/make_handler.jl:40 within `validate_layout`
│   │┌ @ /home/vtjnash/.julia/packages/Dash/yscRy/src/Components.jl:23 within `validate`
│   ││┌ @ set.jl:9 within `Set`
│   │││ %24  = invoke Dict{Symbol, Nothing}()::Dict{Symbol, Nothing}
│   │││┌ @ set.jl:6 within `_Set`
│   ││││ %25  = %new(Set{Symbol}, %24)::Set{Symbol}
│   ││└└
│   ││        invoke Dash._validate(%23::Component, %25::Set{Symbol})::Union{Nothing, Bool}
│   └└
└───        goto #11
10 ─        Dash.validate_layout(%17)::Any
└───        goto #11
11 ┄        nothing::Nothing
 • %12  = < consteval > validate_layout(::Union{Nothing, Function, Component})::Any
   %12  = validate_layout(::Function)::Any
   %12  = validate_layout(::Component)::Any

@aviatesk
Copy link
Sponsor Member

We seem to propagate effects of a thrown concrete-call wrongly. #44762 should fix this.

aviatesk added a commit to aviatesk/julia that referenced this issue Mar 29, 2022
KristofferC pushed a commit that referenced this issue Mar 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression Regression in behavior compared to a previous version
Projects
None yet
Development

No branches or pull requests

3 participants