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

[BUGFIX] Replace STL regex with Boost for MinGW bug #3251

Merged
merged 2 commits into from
Jan 18, 2024
Merged

Conversation

andrjohns
Copy link
Contributor

@andrjohns andrjohns commented Jan 15, 2024

Submission Checklist

  • Run unit tests: ./runTests.py src/test/unit
  • Run cpplint: make cpplint
  • Declare copyright holder and open-source license: see below

Summary

Depending on the user's locale, using std::regex with the MinGW GCC in RTools will result in a memory leak and the program hangs.

This has previously been reported as bug to MinGW here, but it is still an issue.

For Stan, this can cause a hang when parsing JSON data (seen recently as the cause of this downstream issue). We can instead use the Boost Regex headers as a drop-in replacement, which I've verified resolve the issue.

Intended Effect

Avoid hanging under certain locales

How to Verify

Side Effects

N/A

Documentation

N/A

Copyright and Licensing

Please list the copyright holder for the work you are submitting (this will be you or your assignee, such as a university or company): Andrew Johnson

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the following licenses:

@andrjohns
Copy link
Contributor Author

GCC bug listing here as well: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98723

Copy link
Member

@WardBrian WardBrian left a comment

Choose a reason for hiding this comment

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

I think we would be fine to include this in a 2.34.1 if we end up needing one, so it should be good to merge now.

Thanks!

@andrjohns
Copy link
Contributor Author

Awkward, but it looks like I don't have permissions for stan:

image

Can I get a grown-up to merge this for me?

@syclik syclik merged commit 9c30bdd into develop Jan 18, 2024
3 checks passed
@syclik syclik deleted the regex-bugfix branch January 18, 2024 11:41
@syclik
Copy link
Member

syclik commented Jan 18, 2024

@andrjohns, merged! That looks like an oversight.

@WardBrian, can we give @andrjohns permissions?

@WardBrian
Copy link
Member

So far as I can tell he has them:
image

Maybe it was a github hiccup?

@syclik
Copy link
Member

syclik commented Jan 18, 2024

Thanks for checking! I hope it was. @andrjohns please let people know if it happens again!

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.

None yet

3 participants