-
-
Notifications
You must be signed in to change notification settings - Fork 48
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
Fix two issues with tuple functions #1356
Conversation
1. Accidentally returning a reference 2. Integer arguments failed under certain circumstances, are now templated
Codecov Report
@@ Coverage Diff @@
## master #1356 +/- ##
==========================================
+ Coverage 89.39% 89.44% +0.04%
==========================================
Files 65 65
Lines 10607 10644 +37
==========================================
+ Hits 9482 9520 +38
+ Misses 1125 1124 -1
|
IMO this will justify a 2.33.1, but the nice thing is the only thing that would be different between the two versions is the Stanc executables. |
Great catch. Do we wait for a few days or just do the patch release? |
@SteveBronder and I still want to take a slightly deeper look at this, so it might be nice to also see if anything else comes in in the next couple days |
Ok I think this is finally ready for review @SteveBronder It's a bit tricky but honestly I think I like the code better now than before. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two minor comments but imo looks good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm!
@rok-cesnovar @serban-nicusor-toptal I think we should try to do 2.33.1 in the middle of next week, giving a few more days for new issues to come in. At the moment it would just be the 2.33.0 branches from all the repos except for stanc3 |
These were found because the Stan tests were raising a
-Wreturn-stack-address
warning. We should try to catch those types of things in stanc going forward.Submission Checklist
Release notes
Fixed a few issues with the C++ generated for functions which accept tuples as arguments.
Copyright and Licensing
By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the BSD 3-clause license (https://opensource.org/licenses/BSD-3-Clause)