Skip to content

Commit

Permalink
Stop telling people to submit bugs for internal feature ICEs
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Noratrieb committed Oct 17, 2023
1 parent 98c1e3d commit daecd67
Show file tree
Hide file tree
Showing 13 changed files with 108 additions and 23 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_driver_impl/messages.ftl
Original file line number Diff line number Diff line change
@@ -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}
Expand Down
60 changes: 51 additions & 9 deletions compiler/rustc_driver_impl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -224,11 +224,18 @@ pub struct RunCompiler<'a, 'b> {
file_loader: Option<Box<dyn FileLoader + Send + Sync>>,
make_codegen_backend:
Option<Box<dyn FnOnce(&config::Options) -> Box<dyn CodegenBackend> + Send>>,
using_internal_features: Arc<std::sync::atomic::AtomicBool>,
}

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.
Expand Down Expand Up @@ -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<AtomicBool>) -> 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,
)
}
}

Expand All @@ -273,6 +294,7 @@ fn run_compiler(
make_codegen_backend: Option<
Box<dyn FnOnce(&config::Options) -> Box<dyn CodegenBackend> + Send>,
>,
using_internal_features: Arc<std::sync::atomic::AtomicBool>,
) -> interface::Result<()> {
let mut early_error_handler = EarlyErrorHandler::new(ErrorOutputType::default());

Expand Down Expand Up @@ -316,6 +338,7 @@ fn run_compiler(
override_queries: None,
make_codegen_backend,
registry: diagnostics_registry(),
using_internal_features,
expanded_args: args,
};

Expand Down Expand Up @@ -1323,8 +1346,12 @@ fn ice_path() -> &'static Option<PathBuf> {
/// 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<AtomicBool> {
// 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
Expand All @@ -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<'_>| {
Expand Down Expand Up @@ -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.
Expand All @@ -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(
Expand All @@ -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();
Expand Down Expand Up @@ -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()
Expand All @@ -1506,7 +1546,9 @@ pub fn main() -> ! {
})
})
.collect::<Vec<_>>();
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 {
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_driver_impl/src/session_diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_expand/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_interface/src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> = result::Result<T, ErrorGuaranteed>;

Expand Down Expand Up @@ -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<std::sync::atomic::AtomicBool>,

/// 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
Expand Down Expand Up @@ -323,6 +330,7 @@ pub fn run_compiler<R: Send>(config: Config, f: impl FnOnce(&Compiler) -> R + Se
config.make_codegen_backend,
registry.clone(),
config.ice_file,
config.using_internal_features,
config.expanded_args,
);

Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>)>;

Expand Down Expand Up @@ -69,6 +70,7 @@ fn mk_session(handler: &mut EarlyErrorHandler, matches: getopts::Matches) -> (Se
None,
"",
None,
Arc::default(),
Default::default(),
);
(sess, cfg)
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_interface/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -71,6 +71,7 @@ pub fn create_session(
>,
descriptions: Registry,
ice_file: Option<PathBuf>,
using_internal_features: Arc<AtomicBool>,
expanded_args: Vec<String>,
) -> (Session, Box<dyn CodegenBackend>) {
let codegen_backend = if let Some(make_codegen_backend) = make_codegen_backend {
Expand Down Expand Up @@ -114,6 +115,7 @@ pub fn create_session(
target_override,
rustc_version_str().unwrap_or("unknown"),
ice_file,
using_internal_features,
expanded_args,
);

Expand Down
10 changes: 9 additions & 1 deletion compiler/rustc_session/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<AtomicBool>,

/// 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
Expand Down Expand Up @@ -1333,6 +1339,7 @@ pub fn build_session(
target_override: Option<Target>,
cfg_version: &'static str,
ice_file: Option<PathBuf>,
using_internal_features: Arc<AtomicBool>,
expanded_args: Vec<String>,
) -> Session {
// FIXME: This is not general enough to make the warning lint completely override
Expand Down Expand Up @@ -1469,6 +1476,7 @@ pub fn build_session(
target_features: Default::default(),
unstable_target_features: Default::default(),
cfg_version,
using_internal_features,
expanded_args,
};

Expand Down
3 changes: 3 additions & 0 deletions src/librustdoc/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -198,6 +199,7 @@ pub(crate) fn create_config(
..
}: RustdocOptions,
RenderOptions { document_private, .. }: &RenderOptions,
using_internal_features: Arc<AtomicBool>,
) -> rustc_interface::Config {
// Add the doc cfg into the doc build.
cfgs.push("doc".to_string());
Expand Down Expand Up @@ -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,
}
}
Expand Down
1 change: 1 addition & 0 deletions src/librustdoc/doctest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
};

Expand Down
16 changes: 11 additions & 5 deletions src/librustdoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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",
|_| (),
Expand All @@ -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)]
Expand Down Expand Up @@ -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<AtomicBool>,
) -> 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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down
Loading

0 comments on commit daecd67

Please sign in to comment.