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

Export jl_resolve_globals_in_ir in Julia src #32902

Merged
merged 10 commits into from
Oct 3, 2019

Conversation

NHDaly
Copy link
Member

@NHDaly NHDaly commented Aug 15, 2019

This is required (I think) in order for packages to fully implement their own generated-functions macros with custom behavior.

The motivating example for this is https://github.com/NHDaly/StagedFunctions.jl, which provides a complete, working reference implementation for generated functions that are recompiled whenever any of their dependencies change. However, currently it requires modifying Julia to export this function.

(Also, if we expose this function, and then also expose some kind of jl_apply_in_world_age, then we could actually move much of the implementation of Julia's generated functions into julia/base .jl files instead of in julia/src .c files!)

src/julia_internal.h Outdated Show resolved Hide resolved
src/method.c Outdated Show resolved Hide resolved
Instead of exporting the internal function jl_resolve_globals_in_ir,
this factors out the code from generated functions into
`jl_expand_and_resolve`, which lowers an expression into a CodeInfo and
then resolves globals in the codeinfo in light of provided typenames.

I then exposed this to Julia via another method for `Meta.lower()`, this
one taking type parameters.
…ecause it's very particular about its inputs
@NHDaly
Copy link
Member Author

NHDaly commented Sep 13, 2019

Okay, I've pushed up the change we discussed today, which exposes a ccall(:jl_expand_and_resolve, ...) :) Thanks!

@StefanKarpinski
Copy link
Sponsor Member

Bump. @vtjnash, can you review?

Copy link
Sponsor Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, should be pretty stable

src/method.c Show resolved Hide resolved
@NHDaly
Copy link
Member Author

NHDaly commented Oct 2, 2019

Okay, thanks for the review! :) I've updated the comments to my satisfaction, so I'm happy to have this merged whenever! Thanks @vtjnash!

@vchuravy
Copy link
Sponsor Member

vchuravy commented Oct 3, 2019

AnalyzeGC failure is real

@NHDaly
Copy link
Member Author

NHDaly commented Oct 3, 2019

woah, oops. Thanks Valentin!

I've not done too much of this stuff.. I'll read up on the docs and figure out how i'm supposed to fix this. :) ❤️ thanks!
EDIT: Done

(I think I did this right? I couldn't get the static analysis to build
on my machine...)
@NHDaly
Copy link
Member Author

NHDaly commented Oct 3, 2019

Cool, thanks @vchuravy! Fixed the test. (I think the test_win32 is a flake?)

@vchuravy vchuravy merged commit 1739ca0 into JuliaLang:master Oct 3, 2019
@NHDaly NHDaly deleted the export_jl_resolve_globals_in_ir branch October 3, 2019 15:11
@c42f
Copy link
Member

c42f commented Oct 5, 2020

and then also expose some kind of jl_apply_in_world_age

Cool. FWIW this was implemented in #35844

c42f added a commit to c42f/RuntimeGeneratedFunctions.jl that referenced this pull request Oct 5, 2020
Create method of `generated_callfunc` in the user's module so that any
global symbols within the body will be looked up in the user's module
scope.

This is straightforward but clunky.  A neater solution should be to
explicitly expand in the user's module and return a CodeInfo from
`generated_callfunc`, but it seems we'd need `jl_expand_and_resolve`
which doesn't exist until Julia 1.3 or so. See:

* JuliaLang/julia#32902
* https://github.com/NHDaly/StagedFunctions.jl/blob/master/src/StagedFunctions.jl#L30
@NHDaly
Copy link
Member Author

NHDaly commented Oct 5, 2020

(Also, if we expose this function, and then also expose some kind of jl_apply_in_world_age, then we could actually move much of the implementation of Julia's generated functions into julia/base .jl files instead of in julia/src .c files!)

and then also expose some kind of jl_apply_in_world_age

Cool. FWIW this was implemented in #35844

Oooh, very cool indeed! It might be fun to do that rewrite at some point! 😁

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

Successfully merging this pull request may close these issues.

None yet

5 participants