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

Disambiguate Windows run-times when using clang to compile. #825

Merged
merged 2 commits into from
Jul 21, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1619,6 +1619,13 @@ impl Build {
cmd.push_opt_unless_duplicate(format!("-O{}", opt_level).into());
}

if cmd.family == ToolFamily::Clang && target.contains("windows") {
// Disambiguate mingw and msvc on Windows. Problem is that
// depending on the origin clang can default to a mismatchig
// run-time.
cmd.push_cc_arg(format!("--target={}", target).into());
Copy link
Member

Choose a reason for hiding this comment

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

Does this still work if there's a mismatch between the clang target name and rust target name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To my knowledge, there is no mismatch between Rust and clang. Which shouldn't really come as surprise, both are llvm-based.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh! Maybe you refer to aarch64-pc-windows-llvm, or what was it called? Fair enough, I'll double-check...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh! Maybe you refer to aarch64-pc-windows-llvm,

It's *-pc-windows-gnullvm, i.e. not just aarch64, and there is "gnu" in the name. Surprisingly enough clang appears to accept gnullvm and even do arguably right thing. It appears to be due to a pattern matching, because it also accepts gnufoobar, gnu-foobar, and even msvcfoobar and msvc-foobar. And it generates code targeting mingw runtime if there is gnu somewhere in the target name, and vc runtime otherwise.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, clang accepts far more target triples than rustc, which only accepts a finite set (clang, due to compatibility with GCC, does quite a bit of massaging of the value).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To summarize, the answer to the original question is "yes, it still works."

Copy link
Member

Choose a reason for hiding this comment

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

We can presumably check that it continues to work with official targets. My only concern would be third party rustc targets (which can be named anything). But we can either say they aren't supported or require setting the target to a llvm compatible one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can presumably check that it continues to work with official targets.

That's what "yes, it still works" effectively mean. It still works with official targets.

}

if cmd.family == ToolFamily::Clang && target.contains("android") {
// For compatibility with code that doesn't use pre-defined `__ANDROID__` macro.
// If compiler used via ndk-build or cmake (officially supported build methods)
Expand Down Expand Up @@ -2541,6 +2548,10 @@ impl Build {
}
}

if target.contains("msvc") && tool.family == ToolFamily::Gnu {
println!("cargo:warning=GNU compiler is not supported for this target");
}

Ok(tool)
}

Expand Down