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

Allow genexprs in @cfunc functions. #459

Closed
wants to merge 1 commit into from
Closed

Allow genexprs in @cfunc functions. #459

wants to merge 1 commit into from

Conversation

memeplex
Copy link
Contributor

This fails to compile:

@cython.cfunc
def f():
    ','.join(x for x in '123')

The problem is that the cfunc directive forces an AdjustDefByDirectives visit. This is ok for the f node but not for the genexpr DefNode (i.e. node.body.stats[0].expr.args[0].def_node, where node is the f node) which inherits the cfunc directive from f and is also (but wrongly) transformed to a CFuncDefNode.

I'm not sure whether there is a valid use case for inheriting cfunc or not. The current implementation just set the visitor directives attribute to the directives in a CompilerDirectivesNode, so the same directives are active for the entire recursive descent. An alternative is to pop the directive after applying it, in cases for which directive inheritance is undesirable. This PR does what I've just described, but only for cfunc. I consider it a dirty hack until more general criteria are discussed and adopted (for example, it seems inconsistent to me doing this for cfunc and not for ccall).

@scoder
Copy link
Contributor

scoder commented Nov 13, 2015

This doesn't look like the best fix, but
a) this is definitely a bug, thanks for your investigation
b) it's even more general as it also applies to other "directives" that shouldn't get inherited

scoder added a commit that referenced this pull request Sep 22, 2018
…ction or class and those that are generally inherited. Everything that is not inherited should also not have a default value and instead exist or not.

Then, prevent lambdas and generator expressions from inheriting directives from their outer function/scope, e.g. "@cython.cdef".
Closes #459.
@scoder scoder added this to the 0.29 milestone Sep 22, 2018
@scoder scoder closed this in d89862f Sep 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants