Skip to content

Commit

Permalink
Merge pull request #32 from RelationalAI-oss/nhd-default-values
Browse files Browse the repository at this point in the history
Add support for default values for Salsa derived functions
  • Loading branch information
NHDaly authored Jul 29, 2020
2 parents 0437581 + 795eeea commit cdd705b
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
13 changes: 12 additions & 1 deletion src/Salsa.jl
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,14 @@ macro derived(f)
# for untyped args. `fullargs` will have all args w/ names and types.
argnames = _argnames(args)
argtypes = _argtypes(args)
fullargs = [Expr(:(::), argnames[i], argtypes[i]) for i = 1:length(args)]
# Ensure that every argument is named, even unnamed parameters, so that they can be
# forwarded from the outer function into the inner one.
fullargs = deepcopy(args)
for (i,a) in enumerate(fullargs)
if a isa Expr && a.head == :(::) && length(a.args) == 1
pushfirst!(a.args, argnames[i])
end
end

# Build the Key type once, in global scope, since it's expensive to construct at
# runtime. This code will be included at global scope.
Expand Down Expand Up @@ -789,6 +796,10 @@ macro declare_input(e::Expr)
# for untyped args. `fullargs` will have all args w/ names and types.
argnames = _argnames(args)
argtypes = _argtypes(args)
# NOTE: We don't support default values in inputs!
# TODO: Do we want to support this? We'd probably have to / want to change the order
# of the fields in `set_*!()`, so that the value comes first? Or maybe we could fake
# this by generating multiple methods, but frankly that seems a bit weird.
fullargs = [Expr(:(::), argnames[i], argtypes[i]) for i = 1:length(args)]
# Put the filled-out args back into the original expression.
args = callexpr.args[2:end] = fullargs
Expand Down
15 changes: 15 additions & 0 deletions test/Salsa.jl
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,21 @@ end
# Where clauses on inputs aren't yet supported. Do they even make sense?
#Salsa.@declare_input where_input(db, ::Type{T})::T where T
end
@testset "default values - derived functions" begin
@declare_input source_text(rt, name::String)::String
@derived default_source(rt, name::String="stdlib") = source_text(rt, name)

rt = Runtime()
set_source_text!(rt, "stdlib", "hello")
@test default_source(rt) == default_source(rt, "stdlib") == "hello"
end
# Default values are not supported for inputs.
#@testset "default values - inputs" begin
# @declare_input currency_value(rt, country="US")::Float64
#
# rt = Runtime()
# set_currency_value!(rt, 1.0)
#end
end

@testset "inputs and derived functions support docstrings" begin
Expand Down

0 comments on commit cdd705b

Please sign in to comment.