-
-
Notifications
You must be signed in to change notification settings - Fork 198
Add inline to all functions
#3245
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
Conversation
…ed only in on translation unit
Jenkins Console Log Machine informationNo LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu 20.04.3 LTS Release: 20.04 Codename: focalCPU: G++: Clang: |
|
@SteveBronder is there a lint or something we can enable to ensure these stay present over time/on new functions? |
|
We could write a clang tidy for it. Happy to do that but as a separate PR. I think we could write a test that validates this |
|
Clang-tidy already has I think we just aren't running clang-tidy in jenkins atm? |
|
I used to have clang tidy setup in Jenkins. Let me see how hard this is to add. I'd like to merge this sooner rather than later as this touches a lot of code and I'll have merge conflicts if this sits for too long |
|
I've made a new issue for it -- I'll scroll through here and merge if it all looks good |
WardBrian
left a comment
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.
Did a quick scroll through, all seems reasonable
Summary
Since Stan math is a header only library we want to make sure that if the stan math library is included in two separate translation units we do not have name conflicts across those units. #3068 builds all of our tests in cmake via many small translation units that are then merged into one large executable. But this can fail if the compiler has multiple definitions for functions across translation units.
This PR is just a clever series of greps to put
inlinein front of Stan math functions that did not have it. I've been meaning to do this for a while and with the cmake stuff close but needing this it felt like the right time.Tests
No new tests
Checklist
Copyright holder: Steve Bronder
The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
- Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
- Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)
the basic tests are passing
./runTests.py test/unit)make test-headers)make test-math-dependencies)make doxygen)make cpplint)the code is written in idiomatic C++ and changes are documented in the doxygen
the new changes are tested