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

add optional argument to code_lowered to enable generator expansion #22979

Merged
merged 8 commits into from
Aug 20, 2017

Conversation

jrevels
Copy link
Member

@jrevels jrevels commented Jul 27, 2017

Attempt at fixing #22874. It works, but I'm not sure if this is how it should be implemented.

From Jeff's/Yichao's comments in that issue:

The problem is abstract types, for which we don't invoke generators. In those cases, we'd have to give an error (perhaps until we have fallback code for generated functions)

It should always be safe for @code_lowered though. For code_lowered I guess we can have arguments to control this (just like code_llvm) and pick a reasonable default behavior?

By default, I've disabled generator expansion for code_lowered and enabled it for @code_lowered. I guess the mismatch could be a point of confusion...

I'll add some tests once I get confirmation that this is going in the right direction.

@jrevels jrevels added the needs tests Unit tests are required for this change label Jul 27, 2017
asts = map(_methods_by_ftype(tt, -1, typemax(UInt))) do method_data
mtypes, msp, m = method_data
if isdefined(m, :generator)
instance = Core.Inference.code_for_method(m, mtypes, msp, typemax(UInt), false)
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

This will return nothing if mtypes isn't a leaf type.

@JeffBezanson
Copy link
Sponsor Member

I think expand_generated should default to true in both cases.

@jrevels
Copy link
Member Author

jrevels commented Aug 1, 2017

I think expand_generated should default to true in both cases.
This will return nothing if mtypes isn't a leaf type.

What should I do if expand_generated is true && isdefined(m, :generator) && mtypes isn't a leaf type? Should I just throw an error?

@JeffBezanson
Copy link
Sponsor Member

Yes, I think throwing an error is all you can do.

@jrevels
Copy link
Member Author

jrevels commented Aug 15, 2017

This is complete (pending further review) and tests are passing (except for a Travis failure on OSX, which AFAICT is unrelated).

@jrevels
Copy link
Member Author

jrevels commented Aug 18, 2017

I plan on merging this tomorrow afternoon barring any further change requests.

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

2 participants