Fix specialization of float16#654
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #654 +/- ##
==========================================
+ Coverage 88.64% 88.67% +0.02%
==========================================
Files 112 112
Lines 9048 9048
==========================================
+ Hits 8021 8023 +2
+ Misses 1027 1025 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
There is an issue with the integration test but it's not related |
25a16f1 to
16e1141
Compare
| // corresponding struct, so we need explicit specializations for them. | ||
| // Newer libc++ (LLVM 19+) marks these variable templates with | ||
| // [[no_specializations]], making user specializations ill-formed, so we | ||
| // guard against that here. The struct specializations above are sufficient |
There was a problem hiding this comment.
No, the struct specializations are forbidden by https://eel.is/c++draft/type.traits#meta.rqmts-4
There was a problem hiding this comment.
Indeed, we need to define those structures in the sparrow namespace, forward to the std definitions for standard types, and specialize them for half_float. The main fragility here is that we cannot ensure that clients of sparrow will use our traits instead of the standard ones; having a dedicated section about this in the documentation would probably help a bit.
JohanMabille
left a comment
There was a problem hiding this comment.
One objection I had at first sight was that we may want to always have the sparrow traits available, not only when SPARROW_STD_FIXED_FLOAT_SUPPORT is defined, so that the used does not have to bother with that macro and handle both cases. But this does not encourage to switch back to the standard traits on the long run (i.e. when we switch to C++23), so I finally think we should keep it as is.
And we should add a section in the documentation about this (both usage and rationale behind it).
No description provided.