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

Drop unused code-paths associated with "if cython.compiled" early #3507

Merged
merged 4 commits into from Apr 13, 2020

Conversation

da-woods
Copy link
Contributor

@da-woods da-woods commented Apr 12, 2020

This allows things to work like:

# in pxd file
from libc.math cimport sin

# in py file
if not cython.compiled:
    from math import sin  # previously failed with cython compile
       # error because it was assigning to a cdef name

This seems worthwhile because it makes it easier to generate code
that re-assigns cdef names and so works in both modes.

For the sake of simplicitity it only handles very simple cases
and doesn't attempt a full exercise in constant folding at this
stage (hence if not cython.compiled and True isn't handled).
This seems like a sensible compromise.

This allows things to work like:
    # in pxd file
    from libc.math cimport sin

    # in py file
    if not cython.compiled:
        from math import sin  # previously failed with cython compile
           # error because it was assigning to a cdef name

This seems worthwhile because it makes it easier to generate code
that re-assigns cdef names and so works in both modes.

For the sake of simplicitity it only handles very simple cases
and doesn't attempt a full exercise in constant folding at this
stage (hence `if not cython.compiled and True` isn't handled).
This seems like a sensible compromise.
@scoder
Copy link
Contributor

scoder commented Apr 13, 2020

Wouldn't it also be enough to replace the expression with a BoolNode here (as currently done in TransformBuiltinMethods), and then leave the replacement to ConstantFolding ?

The travis build failures might be solvable with a rebase on master.

@da-woods
Copy link
Contributor Author

Updated as you suggested - I had it in my head that ConstantFolding happened much later in the compilation so wouldn't fix this, but actually it's fine for this.

The remaining build failure looks to be nothing to do with this.

Cython/Compiler/ParseTreeTransforms.py Outdated Show resolved Hide resolved
tests/compile/pure_ifclauses.py Outdated Show resolved Hide resolved
Cython/Compiler/ParseTreeTransforms.py Show resolved Hide resolved
@scoder
Copy link
Contributor

scoder commented Apr 13, 2020

Excellent. Thanks!

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