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

support for closures via opaque closures #32

Merged
merged 3 commits into from
Apr 6, 2021

Conversation

simeonschaub
Copy link
Contributor

Should close #28. Kwargs or where parameters might not work, but that's an issue in Julia base.

Should close SciML#28. Kwargs or `where` parameters might not work, but that's an issue in Julia base.
@@ -44,9 +51,13 @@ end
"""
struct RuntimeGeneratedFunction{argnames, cache_tag, context_tag, id} <: Function
body::Expr
function RuntimeGeneratedFunction(cache_tag, context_tag, ex)
function RuntimeGeneratedFunction(cache_tag, context_tag, ex; opaque_closures=true)
Copy link
Member

Choose a reason for hiding this comment

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

doesn't this need to be set based on the Julia version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See the check just below. It's ignored if Base.Experimental.@opaque is not defined.

@ChrisRackauckas
Copy link
Member

Somehow the timing regression tests are failing now?

@codecov
Copy link

codecov bot commented Apr 6, 2021

Codecov Report

Merging #32 (2e6151e) into master (65d2451) will increase coverage by 4.34%.
The diff coverage is 97.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #32      +/-   ##
==========================================
+ Coverage   93.33%   97.67%   +4.34%     
==========================================
  Files           1        1              
  Lines          45       86      +41     
==========================================
+ Hits           42       84      +42     
+ Misses          3        2       -1     
Impacted Files Coverage Δ
src/RuntimeGeneratedFunctions.jl 97.67% <97.14%> (+4.34%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 65d2451...2e6151e. Read the comment docs.

@simeonschaub
Copy link
Contributor Author

Ah, I just forgot to add the keyword to the other method as well.

@ChrisRackauckas ChrisRackauckas merged commit 2dd4778 into SciML:master Apr 6, 2021
@ChrisRackauckas
Copy link
Member

Great thanks!

@simeonschaub simeonschaub deleted the sds/opaque branch April 6, 2021 11:03
@0x0f0f0f
Copy link

0x0f0f0f commented Apr 6, 2021

Thanks!

@shashi
Copy link
Contributor

shashi commented Apr 6, 2021

Thanks! This looks great! We needed this pretty badly.

@c42f
Copy link
Contributor

c42f commented Apr 7, 2021

Nice! So great that the compiler can now do most of the work for us here :)

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.

Closures in RuntimeGeneratedFunctions
5 participants