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

Add <numeric> header to resolve the MSVC building error #133

Merged
merged 1 commit into from
Nov 20, 2023

Conversation

lcheng9
Copy link
Contributor

@lcheng9 lcheng9 commented Nov 16, 2023

*Description of changes: add header to resolve the MSVC building error

EDIT: Includes #134 and #135 as well.

Issue #, if available: MS Visual Studio 2022 complains. needed for std::partial_sum and std::accumulate

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute
this contribution, under the terms of your choice.

@sebastiangrimberg sebastiangrimberg changed the base branch from sjg/release-0.12 to main November 16, 2023 23:20
@sebastiangrimberg sebastiangrimberg added bug Something isn't working build Related to building minor A minor issue or improvement labels Nov 16, 2023
@sebastiangrimberg
Copy link
Contributor

Thanks @lcheng9!

@sebastiangrimberg
Copy link
Contributor

Hi @lcheng9, can you make sure the latest commit compiles with MSVC correctly? I refactored the diagnostics into utils/diagnostic.hpp.

@lcheng9
Copy link
Contributor Author

lcheng9 commented Nov 20, 2023

Hi @sebastiangrimberg, Thank you so much for the refactoring. Your revision builds on MSVS, I just realized that I need to add the _MSC_VER check to prevent the impact on other Windows systems, such as Cygwin.

@sebastiangrimberg
Copy link
Contributor

Hi @sebastiangrimberg, Thank you so much for the refactoring. Your revision builds on MSVS, I just realized that I need to add the _MSC_VER check to prevent the impact on other Windows systems, such as Cygwin.

Great. I would suggest to change the check to just #if defined(_MSC_VER) for simplicity as it should capture all the needed cases for needing warning.

@lcheng9
Copy link
Contributor Author

lcheng9 commented Nov 20, 2023

Great. I would suggest to change the check to just #if defined(_MSC_VER) for simplicity as it should capture all the needed cases for needing warning.

_MSC_VER should be sufficient, since the i686 system probably not be used in the computing nowadays. The change is submitted. Thank you.

@sebastiangrimberg sebastiangrimberg merged commit 75ba549 into awslabs:main Nov 20, 2023
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working build Related to building minor A minor issue or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants