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

Do not link /latomic on MSVC #14564

Merged
merged 2 commits into from
Apr 20, 2024
Merged

Conversation

JosiahWI
Copy link
Contributor

MSVC does not recognize /latomic.

To do

Ready for Review.

How to test

Compile on MSVC and observe that there is no longer any warning about an unrecognized /latomic option.

MSVC does not recognize /latomic.
@JosiahWI JosiahWI marked this pull request as ready for review April 20, 2024 14:12
@sfan5
Copy link
Member

sfan5 commented Apr 20, 2024

We had this before and it didn't work, see #14160

@JosiahWI
Copy link
Contributor Author

Perhaps we should check whether some code compiles without the -atomic flag, and add the flag if that fails?

@JosiahWI
Copy link
Contributor Author

Note that this is not equivalent to what was there before. This change is safe because it still does the compile check.

@sfan5
Copy link
Member

sfan5 commented Apr 20, 2024

If you want to avoid a warning on MSVC I'd rather skip this part on MSVC, instead of a whitelist approach.

@sfan5 sfan5 added the @ Build CMake, build scripts, official builds, compiler and linker errors label Apr 20, 2024
@JosiahWI JosiahWI changed the title Only link -atomic on Clang and GCC Do not link /latomic on MSVC Apr 20, 2024
@sfan5 sfan5 added the Trivial The change is a trivial bug fix, documentation or maintenance change, as per the Git Guidelines label Apr 20, 2024
@SmallJoker SmallJoker merged commit de1d8ec into minetest:master Apr 20, 2024
13 checks passed
@JosiahWI JosiahWI deleted the fix/msvc-latomic branch April 22, 2024 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@ Build CMake, build scripts, official builds, compiler and linker errors One approval ✅ ◻️ Trivial The change is a trivial bug fix, documentation or maintenance change, as per the Git Guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants