-
Notifications
You must be signed in to change notification settings - Fork 219
Fix Build Warnings on Windows #475
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
Conversation
ce1643a
to
f8e4c1b
Compare
06011f3
to
09e516c
Compare
09e516c
to
c76813d
Compare
- Properly link against msvcrt by not overriding the /MD in CMAKE_CXX_FLAGS_RELWITHDEBINFO - Rename libllbuild.lib back to llbuild.lib - Rename libllbuildSwift.lib back to llbuildSwift.lib - Disable incremental linking so llbuild.ilk's don't conflict - Set CRT_NONSTDC_NO_DEPRECATE to ignore _strdup warnings
c76813d
to
bf477a4
Compare
Rebased and addressed comments @swift-ci please smoke test |
"/EHsc") | ||
else () | ||
# Compile with C++14, without RTTI or exceptions. | ||
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-rtti -fno-exceptions") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two flags were left out of the list below, oversight? I ask because this change adds EH functions that breaks linking the Swift package manager on Android. I can always add these flags back specifically for Android, but I wonder if you really wanted to keep them here too.
Hrm no, I meant for those not to have any changes, I must have dropped them
by accident while rebasing :( You're welcome to add them back, else, I'll
put up a patch if I don't see one in a few days. Thanks for catching that!
…On Sat., Aug. 24, 2019, 12:42 a.m. buttaface, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In CMakeLists.txt
<#475 (comment)>:
> else ()
- # Compile with C++14, without RTTI or exceptions.
- set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-rtti -fno-exceptions")
These two flags were left out of the list below, oversight? I ask because
this change adds EH functions that breaks linking the Swift package manager
on Android. I can always add these flags back specifically for Android, but
I wonder if you really wanted to keep them here too.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#475>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABQPW6DWR3DRQLR3TFKG4LTQGDQ5RANCNFSM4HIW7N6A>
.
|
Done in #544, thanks for the quick answer. |
…n Windows These were originally added 6 years ago in swiftlang#475 to "Disable incremental linking so llbuild.ilk's don't conflict" because at the time the command line tool and library targets were given the same OUTPUT_NAME, which conflicted. In swiftlang#1005 these targets were given distinct names, so the workaround should no longer be necessary. This helps unblock swiftlang/swift-build#757 because this flag is currently added without regard for the specific compiler driver in use, so the flag might not work based on whether clang-cl.exe or cl.exe vs clang.exe (where it would require a -Xlinker prefix), is used.
…n Windows These were originally added 6 years ago in #475 to "Disable incremental linking so llbuild.ilk's don't conflict" because at the time the command line tool and library targets were given the same OUTPUT_NAME, which conflicted. In #1005 these targets were given distinct names, so the workaround should no longer be necessary. This helps unblock swiftlang/swift-build#757 because this flag is currently added without regard for the specific compiler driver in use, so the flag might not work based on whether clang-cl.exe or cl.exe vs clang.exe (where it would require a -Xlinker prefix), is used.
CMAKE_CXX_FLAGS_RELWITHDEBINFO