Use generated function rather than eval
#2
Conversation
Since the lowered code being generated does not have external inputs, a generated function is preferred. Otherwise, it can be tricky to deal with the world age issues. As it stands, an `invokelatest` is required in several cases (and several more in 1.12 - see JuliaLang/julia#56509).
|
Thanks for the PR, I was not aware of generated functions, but a quick read on them indeed suggests that they should be a capable tool for the behaviour I intend to have, without the need of an eval. I will look at the PR, and might ask some questions, but in general I like the idea. |
KeithWM
left a comment
KeithWM
left a comment
There was a problem hiding this comment.
Thanks again for the suggestion. Could you take a look at my questions and comments? I do understand the request for some comments in the generated function is a bit richt for a code base that features no comments nor docstrings, but still.
|
|
||
| function further_substitution(it::InverseTaylor{N}, subbed::Num) where {N} | ||
| for i in 1:N | ||
| @info "Expanding term $i to create ivnersion expression" |
There was a problem hiding this comment.
Do I understand correctly that these @infos are removed for the sake of keeping the @generated function pure?
There was a problem hiding this comment.
Yes, printing is not allowed from inside generated functions.
|
|
||
| function create_expressions(n::Int) | ||
| it = InverseTaylor{n}() | ||
| @generated function (::TaylorInverter{N})(var"ˍ₋arg1") where {N} |
There was a problem hiding this comment.
Is it necessary to switch from a regular function to a callable struct? If so, why exactly? And if not, why do you choose to do so anyway?
There was a problem hiding this comment.
It's not necessary, but it's the easiest way to get around to it.
|
|
||
| function create_expressions(n::Int) | ||
| it = InverseTaylor{n}() | ||
| @generated function (::TaylorInverter{N})(var"ˍ₋arg1") where {N} |
There was a problem hiding this comment.
GIven the fact that the @generated macro induces a behaviour that might surprise people, I would quite appreciate some comments being added to this function. In particular:
- A comment on the name of the argument (
var"_-arg1, not sure what the best place would be for this comment) - A comment that the return value of this function as read "naively" is not what the function will return, but a string that will be evaluated when the code is lowered. That is, if I understand correctly what happens with a
@generatedfunction, kind of proving that it would be wise to document this (briefly, I understand we should not rewrite the Metaprogramming page of the Julia Docs.) - Providing a docstring for this function that indicates the behaviour of the generated function. One of the (many) joys of Julia is it's innate readability, but with a generated function, this is somewhat hampered, for instance by the obscure argument name.
There was a problem hiding this comment.
- A comment on the name of the argument (
var"_-arg1, not sure what the best place would be for this comment)
It's the (Symbolics.jl) mangled name of the symbolic argument which is defined by this package. The argument name is the same as the argument name of the function that you eval'd
2. A comment that the return value of this function as read "naively" is not what the function will return, but a string that will be evaluated when the code is lowered.
The generator returns an AST.
obscure argument name.
The argument name is defined by this package when choosing the symbolic Num - if you change that, you get whatever symbol you want.
|
P.S. I am also quite curious why you wanted a PR here. Are you using the package, or did you find it via some other way? |
PkgEval flagged it for world age issues on 1.12 |
2 participants
Since the lowered code being generated does not have external inputs, a generated function is preferred. Otherwise, it can be tricky to deal with the world age issues. As it stands, an
invokelatestis required in several cases (and several more in 1.12 - seeJuliaLang/julia#56509).