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

Don't accidently include both math.msvc.inl and math.gcc_builtin.inl. #1156

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jeffdav
Copy link

@jeffdav jeffdav commented Oct 11, 2024

Issue #, if available: #1155

Description of changes:
Add additional check to make sure we're not including both math.msvc.inl and math.gcc_builtin.inl.

Note: I'm not 100% sure this is the most correct solution, so I'd love feedback and I'm happy to iterate.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@waahm7
Copy link
Contributor

waahm7 commented Dec 11, 2024

Looking into AWS_HAVE_MSVC_INTRINSICS_X64, it will be only set if compiler is MSVC. We should not be setting MSVC and (GCC or Clang) at the same time.

@jeffdav
Copy link
Author

jeffdav commented Dec 11, 2024

I agree. I think the problem here is it gets built with MSVC, because I use your CMakeLists via ExternalProject. But we're not using the C modules directly; we're using AwsSdkSwift. And when I build that with CMake as part of the rest of the project the headers get imported as a module by clang, and that's where I start getting duplicate symbol errors.

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.

2 participants