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

v2 API: Redefining an input with a new type? #22

Open
ghost opened this issue Jun 4, 2020 · 0 comments
Open

v2 API: Redefining an input with a new type? #22

ghost opened this issue Jun 4, 2020 · 0 comments

Comments

@ghost
Copy link

ghost commented Jun 4, 2020

What should happen if you re-declare a salsa input with a new type? Currently, it succeeds, and the old setter remains, but the getter is replaced, so setting and getting the old type throws an error during the getter:

julia> @declare_input x(rt)::Int
(x, set_x!, delete_x!)

julia> @declare_input x(rt)::Vector{Int}
(x, set_x!, delete_x!)

julia> rt = Runtime()
Salsa.Runtime(Salsa.DefaultStorage(0, ...))

julia> set_x!(rt, 2)

julia> set_x!(rt, Int[])

julia> x(rt)
0-element Array{Int64,1}

julia> set_x!(rt, 2)

julia> x(rt)
ERROR: TypeError: in typeassert, expected Array{Int64,1}, got Int64
Stacktrace:
 [1] x(::Salsa._TopLevelRuntime{Salsa.EmptyContext,Salsa._DefaultSalsaStorage.DefaultStorage}) at /Users/nathandaly/.julia/dev/Salsa/src/Salsa.jl:769
 [2] top-level scope at REPL[51]:1

julia> 

What should this do? If you declare a new input with a different signature, that's fine. But if it has the same signature with different output type? What should that do?

  • Throw an error when declaring x with a new type?
  • Automatically erase the old setter method for x? (And maybe print a warning, just like redefining a method throws a warning in julia? -- though i guess we'll already get the warnings automatically from julia if this is done in a module, so maybe no need for the warning...)

I think I'm leaning towards automatically, silently, erasing the old setter method.

@NHDaly NHDaly assigned NHDaly and unassigned NHDaly Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant