Fix clang analyzer warning about FilterGenerator in v2.x #2314
Conversation
Codecov Report
@@ Coverage Diff @@
## v2.x #2314 +/- ##
==========================================
+ Coverage 90.09% 90.10% +0.01%
==========================================
Files 113 113
Lines 5035 5038 +3
==========================================
+ Hits 4536 4539 +3
Misses 499 499 |
| @@ -51,7 +51,7 @@ namespace Generators { | |||
|
|
|||
|
|
|||
| template <typename T, typename Predicate> | |||
| class FilterGenerator : public IGenerator<T> { | |||
There was a problem hiding this comment.
This is, very very hypothetically, an API breaking change.
I think it is very low risk, but there is a bit uglier but guaranteed non-breaking change that also fixes this: turn the current implementation of next into a private helper, e.g. nextImpl, and have both the constructor and the new next implementation call it directly, e.g.
bool next() override {
return nextImpl();
}
There was a problem hiding this comment.
This is, very very hypothetically, an API breaking change.
I think it is very low risk, but there is a bit uglier but guaranteed non-breaking change that also fixes this: turn the current implementation of
nextinto a private helper, e.g.nextImpl, and have both the constructor and the newnextimplementation call it directly, e.g.
Thanks for pointing out that important consideration. I think this is your call. Would you like me to revise this PR for the nextImpl route?
There was a problem hiding this comment.
Yes, sorry for making it implicit only.
I would merge a PR that adds a test or two (e.g. the ones I added to devel recently) and goes the nextImpl route.
Refactor FilterGenerator to remove ctor call to overridden method next()
in order to address clang static analyzer diagnostic:
catch2-src/single_include/catch2/catch.hpp:4166:42: note: Call to virtual method 'FilterGenerator::next' during construction bypasses virtual dispatch
auto has_initial_value = next();
^~~~~~
PR #2313 backported to v2.x since that's the one I'm actually using.