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

Stack overflow on macOS with a big source-file #2485

Closed
njlr opened this issue Sep 7, 2022 · 3 comments
Closed

Stack overflow on macOS with a big source-file #2485

njlr opened this issue Sep 7, 2022 · 3 comments

Comments

@njlr
Copy link
Contributor

njlr commented Sep 7, 2022

Note: I am unable to reproduce this issue in the online tool, or indeed on Linux.

Running a check format on this source-file causes a stack overflow on macOS, but not on Linux.

The full error message can be seen here: https://github.com/njlr/bazel-fantomas/runs/8233614713?check_suite_focus=true

On Linux the formatting check passes (as expected).

The version of Fantomas is 5.0.0-beta-009

@nojaf
Copy link
Contributor

nojaf commented Sep 8, 2022

Hello,

It appears

let rec (|Sequentials|_|) =
function
| Sequential (e, Sequentials es, _) -> Some(e :: es)
| Sequential (e1, e2, _) -> Some [ e1; e2 ]
| _ -> None

is not tail recursive.

I would accept a PR if you can make it so.

@njlr
Copy link
Contributor Author

njlr commented Sep 9, 2022

Here we go: #2495

I actually went with an imperative solution for easier optimization - F# has no [<RequireTailCallOptimization>] attribute AFAIK.

njlr added a commit to njlr/fantomas that referenced this issue Sep 9, 2022
nojaf added a commit that referenced this issue Sep 10, 2022
* * Switches Sequentials to an imperative impl for efficiency and to not blow up stack

* * Further simplifications

* * Switches to tail recursion
 * Adds unit-test

* * Adds note on #2485 to changelog

* Add release notes for beta 10.

Co-authored-by: Florian Verdonck <florian.verdonck@outlook.com>
@njlr
Copy link
Contributor Author

njlr commented Sep 11, 2022

Fixed in 5.0.0-beta-010

@njlr njlr closed this as completed Sep 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants