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

BasicTransformer exponential complexity #125

Closed
note opened this issue Dec 17, 2016 · 6 comments
Closed

BasicTransformer exponential complexity #125

note opened this issue Dec 17, 2016 · 6 comments

Comments

@note
Copy link

note commented Dec 17, 2016

I am using 1.0.6. I just pasted code from https://issues.scala-lang.org/browse/SI-4528, namely:

var i = 0
  def translate(text: String): String = {
    println(i + " -> " + text)
    i += 1

    return "!%s!".format(text)
  }

  val xmlNode = <a><b><c><h1>Hello Example</h1></c></b></a>;

  new scala.xml.transform.RuleTransformer(new scala.xml.transform.RewriteRule {
    override def transform(n: Node): Seq[Node] = n match {
      case t: Text if !t.text.trim.isEmpty => Text(translate(t.text.trim))
      case _ => n
    }
  }).transform(xmlNode)

and the output is:

0 -> Hello Example
1 -> Hello Example
.
. (lines between left empty for sake of brevity)
30 -> Hello Example
31 -> Hello Example

It may seem it was covered by #59 but either in fact it does not or some regression was introduced.

@ashawley
Copy link
Member

Seems it wasn't fixed in b2eef1f, see below. I had written a test in #88, but I guess it wasn't a good one. Can the test be fixed somehow to verify the bug still exists?

$ git checkout -b pr-58 b2eef1f
Switched to a new branch 'pr-58' 
$ sbt
> console
Welcome to Scala version 2.11.6 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_40).
Type in expressions to have them evaluated.
Type :help for more information.

scala> :paste
// Entering paste mode (ctrl-D to finish)

import scala.xml._

var i = 0;
def translate(text: String): String = {
    println(i + " -> " + text)
    i += 1
 
    return "!%s!".format(text)
}
 
val xmlNode = <a><b><c><h1>Hello Example</h1></c></b></a>;
 
new transform.RuleTransformer(new transform.RewriteRule {
    override def transform(n: Node): Seq[Node] = n match {
        case t: Text if !t.text.trim.isEmpty => Text(translate(t.text.trim))
        case _ => n
    }
}).transform(xmlNode);

// Exiting paste mode, now interpreting.

0 -> Hello Example
1 -> Hello Example
...
30 -> Hello Example
31 -> Hello Example
import scala.xml._
i: Int = 32
translate: (text: String)String
xmlNode: scala.xml.Elem = <a><b><c><h1>Hello Example</h1></c></b></a>
res0: Seq[scala.xml.Node] = <a><b><c><h1>!Hello Example!</h1></c></b></a>

@note
Copy link
Author

note commented Dec 18, 2016

OK, I found explanation. So bug is in fact gone but it take me some time to see correct behavior.

  1. I pulled master and run example code on that code - I saw undesired 32 Hello Example messages.
  2. I started to debug and then I noticed that debugger actually uses files from 1.0.5 jar.
  3. I reminded myself that scala 2.12.1 pulled the most recent scala-xml. I changed build.sbt to use scala 2.12.1.
  4. Now I saw desired behavior (just 1 message).

So because scala-xml is a dependency of scala-compiler it brings it to classpath and since 2.12.0-RC1 used 1.0.5 http://mvnrepository.com/artifact/org.scala-lang/scala-compiler/2.12.0-RC1 I was seeing incorrect behavior.

There's still open question for users of 2.11.8 how to get 1.0.6. I got scala-xml 1.0.6 explicitly in my build.sbt but it's not enough and I am little afraid it may be hard to accomplish because sbt may treat scala-compiler differently and simple exclude will not work.

To sum up - you can close the issue. Maybe scala version in build.sbt should be bumped to 2.12.1.

@note
Copy link
Author

note commented Dec 18, 2016

OK I guess I can find an answer in scala-xml's itself build.sbt (#20)

@SethTisue
Copy link
Member

Maybe scala version in build.sbt should be bumped to 2.12.1

I have submitted #127 on this

@SethTisue
Copy link
Member

There's still open question for users of 2.11.8 how to get 1.0.6.

a PR adding appropriate documentation on this to the README would be very welcome.

@ashawley
Copy link
Member

@SethTisue I agree. The documentation could be better. The link to the example cross-building project is not enough.

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

3 participants