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

Replace Boost PP_Iterate by C++11 variadic templates #110

Merged
merged 7 commits into from
Feb 16, 2022

Conversation

Flamefire
Copy link
Collaborator

@Flamefire Flamefire commented Feb 3, 2022

This is basically the continuation of #88

It allows support for (almost) any number of arguments and makes setting MOCK_MAX_ARGS and MOCK_MAX_SEQUENCES unnecessary. The only remaining limitation is imposed by Boost.Preprocessor in BOOST_PP_REPEAT which has a large limit (e.g. 256 in 1.69)
It also allows for easier debugging due to being able to step into actual code instead of preprocessor generated stuff

Not sure how well git recognizes this but I basically removed the suffix from the *_template.hpp files and replaced the BOOST_PP_REPEAT/BOOST_PP_ENUM macros by variadic templates and parameter pack expansions.

It also cuts down on the use of macros which should make the code a bit cleaner and easier to follow. Especially in regard to r-value-references and move-only types (unique_ptr...) I suspect a few issues. I'll also include a few cleanups with template aliases I noticed while doing this, e.g. typename arg_type<...>::type

Copy link
Owner

@mat007 mat007 left a comment

Choose a reason for hiding this comment

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

Less preprocessor is always good 😁

test/test_max_args.cpp Outdated Show resolved Hide resolved
@Flamefire Flamefire force-pushed the remove_pp_iterate branch 3 times, most recently from 28fcea2 to 167caa2 Compare February 5, 2022 19:28
@Flamefire Flamefire marked this pull request as ready for review February 7, 2022 17:49
Repository owner deleted a comment from coveralls Feb 7, 2022
@Flamefire Flamefire force-pushed the remove_pp_iterate branch 2 times, most recently from ab8f70b to 1b7bcfc Compare February 9, 2022 12:57
@Flamefire Flamefire mentioned this pull request Feb 9, 2022
@Flamefire Flamefire marked this pull request as draft February 9, 2022 15:00
This allows support for any number of arguments and makes setting MOCK_MAX_ARGS unnecessary.
It also allows for easier debugging due to being able to step into actual code instead of preprocessor generated stuff
MSVC has issues with a tuple of references of incomplete classes and `virtual ~value() = default;`
Some compilers warn for constant expressions in the for-loop-condition
Shortens the call sites a lot: `typename ref_arg<Ts>::type` -> `ref_arg_t<Ts>`
MSVC 2017 seems to have issues with std::conditional_t so use the C++11
variant here.
This removes the need for those preprocessor macros and the MOCK_MAX_SEQUENCES define.
Chaining is done via the wrapper class not the expectation class
@Flamefire Flamefire marked this pull request as ready for review February 10, 2022 16:51
@Flamefire
Copy link
Collaborator Author

Flamefire commented Feb 10, 2022

Rebased after #112, now only 7 commits with changed line stats of "+440 −3,924" :D

Ok, most of the removed lines are in the benchmark files which are no longer required, but still: Less code ;-)
And:

Coverage increased (+0.01%) to 99.592%

@Flamefire
Copy link
Collaborator Author

@mat007 After this is merged, I'd prepare a release 2.0.0. The last release is from 19 June 2020, so it's about time... ;-)
And it needs to be a new major version due to the breaking changes. It might also serve as a proof that this great project is still alive!

I'd probably include fixes for the (small) issues #94, #81 and check #69 (comment) (might also be a small breaking change, not sure)

Support for method qualifiers (noexcept, rvalue, lvalue, ...) and override will need to wait for the next version or so.

I'd also like to rename the MOCK_METHOD*_EXT macros. They are basically the non-variadic form of MOCK_METHOD* and hence no longer required as the variadic form is basically exactly the same. I'd go for MOCK_METHOD_IMPL, for "implementation"
Reason is, that for the qualifiers, I'd use that name and hence want to "free" it in this release, see the idea at #48 (comment) which could become something like:

MOCK_METHOD(name, cardinality, opt. list of specifiers, opt. id) -> for each entry in the specifier list an overload is generated
MOCK_METHOD_EXT(name, cardinality, signature, id, opt. list of specifiers) -> similar

Details don't really matter. Basically: These macros are no longer required and (now) undocumented similar to the TPL-macros, so I'd remove them while we're at it, so we can reuse the name later on for maybe something really different.

@Flamefire Flamefire merged commit 0dd0dfa into mat007:master Feb 16, 2022
@Flamefire Flamefire deleted the remove_pp_iterate branch February 16, 2022 15:04
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.

2 participants