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

Optimizer seems too restricted #733

Closed
davidism opened this issue Jul 5, 2017 · 2 comments · Fixed by #1111
Closed

Optimizer seems too restricted #733

davidism opened this issue Jul 5, 2017 · 2 comments · Fixed by #1111
Milestone

Comments

@davidism
Copy link
Member

@davidism davidism commented Jul 5, 2017

While looking at #476, I noticed that Optimizer only folds certain nodes. For example, Concat nodes are not optimized, but should be. Since all it's doing is trying to call node.as_const, and all Expr nodes define as_const and raise Impossible appropriately, can we safely make fold apply to all nodes that define as_const? Something like:

def generic_visit(self, node):
    if hasattr(node, 'as_const'):
        try:
            return self.fold(node)
        except Impossible:
            pass

    return super().generic_visit(node)

@mitsuhiko

@mitsuhiko

This comment has been minimized.

Copy link
Member

@mitsuhiko mitsuhiko commented Jul 5, 2017

It looks sane but I'm not quite sure if there are some unintended consequences to this. As far as I know it should not break anything or it was not sound before either.

@davidism

This comment has been minimized.

Copy link
Member Author

@davidism davidism commented Jul 5, 2017

@mitsuhiko thanks, just wanted to check that there weren't any obvious problems or design reasons before going forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants
You can’t perform that action at this time.