From daecd67d05ee856ac2c284cc7b275040f496f8b7 Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Mon, 16 Oct 2023 22:11:57 +0200 Subject: [PATCH] Stop telling people to submit bugs for internal feature ICEs This keeps track of usage of internal features, and changes the message to instead tell them that using internal features is not supported. See MCP 620. --- compiler/rustc_driver_impl/messages.ftl | 1 + compiler/rustc_driver_impl/src/lib.rs | 60 ++++++++++++++++--- .../src/session_diagnostics.rs | 4 ++ compiler/rustc_expand/src/config.rs | 3 + compiler/rustc_interface/src/interface.rs | 8 +++ compiler/rustc_interface/src/tests.rs | 2 + compiler/rustc_interface/src/util.rs | 4 +- compiler/rustc_session/src/session.rs | 10 +++- src/librustdoc/core.rs | 3 + src/librustdoc/doctest.rs | 1 + src/librustdoc/lib.rs | 16 +++-- src/tools/clippy/src/driver.rs | 8 ++- src/tools/miri/src/bin/miri.rs | 11 ++-- 13 files changed, 108 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_driver_impl/messages.ftl b/compiler/rustc_driver_impl/messages.ftl index d3bd3244a52aa..39462112dc287 100644 --- a/compiler/rustc_driver_impl/messages.ftl +++ b/compiler/rustc_driver_impl/messages.ftl @@ -1,5 +1,6 @@ driver_impl_ice = the compiler unexpectedly panicked. this is a bug. driver_impl_ice_bug_report = we would appreciate a bug report: {$bug_report_url} +driver_impl_ice_bug_report_internal_feature = using internal features is not supported and expected to cause internal compiler errors when used incorrectly driver_impl_ice_exclude_cargo_defaults = some of the compiler flags provided by cargo are hidden driver_impl_ice_flags = compiler flags: {$flags} diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 5bb7c41677cd1..54a74d32d7935 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -60,7 +60,7 @@ use std::path::PathBuf; use std::process::{self, Command, Stdio}; use std::str; use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::OnceLock; +use std::sync::{Arc, OnceLock}; use std::time::{Instant, SystemTime}; use time::format_description::well_known::Rfc3339; use time::OffsetDateTime; @@ -224,11 +224,18 @@ pub struct RunCompiler<'a, 'b> { file_loader: Option>, make_codegen_backend: Option Box + Send>>, + using_internal_features: Arc, } impl<'a, 'b> RunCompiler<'a, 'b> { pub fn new(at_args: &'a [String], callbacks: &'b mut (dyn Callbacks + Send)) -> Self { - Self { at_args, callbacks, file_loader: None, make_codegen_backend: None } + Self { + at_args, + callbacks, + file_loader: None, + make_codegen_backend: None, + using_internal_features: Arc::default(), + } } /// Set a custom codegen backend. @@ -260,9 +267,23 @@ impl<'a, 'b> RunCompiler<'a, 'b> { self } + /// Set the session-global flag that checks whether internal features have been used, + /// suppressing the message about submitting an issue in ICEs when enabled. + #[must_use] + pub fn set_using_internal_features(mut self, using_internal_features: Arc) -> Self { + self.using_internal_features = using_internal_features; + self + } + /// Parse args and run the compiler. pub fn run(self) -> interface::Result<()> { - run_compiler(self.at_args, self.callbacks, self.file_loader, self.make_codegen_backend) + run_compiler( + self.at_args, + self.callbacks, + self.file_loader, + self.make_codegen_backend, + self.using_internal_features, + ) } } @@ -273,6 +294,7 @@ fn run_compiler( make_codegen_backend: Option< Box Box + Send>, >, + using_internal_features: Arc, ) -> interface::Result<()> { let mut early_error_handler = EarlyErrorHandler::new(ErrorOutputType::default()); @@ -316,6 +338,7 @@ fn run_compiler( override_queries: None, make_codegen_backend, registry: diagnostics_registry(), + using_internal_features, expanded_args: args, }; @@ -1323,8 +1346,12 @@ fn ice_path() -> &'static Option { /// If you have no extra info to report, pass the empty closure `|_| ()` as the argument to /// extra_info. /// +/// Returns a flag that can be set to disable the note for submitting a bug. This can be passed to +/// [`RunCompiler::set_using_internal_features`] to let macro expansion set it when encountering +/// internal features. +/// /// A custom rustc driver can skip calling this to set up a custom ICE hook. -pub fn install_ice_hook(bug_report_url: &'static str, extra_info: fn(&Handler)) { +pub fn install_ice_hook(bug_report_url: &'static str, extra_info: fn(&Handler)) -> Arc { // If the user has not explicitly overridden "RUST_BACKTRACE", then produce // full backtraces. When a compiler ICE happens, we want to gather // as much information as possible to present in the issue opened @@ -1335,6 +1362,8 @@ pub fn install_ice_hook(bug_report_url: &'static str, extra_info: fn(&Handler)) std::env::set_var("RUST_BACKTRACE", "full"); } + let using_internal_features = Arc::new(std::sync::atomic::AtomicBool::default()); + let using_internal_features_hook = using_internal_features.clone(); panic::update_hook(Box::new( move |default_hook: &(dyn Fn(&PanicInfo<'_>) + Send + Sync + 'static), info: &PanicInfo<'_>| { @@ -1384,9 +1413,11 @@ pub fn install_ice_hook(bug_report_url: &'static str, extra_info: fn(&Handler)) } // Print the ICE message - report_ice(info, bug_report_url, extra_info); + report_ice(info, bug_report_url, extra_info, &using_internal_features_hook); }, )); + + using_internal_features } /// Prints the ICE message, including query stack, but without backtrace. @@ -1395,7 +1426,12 @@ pub fn install_ice_hook(bug_report_url: &'static str, extra_info: fn(&Handler)) /// /// When `install_ice_hook` is called, this function will be called as the panic /// hook. -fn report_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str, extra_info: fn(&Handler)) { +fn report_ice( + info: &panic::PanicInfo<'_>, + bug_report_url: &str, + extra_info: fn(&Handler), + using_internal_features: &AtomicBool, +) { let fallback_bundle = rustc_errors::fallback_fluent_bundle(crate::DEFAULT_LOCALE_RESOURCES.to_vec(), false); let emitter = Box::new(rustc_errors::emitter::EmitterWriter::stderr( @@ -1412,7 +1448,11 @@ fn report_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str, extra_info: fn( handler.emit_err(session_diagnostics::Ice); } - handler.emit_note(session_diagnostics::IceBugReport { bug_report_url }); + if using_internal_features.load(std::sync::atomic::Ordering::Relaxed) { + handler.emit_note(session_diagnostics::IceBugReportInternalFeature); + } else { + handler.emit_note(session_diagnostics::IceBugReport { bug_report_url }); + } let version = util::version_str!().unwrap_or("unknown_version"); let triple = config::host_triple(); @@ -1496,7 +1536,7 @@ pub fn main() -> ! { init_rustc_env_logger(&handler); signal_handler::install(); let mut callbacks = TimePassesCallbacks::default(); - install_ice_hook(DEFAULT_BUG_REPORT_URL, |_| ()); + let using_internal_features = install_ice_hook(DEFAULT_BUG_REPORT_URL, |_| ()); let exit_code = catch_with_exit_code(|| { let args = env::args_os() .enumerate() @@ -1506,7 +1546,9 @@ pub fn main() -> ! { }) }) .collect::>(); - RunCompiler::new(&args, &mut callbacks).run() + RunCompiler::new(&args, &mut callbacks) + .set_using_internal_features(using_internal_features) + .run() }); if let Some(format) = callbacks.time_passes { diff --git a/compiler/rustc_driver_impl/src/session_diagnostics.rs b/compiler/rustc_driver_impl/src/session_diagnostics.rs index 442989f8de83a..2b31fdd77cca3 100644 --- a/compiler/rustc_driver_impl/src/session_diagnostics.rs +++ b/compiler/rustc_driver_impl/src/session_diagnostics.rs @@ -42,6 +42,10 @@ pub(crate) struct IceBugReport<'a> { pub bug_report_url: &'a str, } +#[derive(Diagnostic)] +#[diag(driver_impl_ice_bug_report_internal_feature)] +pub(crate) struct IceBugReportInternalFeature; + #[derive(Diagnostic)] #[diag(driver_impl_ice_version)] pub(crate) struct IceVersion<'a> { diff --git a/compiler/rustc_expand/src/config.rs b/compiler/rustc_expand/src/config.rs index b73c7593381c3..0108845b2427d 100644 --- a/compiler/rustc_expand/src/config.rs +++ b/compiler/rustc_expand/src/config.rs @@ -167,6 +167,9 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { // If the declared feature is unstable, record it. if let Some(f) = UNSTABLE_FEATURES.iter().find(|f| name == f.feature.name) { (f.set_enabled)(&mut features); + if features.internal(name) { + sess.using_internal_features.store(true, std::sync::atomic::Ordering::Relaxed); + } features.set_declared_lang_feature(name, mi.span(), None); continue; } diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index 1c330c064ab69..4334d729d8e1a 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -23,6 +23,7 @@ use rustc_span::source_map::{FileLoader, FileName}; use rustc_span::symbol::sym; use std::path::PathBuf; use std::result; +use std::sync::Arc; pub type Result = result::Result; @@ -280,6 +281,12 @@ pub struct Config { /// Registry of diagnostics codes. pub registry: Registry, + /// The inner atomic value is set to true when a feature marked as `internal` is + /// enabled. Makes it so that "please report a bug" is hidden, as ICEs with + /// internal features are wontfix, and they are usually the cause of the ICEs. + /// None signifies that this is not tracked. + pub using_internal_features: Arc, + /// All commandline args used to invoke the compiler, with @file args fully expanded. /// This will only be used within debug info, e.g. in the pdb file on windows /// This is mainly useful for other tools that reads that debuginfo to figure out @@ -323,6 +330,7 @@ pub fn run_compiler(config: Config, f: impl FnOnce(&Compiler) -> R + Se config.make_codegen_backend, registry.clone(), config.ice_file, + config.using_internal_features, config.expanded_args, ); diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 7799af370089a..073102043e563 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -35,6 +35,7 @@ use rustc_target::spec::{RelroLevel, SanitizerSet, SplitDebuginfo, StackProtecto use std::collections::{BTreeMap, BTreeSet}; use std::num::NonZeroUsize; use std::path::{Path, PathBuf}; +use std::sync::Arc; type CfgSpecs = FxHashSet<(String, Option)>; @@ -69,6 +70,7 @@ fn mk_session(handler: &mut EarlyErrorHandler, matches: getopts::Matches) -> (Se None, "", None, + Arc::default(), Default::default(), ); (sess, cfg) diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index 0634e44c5620b..d2455a036ccd5 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -26,7 +26,7 @@ use std::env::consts::{DLL_PREFIX, DLL_SUFFIX}; use std::mem; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::OnceLock; +use std::sync::{Arc, OnceLock}; use std::thread; /// Function pointer type that constructs a new CodegenBackend. @@ -71,6 +71,7 @@ pub fn create_session( >, descriptions: Registry, ice_file: Option, + using_internal_features: Arc, expanded_args: Vec, ) -> (Session, Box) { let codegen_backend = if let Some(make_codegen_backend) = make_codegen_backend { @@ -114,6 +115,7 @@ pub fn create_session( target_override, rustc_version_str().unwrap_or("unknown"), ice_file, + using_internal_features, expanded_args, ); diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 5cac11cc8f782..78cd28b68329a 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -44,7 +44,7 @@ use std::fmt; use std::ops::{Div, Mul}; use std::path::{Path, PathBuf}; use std::str::FromStr; -use std::sync::Arc; +use std::sync::{atomic::AtomicBool, Arc}; use std::time::Duration; pub struct OptimizationFuel { @@ -201,6 +201,12 @@ pub struct Session { /// The version of the rustc process, possibly including a commit hash and description. pub cfg_version: &'static str, + /// The inner atomic value is set to true when a feature marked as `internal` is + /// enabled. Makes it so that "please report a bug" is hidden, as ICEs with + /// internal features are wontfix, and they are usually the cause of the ICEs. + /// None signifies that this is not tracked. + pub using_internal_features: Arc, + /// All commandline args used to invoke the compiler, with @file args fully expanded. /// This will only be used within debug info, e.g. in the pdb file on windows /// This is mainly useful for other tools that reads that debuginfo to figure out @@ -1333,6 +1339,7 @@ pub fn build_session( target_override: Option, cfg_version: &'static str, ice_file: Option, + using_internal_features: Arc, expanded_args: Vec, ) -> Session { // FIXME: This is not general enough to make the warning lint completely override @@ -1469,6 +1476,7 @@ pub fn build_session( target_features: Default::default(), unstable_target_features: Default::default(), cfg_version, + using_internal_features, expanded_args, }; diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 3e6066c78fba2..a43cdf93700a6 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -23,6 +23,7 @@ use std::cell::RefCell; use std::mem; use std::rc::Rc; use std::sync::LazyLock; +use std::sync::{atomic::AtomicBool, Arc}; use crate::clean::inline::build_external_trait; use crate::clean::{self, ItemId}; @@ -198,6 +199,7 @@ pub(crate) fn create_config( .. }: RustdocOptions, RenderOptions { document_private, .. }: &RenderOptions, + using_internal_features: Arc, ) -> rustc_interface::Config { // Add the doc cfg into the doc build. cfgs.push("doc".to_string()); @@ -292,6 +294,7 @@ pub(crate) fn create_config( make_codegen_backend: None, registry: rustc_driver::diagnostics_registry(), ice_file: None, + using_internal_features, expanded_args, } } diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 741d329fb1922..6d2d090ac8f78 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -109,6 +109,7 @@ pub(crate) fn run(options: RustdocOptions) -> Result<(), ErrorGuaranteed> { make_codegen_backend: None, registry: rustc_driver::diagnostics_registry(), ice_file: None, + using_internal_features: Arc::default(), expanded_args: options.expanded_args.clone(), }; diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 67f5ea5d98b2a..41aee06c8d26f 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -74,6 +74,7 @@ extern crate jemalloc_sys; use std::env::{self, VarError}; use std::io::{self, IsTerminal}; use std::process; +use std::sync::{atomic::AtomicBool, Arc}; use rustc_driver::abort_on_err; use rustc_errors::ErrorGuaranteed; @@ -157,7 +158,7 @@ pub fn main() { let mut handler = EarlyErrorHandler::new(ErrorOutputType::default()); - rustc_driver::install_ice_hook( + let using_internal_features = rustc_driver::install_ice_hook( "https://github.com/rust-lang/rust/issues/new\ ?labels=C-bug%2C+I-ICE%2C+T-rustdoc&template=ice.md", |_| (), @@ -177,7 +178,7 @@ pub fn main() { rustc_driver::init_env_logger(&handler, "RUSTDOC_LOG"); let exit_code = rustc_driver::catch_with_exit_code(|| match get_args(&handler) { - Some(args) => main_args(&mut handler, &args), + Some(args) => main_args(&mut handler, &args, using_internal_features), _ => { #[allow(deprecated)] @@ -701,7 +702,11 @@ fn run_renderer<'tcx, T: formats::FormatRenderer<'tcx>>( } } -fn main_args(handler: &mut EarlyErrorHandler, at_args: &[String]) -> MainResult { +fn main_args( + handler: &mut EarlyErrorHandler, + at_args: &[String], + using_internal_features: Arc, +) -> MainResult { // Throw away the first argument, the name of the binary. // In case of at_args being empty, as might be the case by // passing empty argument array to execve under some platforms, @@ -752,7 +757,8 @@ fn main_args(handler: &mut EarlyErrorHandler, at_args: &[String]) -> MainResult (false, true) => { let input = options.input.clone(); let edition = options.edition; - let config = core::create_config(handler, options, &render_options); + let config = + core::create_config(handler, options, &render_options, using_internal_features); // `markdown::render` can invoke `doctest::make_test`, which // requires session globals and a thread pool, so we use @@ -785,7 +791,7 @@ fn main_args(handler: &mut EarlyErrorHandler, at_args: &[String]) -> MainResult let scrape_examples_options = options.scrape_examples_options.clone(); let bin_crate = options.bin_crate; - let config = core::create_config(handler, options, &render_options); + let config = core::create_config(handler, options, &render_options, using_internal_features); interface::run_compiler(config, |compiler| { let sess = compiler.session(); diff --git a/src/tools/clippy/src/driver.rs b/src/tools/clippy/src/driver.rs index d47767faed9ed..3876da150c550 100644 --- a/src/tools/clippy/src/driver.rs +++ b/src/tools/clippy/src/driver.rs @@ -178,7 +178,7 @@ pub fn main() { rustc_driver::init_rustc_env_logger(&handler); - rustc_driver::install_ice_hook(BUG_REPORT_URL, |handler| { + let using_internal_features = rustc_driver::install_ice_hook(BUG_REPORT_URL, |handler| { // FIXME: this macro calls unwrap internally but is called in a panicking context! It's not // as simple as moving the call from the hook to main, because `install_ice_hook` doesn't // accept a generic closure. @@ -265,9 +265,11 @@ pub fn main() { let clippy_enabled = !cap_lints_allow && (!no_deps || in_primary_package); if clippy_enabled { args.extend(clippy_args); - rustc_driver::RunCompiler::new(&args, &mut ClippyCallbacks { clippy_args_var }).run() + rustc_driver::RunCompiler::new(&args, &mut ClippyCallbacks { clippy_args_var }) + .set_using_internal_features(using_internal_features).run() } else { - rustc_driver::RunCompiler::new(&args, &mut RustcCallbacks { clippy_args_var }).run() + rustc_driver::RunCompiler::new(&args, &mut RustcCallbacks { clippy_args_var }) + .set_using_internal_features(using_internal_features).run() } })) } diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index fc6151772a0fd..531128ed2ec92 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -241,6 +241,7 @@ fn run_compiler( mut args: Vec, target_crate: bool, callbacks: &mut (dyn rustc_driver::Callbacks + Send), + using_internal_features: std::sync::Arc ) -> ! { if target_crate { // Miri needs a custom sysroot for target crates. @@ -273,7 +274,8 @@ fn run_compiler( // Invoke compiler, and handle return code. let exit_code = rustc_driver::catch_with_exit_code(move || { - rustc_driver::RunCompiler::new(&args, callbacks).run() + rustc_driver::RunCompiler::new(&args, callbacks) + .set_using_internal_features(using_internal_features).run() }); std::process::exit(exit_code) } @@ -295,7 +297,7 @@ fn main() { // If the environment asks us to actually be rustc, then do that. if let Some(crate_kind) = env::var_os("MIRI_BE_RUSTC") { // Earliest rustc setup. - rustc_driver::install_ice_hook(rustc_driver::DEFAULT_BUG_REPORT_URL, |_| ()); + let using_internal_features = rustc_driver::install_ice_hook(rustc_driver::DEFAULT_BUG_REPORT_URL, |_| ()); rustc_driver::init_rustc_env_logger(&handler); let target_crate = if crate_kind == "target" { @@ -311,11 +313,12 @@ fn main() { env::args().collect(), target_crate, &mut MiriBeRustCompilerCalls { target_crate }, + using_internal_features, ) } // Add an ICE bug report hook. - rustc_driver::install_ice_hook("https://github.com/rust-lang/miri/issues/new", |_| ()); + let using_internal_features = rustc_driver::install_ice_hook("https://github.com/rust-lang/miri/issues/new", |_| ()); // Init loggers the Miri way. init_early_loggers(&handler); @@ -578,5 +581,5 @@ fn main() { debug!("rustc arguments: {:?}", rustc_args); debug!("crate arguments: {:?}", miri_config.args); - run_compiler(rustc_args, /* target_crate: */ true, &mut MiriCompilerCalls { miri_config }) + run_compiler(rustc_args, /* target_crate: */ true, &mut MiriCompilerCalls { miri_config }, using_internal_features) }