Repository navigation
C++20 updates - #25
Conversation
|
closes #24 |
jeandet
left a comment
jeandet
left a comment
There was a problem hiding this comment.
Thanks, nice cleanup. I built it locally with GCC 16 and Clang 22, and all tests pass.
The coverage job fails, though. It now builds with CXX=clang++, but lcov reads the coverage data with GCC's gcov, which rejects Clang's format (Incompatible GCC/GCOV version found). Since Clang already has its own CI job, could we drop CXX=clang++ there? If you want Clang for coverage, lcov --gcov-tool pointing at llvm-cov gcov should also work.
| using Ts::operator()...; | ||
| }; | ||
| template<typename... Ts> | ||
| Visitor(Ts...) -> Visitor<Ts...>; |
There was a problem hiding this comment.
I think this guide is unused, since make_visitor below spells out Visitor<Ts...>. Can we drop it?
Also minor, on make_visitor: Visitor<Ts...>(lambdas...) builds an aggregate with parentheses, which only works from Clang 16. Clang 10–15 handle the concepts fine but fail on that line. Using braces (Visitor<Ts...>{lambdas...}) avoids that, unless we're fine with Clang 16 as the minimum.
| } | ||
| } // namespace | ||
| template<typename T1, typename... T2> | ||
| concept any_of = (std::same_as<T1, T2> || ...); |
There was a problem hiding this comment.
Nit: this used to be in an anonymous namespace and is now public as cppdict::any_of. If a user has both using namespace std; and using namespace cppdict;, an unqualified any_of(...) call becomes ambiguous with std::any_of. Maybe move it into detail?
|
the coverage build has been failing for a while so I'm not sure it was introduced here I can try drop clang tho, not sure it's necessary |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #25 +/- ##
==========================================
- Coverage 96.47% 92.10% -4.38%
==========================================
Files 3 1 -2
Lines 284 114 -170
==========================================
- Hits 274 105 -169
+ Misses 10 9 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
48dae68
into
LaboratoryOfPlasmaPhysics:master
No description provided.