Skip to content

Replace std::min with ternary operators to avoid <algorithm> dependency#379

Merged
lemire merged 2 commits intofastfloat:mainfrom
mlippautz:patch-1
Apr 16, 2026
Merged

Replace std::min with ternary operators to avoid <algorithm> dependency#379
lemire merged 2 commits intofastfloat:mainfrom
mlippautz:patch-1

Conversation

@mlippautz
Copy link
Copy Markdown
Contributor

fastfloat_strncasecmp relies on std::min.

`fastfloat_strncasecmp` relies on `std::min`.
@mlippautz
Copy link
Copy Markdown
Contributor Author

Alternatively, maybe it's better to avoid std::min in first place to avoid the dependency.

@jwakely
Copy link
Copy Markdown
Contributor

jwakely commented Apr 16, 2026

Yes, <algorithm> is a large header, especially since all the range overloads got added in C++20.

@mlippautz
Copy link
Copy Markdown
Contributor Author

Do you prefer just adding a constexpr size_t fastfloat_min(size_t a, size_t b) or completely inlining the call?

Context: We'd like to roll to the latest fast_float version in Chrome and ideally avoid a downstream patch.

@biojppm
Copy link
Copy Markdown
Contributor

biojppm commented Apr 16, 2026

I agree that the include should be avoided due to its large size.

As for picking between a function or using the ternary operator (which is what I guess you mean by inlining the call?): I guess both are fine, but maybe the latter is preferable, given that std::min is scarcely used. But this is not a strong position.

Grepping for std::min in the source, I get

File: float_common.h
401:12:      sz = std::min(sz, length - i);

File: ascii_number.h
597:22:      nd = (uint32_t)std::min((size_t)nd, len);

File: digit_comparison.h
112:12:    cb(am, std::min<int32_t>(shift, 64));

@lemire
Copy link
Copy Markdown
Member

lemire commented Apr 16, 2026

@mlippautz Something like fast_float::details::min would be nice.

Replaces uses of std::min with ternary operators in ascii_number.h, digit_comparison.h, and float_common.h to remove the dependency on the <algorithm> header in those files.
@mlippautz mlippautz changed the title Include <algorithm> in float_common.h Replace std::min with ternary operators to avoid <algorithm> dependency Apr 16, 2026
@mlippautz
Copy link
Copy Markdown
Contributor Author

@mlippautz Something like fast_float::details::min would be nice.

Sorry, didn't see this comment until I now updated the PR. Shall we go with fast_float::details::min or the ternary operators now? :)

@lemire
Copy link
Copy Markdown
Member

lemire commented Apr 16, 2026

@mlippautz Ternary is fine.

@lemire
Copy link
Copy Markdown
Member

lemire commented Apr 16, 2026

Running tests. I expect them to pass.

@lemire
Copy link
Copy Markdown
Member

lemire commented Apr 16, 2026

The MINGW32 failure is unrelated.

@mlippautz
Copy link
Copy Markdown
Contributor Author

@lemire can you help with landing this?

Also, could you cut a release for this as well? We set up auto rollers now for this repository. As long as things are compatible we will semi automatically get the latest changes.

@lemire lemire merged commit b57ec06 into fastfloat:main Apr 16, 2026
37 of 38 checks passed
@lemire
Copy link
Copy Markdown
Member

lemire commented Apr 16, 2026

Yes, of course.

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.

4 participants