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

Make contains_std_vector variadic. #999

Merged
merged 3 commits into from
Aug 24, 2018

Conversation

cqfd
Copy link
Contributor

@cqfd cqfd commented Aug 23, 2018

Summary

This is a small change that reimplements the contains_std_vector metafunction with variadic templates. The previous implementation used a helper metafunction, is_std_vector, but I think it's a little simpler to do that logic directly in contains_std_vector. So, I've also removed is_std_vector and its tests (they weren't testing anything very interesting).

These changes are along the lines of things we've been discussing on Discourse: https://discourse.mc-stan.org/t/stan-math-library-c-code-modernization-ideas/5293/8.

I didn't go this far in this PR, but the pattern of checking if anything in a parameter pack satisfies some type-level predicate (in this case, if something is a std::vector) presumably comes up a lot, and could be handled in a more general way, e.g. https://gist.github.com/cqfd/aae7a1d0721279e5e1e04ad8a7205d62

Tests

No new tests (in fact, fewer tests). There are a few tests that used to reference is_std_vector, which I've tweaked to use contains_std_vector (it's variadic, so it handles a single argument like is_std_vector just fine).

Side Effects

None.

Checklist

  • Math issue #(issue number)

  • Copyright holder: (fill in copyright holder information)

    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/)

  • [ x] the basic tests are passing

    • unit tests pass (to run, use: ./runTests.py test/unit)
    • header checks pass, (make test-headers)
    • docs build, (make doxygen)
    • code passes the built in C++ standards checks (make cpplint)
  • the code is written in idiomatic C++ and changes are documented in the doxygen

  • [x ] the new changes are tested

@seantalts seantalts self-requested a review August 23, 2018 14:23
@seantalts
Copy link
Member

cc @bob-carpenter @bbbales2 - I like this new style! further examples of modernization.

@syclik
Copy link
Member

syclik commented Aug 23, 2018

Sweet! This really does make it a lot cleaner. Thanks!

@seantalts seantalts merged commit 30ca7c1 into stan-dev:develop Aug 24, 2018
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

4 participants