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.

Sign up for GitHub

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

Jump to bottom

Only error about MSVC + PGO + unwind if we're generating code #62615

Merged
merged 2 commits into from
Jul 18, 2019
Merged

Only error about MSVC + PGO + unwind if we're generating code #62615

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
374daa7
Revert "Emit warning when trying to use PGO in conjunction with unwin…
wesleywiser Jul 12, 2019
3622311
Only error about MSVC + PGO + unwind if we're generating code
wesleywiser Jul 12, 2019
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
15 changes: 9 additions & 6 deletions src/librustc/session/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::lint;
use crate::lint::builtin::BuiltinLintDiagnostics;
use crate::middle::allocator::AllocatorKind;
use crate::middle::dependency_format;
use crate::session::config::{OutputType, SwitchWithOptPath};
use crate::session::config::{OutputType, PrintRequest, SwitchWithOptPath};
use crate::session::search_paths::{PathKind, SearchPath};
use crate::util::nodemap::{FxHashMap, FxHashSet};
use crate::util::common::{duration_to_secs_str, ErrorReported};
Expand Down Expand Up @@ -1303,15 +1303,18 @@ fn validate_commandline_args_with_session_available(sess: &Session) {
}

// PGO does not work reliably with panic=unwind on Windows. Let's make it
// a warning to combine the two for now. It always runs into an assertions
// an error to combine the two for now. It always runs into an assertions
// if LLVM is built with assertions, but without assertions it sometimes
// does not crash and will probably generate a corrupted binary.
// We should only display this error if we're actually going to run PGO.
// If we're just supposed to print out some data, don't show the error (#61002).
if sess.opts.cg.profile_generate.enabled() &&
sess.target.target.options.is_like_msvc &&
sess.panic_strategy() == PanicStrategy::Unwind {
sess.warn("Profile-guided optimization does not yet work in conjunction \
with `-Cpanic=unwind` on Windows when targeting MSVC. \
See https://github.com/rust-lang/rust/issues/61002 for details.");
sess.panic_strategy() == PanicStrategy::Unwind &&
sess.opts.prints.iter().all(|&p| p == PrintRequest::NativeStaticLibs) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this all because, when we are "printing" native static libs, that's the only thing we print? (I would sort of expect any.)

Copy link
Member Author

Choose a reason for hiding this comment

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

PrintRequest::NativeStaticLibs is special in that we still generate binaries and we also emit an info message with details about those native libs during that process. All of the other print options halt compilation after the requested information is printed.

I used .all() here because we want the following behavior:

all(prints: []) => true // this causes the error to be emitted
all(prints: [NativeStaticLibs]) => true // this causes the error to be emitted
all(prints: [ _ ]) => false // skip emitting the error since we're going to stop compilation after printing the requested data

sess.err("Profile-guided optimization does not yet work in conjunction \
with `-Cpanic=unwind` on Windows when targeting MSVC. \
See https://github.com/rust-lang/rust/issues/61002 for details.");
}
}

Expand Down