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

deprecate sum::small and sum::large as workaround for bug in windows.h #343

Merged
merged 8 commits into from
Sep 30, 2021

Conversation

HDembinski
Copy link
Collaborator

Closes #342

This is based on the discussion in #341, but without immediately breaking the API. Instead, the old API is deprecated in favor of an API that works around the issue.

We should not have to work around Microsoft violating the C++ standard, but here we are.

@HDembinski
Copy link
Collaborator Author

@emmenlau FYI

@HDembinski HDembinski merged commit 7ec89f4 into develop Sep 30, 2021
@HDembinski HDembinski deleted the workaround_small_define branch September 30, 2021 09:20
@emmenlau
Copy link

Oh my, I'm so sorry it had to come to this! But its highly appreciated and will possibly help many users in the long run. Thanks a lot @HDembinski for all your effort!

@henryiii
Copy link
Contributor

henryiii commented Sep 30, 2021

Good workaround for an unfortunate issue. In boost-histogram, should we move to using the new names or keep the old names (since they are just strings in pybind11, and Python obviously supports large and small)? The old names are shorter and easier to remember/use in the REPL, etc, but the longer names match Boost.Histogram better. (and the sum accumulator doesn't have much use except for teaching about accumulators or for doing a manual highly-accurate accumulation, where most users won't need the two parts). We could even provide the official names, but have the short names as non-deprecated shortcuts if you want.

HDembinski added a commit that referenced this pull request Nov 15, 2021
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.

windows.h illegally uses #define small char, explore possible work-arounds
3 participants