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

Conditional type support #70

Merged

Conversation

cjvaughter
Copy link
Contributor

@cjvaughter cjvaughter commented Jun 7, 2022

Resolves non-locale problems listed in #69.

There are probably a few sections which do not require conditional compilation, as eliminating calls further up the chain would suffice. But I have not determined where best to do so, and this is easy to modify later.

After becoming more acquainted with the code, I found the correct location. I also decided that all types should fall into this setup, not just floating-point.

The one thing I'm not 100% sure of is the static_assert placement. I think the two of them cover everything, but it's a little hard to tell.

The size reduction for my project is 23K (no fallbacks; schar, uchar, long, ulong, bool, float, buffer, and custom enabled).

@codecov
Copy link

codecov bot commented Jun 7, 2022

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Files Coverage Δ
include/scn/detail/args.h 96.36% <ø> (ø)
src/reader_float.cpp 87.15% <0.00%> (ø)

📢 Thoughts on this report? Let us know!.

@cjvaughter cjvaughter changed the title Conditional float support Conditional type support Jun 8, 2022
@cjvaughter
Copy link
Contributor Author

Apologies for the churn. Modern C++ has gotten so much easier, I often forget how to do it the old way (and which old way goes with which standard).

Add compile-time options `SCN_TYPE_*` for all types,
and `SCN_USE_FROM_CHARS` and `SCN_USE_CSTD`, primarily for
compiled size reductions, and in case a platform doesn't support
some of them.
@eliaskosunen eliaskosunen force-pushed the feature-conditional-float branch 2 times, most recently from 11d5be1 to 7d1abbb Compare November 2, 2023 19:33
Invert the condition: SCN_TYPE_FOO -> SCN_DISABLE_TYPE_FOO
Also, define defaults in config.h (all enabled), and
don't add definitions in CMake if type not disabled.

As a drive-by fix, also improve some error messages in reader_float
@eliaskosunen eliaskosunen merged commit dd79497 into eliaskosunen:master Nov 2, 2023
107 of 108 checks passed
@eliaskosunen
Copy link
Owner

I inverted the conditions, so that the macros are SCN_DISABLE_TYPE_*. I think it's better that way, since the default with no extra flags given should be to support everything. I also modified the CMake scripts to only add these flags if a type is actually disabled, and added #define SCN_TYPE_* 0 to config.h if SCN_TYPE_* is not defined.

Other that than, looks good. Rebased, signed, and merged. Thank you!

@cjvaughter
Copy link
Contributor Author

Hey I'm just happy to have the options! Thanks for getting this cleaned up and merged.

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.

None yet

2 participants