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

Fix compilation and runtime issues on windows #130

Merged
merged 2 commits into from
Aug 2, 2022
Merged

Conversation

BlackMark29A
Copy link

@BlackMark29A BlackMark29A commented Aug 2, 2022

This PR is based on #129, so that one must be merged first.

Some code changes in past PRs have introduced code that no longer compiles on windows, this PR remedies that by adding missing headers and some minor code tweaks.

Additionally, PR #106 refactored the unit tests, specifically changing when MPI_Init is called relative to when Catch2 is initialized. This surfaced an issue where MSMPI changes the top-level exception filter, after Catch2 has already changed it, resulting in a sanity check inside Catch2 failing, where it checks that the previous filter was restored correctly. This PR disables windows SEH for Catch2, mitigating the issue.

@BlackMark29A BlackMark29A requested a review from fknorr August 2, 2022 10:15
Copy link
Contributor

@PeterTh PeterTh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@github-actions
Copy link

github-actions bot commented Aug 2, 2022

clang-tidy review says "All clean, LGTM! 👍"

@github-actions
Copy link

github-actions bot commented Aug 2, 2022

clang-tidy review says "All clean, LGTM! 👍"

1 similar comment
@github-actions
Copy link

github-actions bot commented Aug 2, 2022

clang-tidy review says "All clean, LGTM! 👍"

@BlackMark29A BlackMark29A merged commit f65a854 into master Aug 2, 2022
@BlackMark29A BlackMark29A deleted the windows-fixes branch August 2, 2022 11:21
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.

3 participants