-
Notifications
You must be signed in to change notification settings - Fork 6
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
Generate default keyword arguments in codegen #18
base: main
Are you sure you want to change the base?
Conversation
thanks for the PR! actually I just realize we cannot change this convention for functions, this is because the https://github.com/Roger-luo/Expronicon.jl/blob/master/src/codegen.jl#L392 the function keyword should be specified as |
I'm not sure, maybe have another arg you can pass in to denote the context is a function or sturct? |
I added |
Actually I'd like to learn your use case first, what's your use case of using JLKwField? Part of the reason why I didn't use a type for function kwargs was because it's Expr is quite simple (unlike struct field that can have line number and docstring) |
I'm using it to generate GraphQL types/functions from a schema - https://github.com/domluna/GraphQLGen.jl. I'm rewriting some portions to use Expronicon instead of pure exprs since it's easier to manage things that way, and to parse things for testing. https://github.com/domluna/GraphQLGen.jl/blob/docstrings/src/jltype.jl#L234-L249 The kwarg generation is used to create functions
This part here
should generate the default values and with patch it does. |
Ah I see why now, function kwargs could have type annotations, this would be a case when a struct for it comes in convenience, what about implementing a |
Would we need JLArg too then? or would be reuse JLField for that? |
Umm now after more thinking on this I believe the right way is to actually have types that represent certain syntax such as arg-like objects that can be used as fields or function arguments and kwarg like objects that can be used as kw fields or kwargs. I also have something similar in Comonicon. I think I should have a closer look on an implementation but I got quite occupied recently (so sorry for this late reply) I think I'd like to work on this early next month after a paper I'm working on is out. Meanwhile would it be ok for now to use the Expr(:kw, a, b) as a workaround for now? |
what do you mean? |
I mean for now you can use julia> fn = JLFunction(;name=:foo, kwargs=[Expr(:kw, :(name::Int), 1)])
function foo(; name::Int = 1)
end
julia> codegen_ast(fn)
:(function foo(; name::Int = 1)
end)
julia> codegen_ast(fn)|>eval
foo (generic function with 1 method) for now, this interface should always work. also note, the function name should be a We just need to canonicalize the internal representation on these syntax types. |
oh ic, yeah that should be a sufficient workaround |
fixes #17