optimize toString slightly #2635
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2635 +/- ##
=======================================
Coverage 63.92% 63.92%
=======================================
Files 103 103
Lines 22306 22310 +4
Branches 10796 10797 +1
=======================================
+ Hits 14259 14262 +3
Misses 5827 5827
- Partials 2220 2221 +1
|
|
Hmmm after talking with ChatGPT, this is a bad idea on C++11. Wonder if we should remove if constexpr from public headers. |
mergify
bot
dismissed stale reviews from kevinbackhouse and kmilos
Pull request has been modified.
|
Updated to remove if constexpr. @kmilos I remember you saying there was some issue. Is it still present? |
I'll check. You still have it in types.hpp though... |
|
Needs some work to compile properly. I'll check later. The remaining one is in a big function. Doesn't make much sense to get rid of it there. |
Also get rid of if constexpr for C++11 compatibility. The else condition results in extra generated code as compilers are not free to promote if to if constexpr.
|
ping @kmilos |
|
Not seeing that constexpr error any longer w/ 0.28.x head and this PR on top of it, so good to go I guess. P.S. It's possible I was also missing -std=c++17 when I first tried? |
|
@kmilos the headers should be kept C++11 compatible in any case. As mentioned earlier, the if constexpr stuff should also be removed. |
|
@Mergifyio backport 0.28.x |
✅ Backports have been created
|
No description provided.