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

Board::get_piece_locations() causes stack overflow #117

Closed
AntonHermann opened this issue Oct 14, 2018 · 5 comments
Closed

Board::get_piece_locations() causes stack overflow #117

AntonHermann opened this issue Oct 14, 2018 · 5 comments

Comments

@AntonHermann
Copy link

Running Board::get_piece_locations() causes a stack overflow when run.
This is the code that causes this bug for me:
let board = Board::start_pos()
for (sq, piece) in board.get_piece_locations() {
debug!("{} {:?}", sq, piece); }`

@sfleischman105
Copy link
Collaborator

Thanks for the report.

The problem can be reduced to:

let board = Board::start_pos();
let piece_locations = board.get_piece_locations();
for (sq, _) in piece_locations {
   println!("{}", sq);
}

Something about printing SQ's causes a stack overflow... weird. Looking at it right now.

@AntonHermann
Copy link
Author

I've found the problem:
The Display implementation of SQ uses self.toString(), which doesn't call the toString method defined earlier, it calls the toString implementation implementet for everything that implements Display (through the ToString trait I believe, not entirely sure).
So renaming that or somehow directly specifying what method you mean should fix this
&SQ::to_string(self) instead of &self.to_string works

@AntonHermann
Copy link
Author

What's also blowing up the stack is that PieceLocationsIter::next() calls self.next() recursively if piece == Piece::None
On default starting pos this means 4*8 = 32 recursive calls. You should go for a while loop to get to the next piece instead.

@AntonHermann
Copy link
Author

I think I can make a PullRequest tomorrow or later this day, to fix these issueas

@sfleischman105
Copy link
Collaborator

Thanks for the insights, that seems to be the problem. I remade the toString method to index into a static array, and modified the Iterator for PieceLocations to loop instead of using recursion. A good TIL for today: Apparently Rust doesn't support tail-call recursion, causing stack explosions like this one.

The fix is currently in the Beta-Branch right now, and I'll merge it once CI tests pass.

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

2 participants