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

Some rewritings diverge instead of stopping on a recursion limit #3

Closed
LPTK opened this issue Dec 2, 2016 · 2 comments
Closed

Some rewritings diverge instead of stopping on a recursion limit #3

LPTK opened this issue Dec 2, 2016 · 2 comments
Milestone

Comments

@LPTK
Copy link
Member

LPTK commented Dec 2, 2016

See:

scala> ir"val x = 42; println(x.toDouble)" fix_rewrite { case ir"($n:Int).toDouble" => ir"$n.toDouble+1" }
java.lang.StackOverflowError

on the other hand, this one does not exhibit the problem:

scala> ir"val x = 42; println(x.toDouble)" fix_rewrite { case ir"($n:Int).toDouble" => ir"$n.toDouble" }
Rewrite rules did not converge after 8 iterations.
ir"""{
  val x_0 = 42;
  scala.Predef.println(x_0.toDouble)
}"""
@LPTK
Copy link
Member Author

LPTK commented Sep 20, 2017

Note: the problem also appears when rewrite is used instead of fix_rewrite.
This is because the term grows in size and thus the top-down transformation never finishes.

@LPTK LPTK changed the title Some fixed point rewritings diverge instead of stopping on a recursion limit Some rewritings diverge instead of stopping on a recursion limit Sep 20, 2017
@LPTK
Copy link
Member Author

LPTK commented Oct 8, 2017

A good fix would be to make rewrite use a bottom-up traversal by default, which does not have this problem. Top-down transformations are either more dangerous (if they are allowed to recurse into freshly-rewritten terms) or less powerful in term of expressiveness (if they are not).

@LPTK LPTK added this to the Version 0.2 milestone Oct 8, 2017
LPTK added a commit that referenced this issue Dec 12, 2017
Bottom-up rewriting is safer (less prone to divergence) and often more 
natural, so it’s a better default behavior.

Note that this is potentially a breaking change, though I expect that 
most code that uses `rewrite` will still work.

Currently some tests are broken; they will be fixed in the next commit.
@LPTK LPTK closed this as completed in 8d98f22 Dec 12, 2017
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

No branches or pull requests

1 participant