Skip to content

Conversation

@kcsongor
Copy link
Contributor

@kcsongor kcsongor commented Jul 11, 2016

The code example in #1375 caused the REPL to get in an infinite loop, and eventually terminate in an out of memory error.

The cause of this was the way the / character was handled, or more precisely, the character following a /. Instead of peeking the next character, it was polled, and in case of a no match, was put back using the lazy stream cons, which doesn't play nicely with expressions of the form stream = n #:: stream (or I should say it can be surprising).

@smarter
Copy link
Member

smarter commented Jul 12, 2016

/rebuild

@smarter
Copy link
Member

smarter commented Jul 12, 2016

@kcsongor: Thanks for your contribution! Before we can accept your patch, you'll need to sign the Scala CLA at http://www.lightbend.com/contribute/cla/scala

@kcsongor
Copy link
Contributor Author

@smarter: thanks, I've signed it

var prev: Char = 0
var remaining = chars.toStream
val newBuf = new StringBuilder
val newBuf = new mutable.StringBuilder
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there is no immutable StringBuilder, this distinction feels slightly redundant.

@felixmulder
Copy link
Contributor

Otherwise LGTM

@kcsongor
Copy link
Contributor Author

I've made the suggested changes, not sure if/how I can squash the commits in the github web editor

@smarter
Copy link
Member

smarter commented Jul 12, 2016

You can't do it online, but you can do it locally with git and then push-force (git push -f): https://ariejan.net/2011/07/05/git-squash-your-latests-commits-into-one/

@kcsongor
Copy link
Contributor Author

I wasn't near my own computer at the time, and I was hoping that I can squash them quickly, but anyway, it should be good now

@felixmulder felixmulder merged commit bef40b4 into scala:master Jul 12, 2016
@felixmulder
Copy link
Contributor

Great, thanks for the contribution :)

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

Successfully merging this pull request may close these issues.

3 participants