From f0be14565fd331e9c96123dda33bf1105f35201c Mon Sep 17 00:00:00 2001 From: jyn Date: Sat, 29 Apr 2023 06:30:21 -0500 Subject: [PATCH 01/12] drive-by cleanup of rustdoc comment --- src/librustdoc/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index c15afca22611b..70030e293344e 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -156,13 +156,13 @@ pub fn main() { rustc_driver::install_ice_hook(); - // When using CI artifacts (with `download_stage1 = true`), tracing is unconditionally built + // When using CI artifacts with `download-rustc`, tracing is unconditionally built // with `--features=static_max_level_info`, which disables almost all rustdoc logging. To avoid // this, compile our own version of `tracing` that logs all levels. // NOTE: this compiles both versions of tracing unconditionally, because // - The compile time hit is not that bad, especially compared to rustdoc's incremental times, and - // - Otherwise, there's no warning that logging is being ignored when `download_stage1 = true`. - // NOTE: The reason this doesn't show double logging when `download_stage1 = false` and + // - Otherwise, there's no warning that logging is being ignored when `download-rustc` is enabled + // NOTE: The reason this doesn't show double logging when `download-rustc = false` and // `debug_logging = true` is because all rustc logging goes to its version of tracing (the one // in the sysroot), and all of rustdoc's logging goes to its version (the one in Cargo.toml). init_logging(); From 2469afef1a9697e671f9e90971c342fa3e0006e6 Mon Sep 17 00:00:00 2001 From: jyn Date: Sat, 29 Apr 2023 06:29:07 -0500 Subject: [PATCH 02/12] Make the BUG_REPORT_URL configurable by tools This greatly simplifies how hard it is to set a custom bug report url; previously tools had to copy the entire hook implementation. - Switch clippy to the new hook This also adds a `extra_info` callback so clippy can include its own version number, which differs from rustc's. - Call `install_ice_hook` in rustfmt --- compiler/rustc_driver_impl/src/lib.rs | 103 ++++++++++++--------- src/librustdoc/lib.rs | 6 +- src/tools/clippy/src/driver.rs | 70 ++------------ src/tools/miri/src/bin/miri.rs | 8 +- src/tools/rustfmt/src/bin/main.rs | 9 ++ tests/rustdoc-ui/ice-bug-report-url.rs | 14 +++ tests/rustdoc-ui/ice-bug-report-url.stderr | 16 ++++ 7 files changed, 115 insertions(+), 111 deletions(-) create mode 100644 tests/rustdoc-ui/ice-bug-report-url.rs create mode 100644 tests/rustdoc-ui/ice-bug-report-url.stderr diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 5fac485de6417..92153e91a277d 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -25,7 +25,7 @@ use rustc_data_structures::profiling::{ use rustc_data_structures::sync::SeqCst; use rustc_errors::registry::{InvalidErrorCode, Registry}; use rustc_errors::{ - DiagnosticMessage, ErrorGuaranteed, PResult, SubdiagnosticMessage, TerminalUrl, + DiagnosticMessage, ErrorGuaranteed, Handler, PResult, SubdiagnosticMessage, TerminalUrl, }; use rustc_feature::find_gated_cfg; use rustc_fluent_macro::fluent_messages; @@ -55,7 +55,7 @@ use std::panic::{self, catch_unwind}; use std::path::PathBuf; use std::process::{self, Command, Stdio}; use std::str; -use std::sync::LazyLock; +use std::sync::OnceLock; use std::time::Instant; // This import blocks the use of panicking `print` and `println` in all the code @@ -119,7 +119,7 @@ pub const EXIT_SUCCESS: i32 = 0; /// Exit status code used for compilation failures and invalid flags. pub const EXIT_FAILURE: i32 = 1; -const BUG_REPORT_URL: &str = "https://github.com/rust-lang/rust/issues/new\ +pub const DEFAULT_BUG_REPORT_URL: &str = "https://github.com/rust-lang/rust/issues/new\ ?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md"; const ICE_REPORT_COMPILER_FLAGS: &[&str] = &["-Z", "-C", "--crate-type"]; @@ -1195,35 +1195,58 @@ pub fn catch_with_exit_code(f: impl FnOnce() -> interface::Result<()>) -> i32 { } } -static DEFAULT_HOOK: LazyLock) + Sync + Send + 'static>> = - LazyLock::new(|| { - let hook = panic::take_hook(); - panic::set_hook(Box::new(|info| { - // If the error was caused by a broken pipe then this is not a bug. - // Write the error and return immediately. See #98700. - #[cfg(windows)] - if let Some(msg) = info.payload().downcast_ref::() { - if msg.starts_with("failed printing to stdout: ") && msg.ends_with("(os error 232)") - { - early_error_no_abort(ErrorOutputType::default(), &msg); - return; - } - }; +/// Stores the default panic hook, from before [`install_ice_hook`] was called. +static DEFAULT_HOOK: OnceLock) + Sync + Send + 'static>> = + OnceLock::new(); + +/// Installs a panic hook that will print the ICE message on unexpected panics. +/// +/// The hook is intended to be useable even by external tools. You can pass a custom +/// `bug_report_url`, or report arbitrary info in `extra_info`. Note that `extra_info` is called in +/// a context where *the thread is currently panicking*, so it must not panic or the process will +/// abort. +/// +/// If you have no extra info to report, pass the empty closure `|_| ()` as the argument to +/// extra_info. +/// +/// 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)) { + // 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 + // by the user. Compiler developers and other rustc users can + // opt in to less-verbose backtraces by manually setting "RUST_BACKTRACE" + // (e.g. `RUST_BACKTRACE=1`) + if std::env::var("RUST_BACKTRACE").is_err() { + std::env::set_var("RUST_BACKTRACE", "full"); + } - // Invoke the default handler, which prints the actual panic message and optionally a backtrace - // Don't do this for delayed bugs, which already emit their own more useful backtrace. - if !info.payload().is::() { - (*DEFAULT_HOOK)(info); + let default_hook = DEFAULT_HOOK.get_or_init(panic::take_hook); - // Separate the output with an empty line - eprintln!(); + panic::set_hook(Box::new(move |info| { + // If the error was caused by a broken pipe then this is not a bug. + // Write the error and return immediately. See #98700. + #[cfg(windows)] + if let Some(msg) = info.payload().downcast_ref::() { + if msg.starts_with("failed printing to stdout: ") && msg.ends_with("(os error 232)") { + early_error_no_abort(ErrorOutputType::default(), &msg); + return; } + }; - // Print the ICE message - report_ice(info, BUG_REPORT_URL); - })); - hook - }); + // Invoke the default handler, which prints the actual panic message and optionally a backtrace + // Don't do this for delayed bugs, which already emit their own more useful backtrace. + if !info.payload().is::() { + (*default_hook)(info); + + // Separate the output with an empty line + eprintln!(); + } + + // Print the ICE message + report_ice(info, bug_report_url, extra_info); + })); +} /// Prints the ICE message, including query stack, but without backtrace. /// @@ -1231,7 +1254,7 @@ static DEFAULT_HOOK: LazyLock) + Sync + Send + /// /// When `install_ice_hook` is called, this function will be called as the panic /// hook. -pub fn report_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) { +pub fn report_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str, extra_info: fn(&Handler)) { let fallback_bundle = rustc_errors::fallback_fluent_bundle(crate::DEFAULT_LOCALE_RESOURCES.to_vec(), false); let emitter = Box::new(rustc_errors::emitter::EmitterWriter::stderr( @@ -1276,6 +1299,10 @@ pub fn report_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) { interface::try_print_query_stack(&handler, num_frames); + // We don't trust this callback not to panic itself, so run it at the end after we're sure we've + // printed all the relevant info. + extra_info(&handler); + #[cfg(windows)] if env::var("RUSTC_BREAK_ON_ICE").is_ok() { // Trigger a debugger if we crashed during bootstrap @@ -1283,22 +1310,6 @@ pub fn report_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) { } } -/// Installs a panic hook that will print the ICE message on unexpected panics. -/// -/// A custom rustc driver can skip calling this to set up a custom ICE hook. -pub fn install_ice_hook() { - // 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 - // by the user. Compiler developers and other rustc users can - // opt in to less-verbose backtraces by manually setting "RUST_BACKTRACE" - // (e.g. `RUST_BACKTRACE=1`) - if std::env::var("RUST_BACKTRACE").is_err() { - std::env::set_var("RUST_BACKTRACE", "full"); - } - LazyLock::force(&DEFAULT_HOOK); -} - /// This allows tools to enable rust logging without having to magically match rustc's /// tracing crate version. pub fn init_rustc_env_logger() { @@ -1369,7 +1380,7 @@ pub fn main() -> ! { init_rustc_env_logger(); signal_handler::install(); let mut callbacks = TimePassesCallbacks::default(); - install_ice_hook(); + install_ice_hook(DEFAULT_BUG_REPORT_URL, |_| ()); let exit_code = catch_with_exit_code(|| { let args = env::args_os() .enumerate() diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 70030e293344e..2746debbfab61 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -154,7 +154,11 @@ pub fn main() { } } - rustc_driver::install_ice_hook(); + 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", + |_| (), + ); // When using CI artifacts with `download-rustc`, tracing is unconditionally built // with `--features=static_max_level_info`, which disables almost all rustdoc logging. To avoid diff --git a/src/tools/clippy/src/driver.rs b/src/tools/clippy/src/driver.rs index 205905d509135..59bf447a7cd07 100644 --- a/src/tools/clippy/src/driver.rs +++ b/src/tools/clippy/src/driver.rs @@ -11,7 +11,6 @@ // FIXME: switch to something more ergonomic here, once available. // (Currently there is no way to opt into sysroot crates without `extern crate`.) extern crate rustc_driver; -extern crate rustc_errors; extern crate rustc_interface; extern crate rustc_session; extern crate rustc_span; @@ -20,13 +19,10 @@ use rustc_interface::interface; use rustc_session::parse::ParseSess; use rustc_span::symbol::Symbol; -use std::borrow::Cow; use std::env; use std::ops::Deref; -use std::panic; use std::path::Path; use std::process::exit; -use std::sync::LazyLock; /// If a command-line option matches `find_arg`, then apply the predicate `pred` on its value. If /// true, then return it. The parameter is assumed to be either `--arg=value` or `--arg value`. @@ -198,66 +194,18 @@ You can use tool lints to allow or deny lints from your code, eg.: const BUG_REPORT_URL: &str = "https://github.com/rust-lang/rust-clippy/issues/new"; -type PanicCallback = dyn Fn(&panic::PanicInfo<'_>) + Sync + Send + 'static; -static ICE_HOOK: LazyLock> = LazyLock::new(|| { - let hook = panic::take_hook(); - panic::set_hook(Box::new(|info| report_clippy_ice(info, BUG_REPORT_URL))); - hook -}); - -fn report_clippy_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) { - // Invoke our ICE handler, which prints the actual panic message and optionally a backtrace - (*ICE_HOOK)(info); - - // Separate the output with an empty line - eprintln!(); - - let fallback_bundle = rustc_errors::fallback_fluent_bundle(rustc_driver::DEFAULT_LOCALE_RESOURCES.to_vec(), false); - let emitter = Box::new(rustc_errors::emitter::EmitterWriter::stderr( - rustc_errors::ColorConfig::Auto, - None, - None, - fallback_bundle, - false, - false, - None, - false, - false, - rustc_errors::TerminalUrl::No, - )); - let handler = rustc_errors::Handler::with_emitter(true, None, emitter); - - // a .span_bug or .bug call has already printed what - // it wants to print. - if !info.payload().is::() { - let mut d = rustc_errors::Diagnostic::new(rustc_errors::Level::Bug, "unexpected panic"); - handler.emit_diagnostic(&mut d); - } - - let version_info = rustc_tools_util::get_version_info!(); - - let xs: Vec> = vec![ - "the compiler unexpectedly panicked. this is a bug.".into(), - format!("we would appreciate a bug report: {bug_report_url}").into(), - format!("Clippy version: {version_info}").into(), - ]; - - for note in &xs { - handler.note_without_error(note.as_ref()); - } - - // If backtraces are enabled, also print the query stack - let backtrace = env::var_os("RUST_BACKTRACE").map_or(false, |x| &x != "0"); - - let num_frames = if backtrace { None } else { Some(2) }; - - interface::try_print_query_stack(&handler, num_frames); -} - #[allow(clippy::too_many_lines)] pub fn main() { rustc_driver::init_rustc_env_logger(); - LazyLock::force(&ICE_HOOK); + + 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. + let version_info = rustc_tools_util::get_version_info!(); + handler.note_without_error(format!("Clippy version: {version_info}")); + }); + exit(rustc_driver::catch_with_exit_code(move || { let mut orig_args: Vec = env::args().collect(); let has_sysroot_arg = arg_value(&orig_args, "--sysroot", |_| true).is_some(); diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index 3aa71bb7e3c87..2c0074951d6e5 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -286,11 +286,10 @@ fn main() { // (`install_ice_hook` might change `RUST_BACKTRACE`.) let env_snapshot = env::vars_os().collect::>(); - // Earliest rustc setup. - rustc_driver::install_ice_hook(); - // 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, |_| ()); rustc_driver::init_rustc_env_logger(); let target_crate = if crate_kind == "target" { @@ -309,6 +308,9 @@ fn main() { ) } + // Add an ICE bug report hook. + rustc_driver::install_ice_hook("https://github.com/rust-lang/miri/issues/new", |_| ()); + // Init loggers the Miri way. init_early_loggers(); diff --git a/src/tools/rustfmt/src/bin/main.rs b/src/tools/rustfmt/src/bin/main.rs index be64559e87745..47846424b06e4 100644 --- a/src/tools/rustfmt/src/bin/main.rs +++ b/src/tools/rustfmt/src/bin/main.rs @@ -1,3 +1,5 @@ +#![feature(rustc_private)] + use anyhow::{format_err, Result}; use io::Error as IoError; @@ -19,7 +21,14 @@ use crate::rustfmt::{ FormatReportFormatterBuilder, Input, Session, Verbosity, }; +const BUG_REPORT_URL: &str = "https://github.com/rust-lang/rustfmt/issues/new?labels=bug"; + +// N.B. these crates are loaded from the sysroot, so they need extern crate. +extern crate rustc_driver; + fn main() { + rustc_driver::install_ice_hook(BUG_REPORT_URL, |_| ()); + env_logger::Builder::from_env("RUSTFMT_LOG").init(); let opts = make_opts(); diff --git a/tests/rustdoc-ui/ice-bug-report-url.rs b/tests/rustdoc-ui/ice-bug-report-url.rs new file mode 100644 index 0000000000000..cc066447d313b --- /dev/null +++ b/tests/rustdoc-ui/ice-bug-report-url.rs @@ -0,0 +1,14 @@ +// compile-flags: -Ztreat-err-as-bug +// failure-status: 101 +// error-pattern: aborting due to +// error-pattern: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-rustdoc&template=ice.md + +// normalize-stderr-test "note: compiler flags.*\n\n" -> "" +// normalize-stderr-test "note: rustc.*running on.*" -> "note: rustc {version} running on {platform}" +// normalize-stderr-test "thread.*panicked at .*, compiler.*" -> "thread panicked at 'aborting due to `-Z treat-err-as-bug`'" +// normalize-stderr-test "\s*\d{1,}: .*\n" -> "" +// normalize-stderr-test "\s at .*\n" -> "" +// normalize-stderr-test ".*note: Some details are omitted.*\n" -> "" + +fn wrong() +//~^ ERROR expected one of diff --git a/tests/rustdoc-ui/ice-bug-report-url.stderr b/tests/rustdoc-ui/ice-bug-report-url.stderr new file mode 100644 index 0000000000000..cfb73a9b9193f --- /dev/null +++ b/tests/rustdoc-ui/ice-bug-report-url.stderr @@ -0,0 +1,16 @@ +error: expected one of `->`, `where`, or `{`, found `` + --> $DIR/ice-bug-report-url.rs:13:10 + | +LL | fn wrong() + | ^ expected one of `->`, `where`, or `{` + +thread panicked at 'aborting due to `-Z treat-err-as-bug`' +stack backtrace: +error: the compiler unexpectedly panicked. this is a bug. + +note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-rustdoc&template=ice.md + +note: rustc {version} running on {platform} + +query stack during panic: +end of query stack From 7dd59fceef202e6849f575dd057bb90351362eba Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 20 Apr 2023 12:37:32 -0300 Subject: [PATCH 03/12] Add Drop terminator to SMIR --- compiler/rustc_smir/src/rustc_smir/mod.rs | 6 +++++- compiler/rustc_smir/src/stable_mir/mir/body.rs | 2 +- tests/ui-fulldeps/stable-mir/crate-info.rs | 13 ++++++++++++- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_smir/src/rustc_smir/mod.rs b/compiler/rustc_smir/src/rustc_smir/mod.rs index 09cb6fd22d5e6..57ceb89969dd4 100644 --- a/compiler/rustc_smir/src/rustc_smir/mod.rs +++ b/compiler/rustc_smir/src/rustc_smir/mod.rs @@ -162,7 +162,11 @@ fn rustc_terminator_to_terminator( Terminate => Terminator::Abort, Return => Terminator::Return, Unreachable => Terminator::Unreachable, - Drop { .. } => todo!(), + Drop { place, target, unwind } => Terminator::Drop { + place: rustc_place_to_place(place), + target: target.as_usize(), + unwind: rustc_unwind_to_unwind(unwind), + }, Call { func, args, destination, target, unwind, from_hir_call: _, fn_span: _ } => { Terminator::Call { func: rustc_op_to_op(func), diff --git a/compiler/rustc_smir/src/stable_mir/mir/body.rs b/compiler/rustc_smir/src/stable_mir/mir/body.rs index bd5e6b68a12fa..b3d0a83573357 100644 --- a/compiler/rustc_smir/src/stable_mir/mir/body.rs +++ b/compiler/rustc_smir/src/stable_mir/mir/body.rs @@ -26,7 +26,7 @@ pub enum Terminator { Drop { place: Place, target: usize, - unwind: Option, + unwind: UnwindAction, }, Call { func: Operand, diff --git a/tests/ui-fulldeps/stable-mir/crate-info.rs b/tests/ui-fulldeps/stable-mir/crate-info.rs index 95f27efa7715c..f89690e1d1592 100644 --- a/tests/ui-fulldeps/stable-mir/crate-info.rs +++ b/tests/ui-fulldeps/stable-mir/crate-info.rs @@ -60,6 +60,15 @@ fn test_stable_mir(tcx: TyCtxt<'_>) { stable_mir::mir::Terminator::Call { .. } => {} other => panic!("{other:?}"), } + + let drop = get_item(tcx, &items, (DefKind::Fn, "drop")).unwrap(); + let body = drop.body(); + assert_eq!(body.blocks.len(), 2); + let block = &body.blocks[0]; + match &block.terminator { + stable_mir::mir::Terminator::Drop { .. } => {} + other => panic!("{other:?}"), + } } // Use internal API to find a function in a crate. @@ -131,7 +140,9 @@ fn generate_input(path: &str) -> std::io::Result<()> { let x_64 = foo::bar(x); let y_64 = foo::bar(y); x_64.wrapping_add(y_64) - }}"# + }} + + pub fn drop(_: String) {{}}"# )?; Ok(()) } From 10b69dde3fd15334ea2382d2dc9e9a261de1afaf Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Wed, 3 May 2023 15:52:31 -0700 Subject: [PATCH 04/12] debuginfo: split method declaration and definition When we're adding a method to a type DIE, we only want a DW_AT_declaration there, because LLVM LTO can't unify type definitions when a child DIE is a full subprogram definition. Now the subprogram definition gets added at the CU level with a specification link back to the abstract declaration. --- .../rustc_codegen_llvm/src/debuginfo/mod.rs | 85 +++++++++++-------- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 15 ++++ .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 22 +++++ .../issue-109934-lto-debuginfo/Makefile | 12 +++ .../issue-109934-lto-debuginfo/lib.rs | 9 ++ 5 files changed, 109 insertions(+), 34 deletions(-) create mode 100644 tests/run-make/issue-109934-lto-debuginfo/Makefile create mode 100644 tests/run-make/issue-109934-lto-debuginfo/lib.rs diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs index 2e9f89f419696..b138b0c0e70a1 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs @@ -322,7 +322,7 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { let tcx = self.tcx; let def_id = instance.def_id(); - let containing_scope = get_containing_scope(self, instance); + let (containing_scope, is_method) = get_containing_scope(self, instance); let span = tcx.def_span(def_id); let loc = self.lookup_debug_loc(span.lo()); let file_metadata = file_metadata(self, &loc.file); @@ -378,8 +378,29 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { } } - unsafe { - return llvm::LLVMRustDIBuilderCreateFunction( + // When we're adding a method to a type DIE, we only want a DW_AT_declaration there, because + // LLVM LTO can't unify type definitions when a child DIE is a full subprogram definition. + // When we use this `decl` below, the subprogram definition gets created at the CU level + // with a DW_AT_specification pointing back to the type's declaration. + let decl = is_method.then(|| unsafe { + llvm::LLVMRustDIBuilderCreateMethod( + DIB(self), + containing_scope, + name.as_ptr().cast(), + name.len(), + linkage_name.as_ptr().cast(), + linkage_name.len(), + file_metadata, + loc.line, + function_type_metadata, + flags, + spflags & !DISPFlags::SPFlagDefinition, + template_parameters, + ) + }); + + return unsafe { + llvm::LLVMRustDIBuilderCreateFunction( DIB(self), containing_scope, name.as_ptr().cast(), @@ -394,9 +415,9 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { spflags, maybe_definition_llfn, template_parameters, - None, - ); - } + decl, + ) + }; fn get_function_signature<'ll, 'tcx>( cx: &CodegenCx<'ll, 'tcx>, @@ -493,14 +514,16 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { names } + /// Returns a scope, plus `true` if that's a type scope for "class" methods, + /// otherwise `false` for plain namespace scopes. fn get_containing_scope<'ll, 'tcx>( cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'tcx>, - ) -> &'ll DIScope { + ) -> (&'ll DIScope, bool) { // First, let's see if this is a method within an inherent impl. Because // if yes, we want to make the result subroutine DIE a child of the // subroutine's self-type. - let self_type = cx.tcx.impl_of_method(instance.def_id()).and_then(|impl_def_id| { + if let Some(impl_def_id) = cx.tcx.impl_of_method(instance.def_id()) { // If the method does *not* belong to a trait, proceed if cx.tcx.trait_id_of_impl(impl_def_id).is_none() { let impl_self_ty = cx.tcx.subst_and_normalize_erasing_regions( @@ -511,39 +534,33 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { // Only "class" methods are generally understood by LLVM, // so avoid methods on other types (e.g., `<*mut T>::null`). - match impl_self_ty.kind() { - ty::Adt(def, ..) if !def.is_box() => { - // Again, only create type information if full debuginfo is enabled - if cx.sess().opts.debuginfo == DebugInfo::Full - && !impl_self_ty.has_param() - { - Some(type_di_node(cx, impl_self_ty)) - } else { - Some(namespace::item_namespace(cx, def.did())) - } + if let ty::Adt(def, ..) = impl_self_ty.kind() && !def.is_box() { + // Again, only create type information if full debuginfo is enabled + if cx.sess().opts.debuginfo == DebugInfo::Full && !impl_self_ty.has_param() + { + return (type_di_node(cx, impl_self_ty), true); + } else { + return (namespace::item_namespace(cx, def.did()), false); } - _ => None, } } else { // For trait method impls we still use the "parallel namespace" // strategy - None } - }); + } - self_type.unwrap_or_else(|| { - namespace::item_namespace( - cx, - DefId { - krate: instance.def_id().krate, - index: cx - .tcx - .def_key(instance.def_id()) - .parent - .expect("get_containing_scope: missing parent?"), - }, - ) - }) + let scope = namespace::item_namespace( + cx, + DefId { + krate: instance.def_id().krate, + index: cx + .tcx + .def_key(instance.def_id()) + .parent + .expect("get_containing_scope: missing parent?"), + }, + ); + (scope, false) } } diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index c95148013eb74..1f98d91c32054 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -1987,6 +1987,21 @@ extern "C" { Decl: Option<&'a DIDescriptor>, ) -> &'a DISubprogram; + pub fn LLVMRustDIBuilderCreateMethod<'a>( + Builder: &DIBuilder<'a>, + Scope: &'a DIDescriptor, + Name: *const c_char, + NameLen: size_t, + LinkageName: *const c_char, + LinkageNameLen: size_t, + File: &'a DIFile, + LineNo: c_uint, + Ty: &'a DIType, + Flags: DIFlags, + SPFlags: DISPFlags, + TParam: &'a DIArray, + ) -> &'a DISubprogram; + pub fn LLVMRustDIBuilderCreateBasicType<'a>( Builder: &DIBuilder<'a>, Name: *const c_char, diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index cadb6b1e23fe9..49acd71b3e106 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -831,6 +831,28 @@ extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateFunction( return wrap(Sub); } +extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateMethod( + LLVMRustDIBuilderRef Builder, LLVMMetadataRef Scope, + const char *Name, size_t NameLen, + const char *LinkageName, size_t LinkageNameLen, + LLVMMetadataRef File, unsigned LineNo, + LLVMMetadataRef Ty, LLVMRustDIFlags Flags, + LLVMRustDISPFlags SPFlags, LLVMMetadataRef TParam) { + DITemplateParameterArray TParams = + DITemplateParameterArray(unwrap(TParam)); + DISubprogram::DISPFlags llvmSPFlags = fromRust(SPFlags); + DINode::DIFlags llvmFlags = fromRust(Flags); + DISubprogram *Sub = Builder->createMethod( + unwrapDI(Scope), + StringRef(Name, NameLen), + StringRef(LinkageName, LinkageNameLen), + unwrapDI(File), LineNo, + unwrapDI(Ty), + 0, 0, nullptr, // VTable params aren't used + llvmFlags, llvmSPFlags, TParams); + return wrap(Sub); +} + extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateBasicType( LLVMRustDIBuilderRef Builder, const char *Name, size_t NameLen, uint64_t SizeInBits, unsigned Encoding) { diff --git a/tests/run-make/issue-109934-lto-debuginfo/Makefile b/tests/run-make/issue-109934-lto-debuginfo/Makefile new file mode 100644 index 0000000000000..3b7a99d3dbc62 --- /dev/null +++ b/tests/run-make/issue-109934-lto-debuginfo/Makefile @@ -0,0 +1,12 @@ +# ignore-cross-compile +include ../tools.mk + +# With the upgrade to LLVM 16, this was getting: +# +# error: Cannot represent a difference across sections +# +# The error stemmed from DI function definitions under type scopes, fixed by +# only declaring in type scope and defining the subprogram elsewhere. + +all: + $(RUSTC) lib.rs --test -C lto=fat -C debuginfo=2 -C incremental=$(TMPDIR)/inc-fat diff --git a/tests/run-make/issue-109934-lto-debuginfo/lib.rs b/tests/run-make/issue-109934-lto-debuginfo/lib.rs new file mode 100644 index 0000000000000..c405928bd1824 --- /dev/null +++ b/tests/run-make/issue-109934-lto-debuginfo/lib.rs @@ -0,0 +1,9 @@ +extern crate alloc; + +#[cfg(test)] +mod tests { + #[test] + fn something_alloc() { + assert_eq!(Vec::::new(), Vec::::new()); + } +} From 964fb67a5fa1b99add4efb01a7dc2a02add4b071 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 20 Apr 2023 03:56:36 +0000 Subject: [PATCH 05/12] Use fulfillment to check Drop impl compatibility --- .../rustc_hir_analysis/src/check/dropck.rs | 324 +++++------------- .../src/infer/lexical_region_resolve/mod.rs | 11 + compiler/rustc_middle/src/traits/mod.rs | 4 + .../src/traits/error_reporting/suggestions.rs | 3 +- 4 files changed, 108 insertions(+), 234 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/dropck.rs b/compiler/rustc_hir_analysis/src/check/dropck.rs index bae80807f71b9..5ba1ca1c807bc 100644 --- a/compiler/rustc_hir_analysis/src/check/dropck.rs +++ b/compiler/rustc_hir_analysis/src/check/dropck.rs @@ -1,12 +1,14 @@ // FIXME(@lcnr): Move this module out of `rustc_hir_analysis`. // // We don't do any drop checking during hir typeck. +use rustc_data_structures::fx::FxHashSet; use rustc_errors::{struct_span_err, ErrorGuaranteed}; -use rustc_middle::ty::error::TypeError; -use rustc_middle::ty::relate::{Relate, RelateResult, TypeRelation}; +use rustc_infer::infer::outlives::env::OutlivesEnvironment; +use rustc_infer::infer::{RegionResolutionError, TyCtxtInferExt}; use rustc_middle::ty::subst::SubstsRef; use rustc_middle::ty::util::IgnoreRegions; -use rustc_middle::ty::{self, Predicate, Ty, TyCtxt}; +use rustc_middle::ty::{self, TyCtxt}; +use rustc_trait_selection::traits::{self, ObligationCtxt}; use crate::errors; use crate::hir::def_id::{DefId, LocalDefId}; @@ -43,21 +45,20 @@ pub fn check_drop_impl(tcx: TyCtxt<'_>, drop_impl_did: DefId) -> Result<(), Erro } } let dtor_self_type = tcx.type_of(drop_impl_did).subst_identity(); - let dtor_predicates = tcx.predicates_of(drop_impl_did); match dtor_self_type.kind() { - ty::Adt(adt_def, self_to_impl_substs) => { + ty::Adt(adt_def, adt_to_impl_substs) => { ensure_drop_params_and_item_params_correspond( tcx, drop_impl_did.expect_local(), adt_def.did(), - self_to_impl_substs, + adt_to_impl_substs, )?; ensure_drop_predicates_are_implied_by_item_defn( tcx, - dtor_predicates, + drop_impl_did.expect_local(), adt_def.did().expect_local(), - self_to_impl_substs, + adt_to_impl_substs, ) } _ => { @@ -78,9 +79,9 @@ fn ensure_drop_params_and_item_params_correspond<'tcx>( tcx: TyCtxt<'tcx>, drop_impl_did: LocalDefId, self_type_did: DefId, - drop_impl_substs: SubstsRef<'tcx>, + adt_to_impl_substs: SubstsRef<'tcx>, ) -> Result<(), ErrorGuaranteed> { - let Err(arg) = tcx.uses_unique_generic_params(drop_impl_substs, IgnoreRegions::No) else { + let Err(arg) = tcx.uses_unique_generic_params(adt_to_impl_substs, IgnoreRegions::No) else { return Ok(()) }; @@ -111,237 +112,94 @@ fn ensure_drop_params_and_item_params_correspond<'tcx>( /// implied by assuming the predicates attached to self_type_did. fn ensure_drop_predicates_are_implied_by_item_defn<'tcx>( tcx: TyCtxt<'tcx>, - dtor_predicates: ty::GenericPredicates<'tcx>, - self_type_did: LocalDefId, - self_to_impl_substs: SubstsRef<'tcx>, + drop_impl_def_id: LocalDefId, + adt_def_id: LocalDefId, + adt_to_impl_substs: SubstsRef<'tcx>, ) -> Result<(), ErrorGuaranteed> { - let mut result = Ok(()); - - // Here is an example, analogous to that from - // `compare_impl_method`. - // - // Consider a struct type: - // - // struct Type<'c, 'b:'c, 'a> { - // x: &'a Contents // (contents are irrelevant; - // y: &'c Cell<&'b Contents>, // only the bounds matter for our purposes.) - // } - // - // and a Drop impl: - // - // impl<'z, 'y:'z, 'x:'y> Drop for P<'z, 'y, 'x> { - // fn drop(&mut self) { self.y.set(self.x); } // (only legal if 'x: 'y) - // } - // - // We start out with self_to_impl_substs, that maps the generic - // parameters of Type to that of the Drop impl. + let infcx = tcx.infer_ctxt().build(); + let ocx = ObligationCtxt::new(&infcx); + + // Take the param-env of the adt and substitute the substs that show up in + // the implementation's self type. This gives us the assumptions that the + // self ty of the implementation is allowed to know just from it being a + // well-formed adt, since that's all we're allowed to assume while proving + // the Drop implementation is not specialized. // - // self_to_impl_substs = {'c => 'z, 'b => 'y, 'a => 'x} - // - // Applying this to the predicates (i.e., assumptions) provided by the item - // definition yields the instantiated assumptions: - // - // ['y : 'z] - // - // We then check all of the predicates of the Drop impl: - // - // ['y:'z, 'x:'y] - // - // and ensure each is in the list of instantiated - // assumptions. Here, `'y:'z` is present, but `'x:'y` is - // absent. So we report an error that the Drop impl injected a - // predicate that is not present on the struct definition. - - // We can assume the predicates attached to struct/enum definition - // hold. - let generic_assumptions = tcx.predicates_of(self_type_did); - - let assumptions_in_impl_context = generic_assumptions.instantiate(tcx, &self_to_impl_substs); - let assumptions_in_impl_context = assumptions_in_impl_context.predicates; - - debug!(?assumptions_in_impl_context, ?dtor_predicates.predicates); - - let self_param_env = tcx.param_env(self_type_did); - - // An earlier version of this code attempted to do this checking - // via the traits::fulfill machinery. However, it ran into trouble - // since the fulfill machinery merely turns outlives-predicates - // 'a:'b and T:'b into region inference constraints. It is simpler - // just to look for all the predicates directly. - - assert_eq!(dtor_predicates.parent, None); - for &(predicate, predicate_sp) in dtor_predicates.predicates { - // (We do not need to worry about deep analysis of type - // expressions etc because the Drop impls are already forced - // to take on a structure that is roughly an alpha-renaming of - // the generic parameters of the item definition.) - - // This path now just checks *all* predicates via an instantiation of - // the `SimpleEqRelation`, which simply forwards to the `relate` machinery - // after taking care of anonymizing late bound regions. - // - // However, it may be more efficient in the future to batch - // the analysis together via the fulfill (see comment above regarding - // the usage of the fulfill machinery), rather than the - // repeated `.iter().any(..)` calls. + // We don't need to normalize this param-env or anything, since we're only + // substituting it with free params, so no additional param-env normalization + // can occur on top of what has been done in the param_env query itself. + let param_env = ty::EarlyBinder(tcx.param_env(adt_def_id)) + .subst(tcx, adt_to_impl_substs) + .with_constness(tcx.constness(drop_impl_def_id)); + + for (pred, span) in tcx.predicates_of(drop_impl_def_id).instantiate_identity(tcx) { + let normalize_cause = traits::ObligationCause::misc(span, adt_def_id); + let pred = ocx.normalize(&normalize_cause, param_env, pred); + let cause = traits::ObligationCause::new(span, adt_def_id, traits::DropImpl); + ocx.register_obligation(traits::Obligation::new(tcx, cause, param_env, pred)); + } - // This closure is a more robust way to check `Predicate` equality - // than simple `==` checks (which were the previous implementation). - // It relies on `ty::relate` for `TraitPredicate`, `ProjectionPredicate`, - // `ConstEvaluatable` and `TypeOutlives` (which implement the Relate trait), - // while delegating on simple equality for the other `Predicate`. - // This implementation solves (Issue #59497) and (Issue #58311). - // It is unclear to me at the moment whether the approach based on `relate` - // could be extended easily also to the other `Predicate`. - let predicate_matches_closure = |p: Predicate<'tcx>| { - let mut relator: SimpleEqRelation<'tcx> = SimpleEqRelation::new(tcx, self_param_env); - let predicate = predicate.kind(); - let p = p.kind(); - match (predicate.skip_binder(), p.skip_binder()) { - ( - ty::PredicateKind::Clause(ty::Clause::Trait(a)), - ty::PredicateKind::Clause(ty::Clause::Trait(b)), - ) => relator.relate(predicate.rebind(a), p.rebind(b)).is_ok(), - ( - ty::PredicateKind::Clause(ty::Clause::Projection(a)), - ty::PredicateKind::Clause(ty::Clause::Projection(b)), - ) => relator.relate(predicate.rebind(a), p.rebind(b)).is_ok(), - ( - ty::PredicateKind::ConstEvaluatable(a), - ty::PredicateKind::ConstEvaluatable(b), - ) => relator.relate(predicate.rebind(a), predicate.rebind(b)).is_ok(), - ( - ty::PredicateKind::Clause(ty::Clause::TypeOutlives(ty::OutlivesPredicate( - ty_a, - lt_a, - ))), - ty::PredicateKind::Clause(ty::Clause::TypeOutlives(ty::OutlivesPredicate( - ty_b, - lt_b, - ))), - ) => { - relator.relate(predicate.rebind(ty_a), p.rebind(ty_b)).is_ok() - && relator.relate(predicate.rebind(lt_a), p.rebind(lt_b)).is_ok() - } - (ty::PredicateKind::WellFormed(arg_a), ty::PredicateKind::WellFormed(arg_b)) => { - relator.relate(predicate.rebind(arg_a), p.rebind(arg_b)).is_ok() - } - _ => predicate == p, + // All of the custom error reporting logic is to preserve parity with the old + // error messages. + // + // They can probably get removed with better treatment of the new `DropImpl` + // obligation cause code, and perhaps some custom logic in `report_region_errors`. + + let errors = ocx.select_all_or_error(); + if !errors.is_empty() { + let mut guar = None; + let mut root_predicates = FxHashSet::default(); + for error in errors { + let root_predicate = error.root_obligation.predicate; + if root_predicates.insert(root_predicate) { + let item_span = tcx.def_span(adt_def_id); + let self_descr = tcx.def_descr(adt_def_id.to_def_id()); + guar = Some( + struct_span_err!( + tcx.sess, + error.root_obligation.cause.span, + E0367, + "`Drop` impl requires `{root_predicate}` \ + but the {self_descr} it is implemented for does not", + ) + .span_note(item_span, "the implementor must specify the same requirement") + .emit(), + ); } - }; - - if !assumptions_in_impl_context.iter().copied().any(predicate_matches_closure) { - let item_span = tcx.def_span(self_type_did); - let self_descr = tcx.def_descr(self_type_did.to_def_id()); - let reported = struct_span_err!( - tcx.sess, - predicate_sp, - E0367, - "`Drop` impl requires `{predicate}` but the {self_descr} it is implemented for does not", - ) - .span_note(item_span, "the implementor must specify the same requirement") - .emit(); - result = Err(reported); } + return Err(guar.unwrap()); } - result -} - -/// This is an implementation of the [`TypeRelation`] trait with the -/// aim of simply comparing for equality (without side-effects). -/// -/// It is not intended to be used anywhere else other than here. -pub(crate) struct SimpleEqRelation<'tcx> { - tcx: TyCtxt<'tcx>, - param_env: ty::ParamEnv<'tcx>, -} - -impl<'tcx> SimpleEqRelation<'tcx> { - fn new(tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> SimpleEqRelation<'tcx> { - SimpleEqRelation { tcx, param_env } - } -} - -impl<'tcx> TypeRelation<'tcx> for SimpleEqRelation<'tcx> { - fn tcx(&self) -> TyCtxt<'tcx> { - self.tcx - } - - fn param_env(&self) -> ty::ParamEnv<'tcx> { - self.param_env - } - - fn tag(&self) -> &'static str { - "dropck::SimpleEqRelation" - } - - fn a_is_expected(&self) -> bool { - true - } - - fn relate_with_variance>( - &mut self, - _: ty::Variance, - _info: ty::VarianceDiagInfo<'tcx>, - a: T, - b: T, - ) -> RelateResult<'tcx, T> { - // Here we ignore variance because we require drop impl's types - // to be *exactly* the same as to the ones in the struct definition. - self.relate(a, b) - } - - fn tys(&mut self, a: Ty<'tcx>, b: Ty<'tcx>) -> RelateResult<'tcx, Ty<'tcx>> { - debug!("SimpleEqRelation::tys(a={:?}, b={:?})", a, b); - ty::relate::super_relate_tys(self, a, b) - } - - fn regions( - &mut self, - a: ty::Region<'tcx>, - b: ty::Region<'tcx>, - ) -> RelateResult<'tcx, ty::Region<'tcx>> { - debug!("SimpleEqRelation::regions(a={:?}, b={:?})", a, b); - - // We can just equate the regions because LBRs have been - // already anonymized. - if a == b { - Ok(a) - } else { - // I'm not sure is this `TypeError` is the right one, but - // it should not matter as it won't be checked (the dropck - // will emit its own, more informative and higher-level errors - // in case anything goes wrong). - Err(TypeError::RegionsPlaceholderMismatch) + let errors = ocx.infcx.resolve_regions(&OutlivesEnvironment::new(param_env)); + if !errors.is_empty() { + let mut guar = None; + for error in errors { + let item_span = tcx.def_span(adt_def_id); + let self_descr = tcx.def_descr(adt_def_id.to_def_id()); + let outlives = match error { + RegionResolutionError::ConcreteFailure(_, a, b) => format!("{b}: {a}"), + RegionResolutionError::GenericBoundFailure(_, generic, r) => { + format!("{generic}: {r}") + } + RegionResolutionError::SubSupConflict(_, _, _, a, _, b, _) => format!("{b}: {a}"), + RegionResolutionError::UpperBoundUniverseConflict(a, _, _, _, b) => { + format!("{b}: {a}", a = tcx.mk_re_var(a)) + } + }; + guar = Some( + struct_span_err!( + tcx.sess, + error.origin().span(), + E0367, + "`Drop` impl requires `{outlives}` \ + but the {self_descr} it is implemented for does not", + ) + .span_note(item_span, "the implementor must specify the same requirement") + .emit(), + ); } + return Err(guar.unwrap()); } - fn consts( - &mut self, - a: ty::Const<'tcx>, - b: ty::Const<'tcx>, - ) -> RelateResult<'tcx, ty::Const<'tcx>> { - debug!("SimpleEqRelation::consts(a={:?}, b={:?})", a, b); - ty::relate::super_relate_consts(self, a, b) - } - - fn binders( - &mut self, - a: ty::Binder<'tcx, T>, - b: ty::Binder<'tcx, T>, - ) -> RelateResult<'tcx, ty::Binder<'tcx, T>> - where - T: Relate<'tcx>, - { - debug!("SimpleEqRelation::binders({:?}: {:?}", a, b); - - // Anonymizing the LBRs is necessary to solve (Issue #59497). - // After we do so, it should be totally fine to skip the binders. - let anon_a = self.tcx.anonymize_bound_vars(a); - let anon_b = self.tcx.anonymize_bound_vars(b); - self.relate(anon_a.skip_binder(), anon_b.skip_binder())?; - - Ok(a) - } + Ok(()) } diff --git a/compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs b/compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs index f1468cae455b5..8482ae2aa38c8 100644 --- a/compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs +++ b/compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs @@ -102,6 +102,17 @@ pub enum RegionResolutionError<'tcx> { ), } +impl<'tcx> RegionResolutionError<'tcx> { + pub fn origin(&self) -> &SubregionOrigin<'tcx> { + match self { + RegionResolutionError::ConcreteFailure(origin, _, _) + | RegionResolutionError::GenericBoundFailure(origin, _, _) + | RegionResolutionError::SubSupConflict(_, _, origin, _, _, _, _) + | RegionResolutionError::UpperBoundUniverseConflict(_, _, _, origin, _) => origin, + } + } +} + struct RegionAndOrigin<'tcx> { region: Region<'tcx>, origin: SubregionOrigin<'tcx>, diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index c237556420841..8366567c2c364 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -444,6 +444,10 @@ pub enum ObligationCauseCode<'tcx> { AscribeUserTypeProvePredicate(Span), RustCall, + + /// Obligations to prove that a `std::ops::Drop` impl is not stronger than + /// the ADT it's being implemented for. + DropImpl, } /// The 'location' at which we try to perform HIR-based wf checking. diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index d34eb193453bd..37beb31a7a4c0 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -2790,7 +2790,8 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { | ObligationCauseCode::LetElse | ObligationCauseCode::BinOp { .. } | ObligationCauseCode::AscribeUserTypeProvePredicate(..) - | ObligationCauseCode::RustCall => {} + | ObligationCauseCode::RustCall + | ObligationCauseCode::DropImpl => {} ObligationCauseCode::SliceOrArrayElem => { err.note("slice and array elements must have `Sized` type"); } From 9d44f9b4e27bbb83108cde6e3dfffbeca7d4e71e Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 20 Apr 2023 04:04:25 +0000 Subject: [PATCH 06/12] Add test for #110557 --- .../drop-impl-pred.no.stderr | 24 ++++++++++++++++++ .../non_lifetime_binders/drop-impl-pred.rs | 25 +++++++++++++++++++ .../drop-impl-pred.yes.stderr | 11 ++++++++ 3 files changed, 60 insertions(+) create mode 100644 tests/ui/traits/non_lifetime_binders/drop-impl-pred.no.stderr create mode 100644 tests/ui/traits/non_lifetime_binders/drop-impl-pred.rs create mode 100644 tests/ui/traits/non_lifetime_binders/drop-impl-pred.yes.stderr diff --git a/tests/ui/traits/non_lifetime_binders/drop-impl-pred.no.stderr b/tests/ui/traits/non_lifetime_binders/drop-impl-pred.no.stderr new file mode 100644 index 0000000000000..a985b1a6e12f6 --- /dev/null +++ b/tests/ui/traits/non_lifetime_binders/drop-impl-pred.no.stderr @@ -0,0 +1,24 @@ +warning: the feature `non_lifetime_binders` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/drop-impl-pred.rs:6:12 + | +LL | #![feature(non_lifetime_binders)] + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #108185 for more information + = note: `#[warn(incomplete_features)]` on by default + +error[E0367]: `Drop` impl requires `H: Foo` but the struct it is implemented for does not + --> $DIR/drop-impl-pred.rs:19:15 + | +LL | for H: Foo, + | ^^^ + | +note: the implementor must specify the same requirement + --> $DIR/drop-impl-pred.rs:12:1 + | +LL | struct Bar(T) where T: Foo; + | ^^^^^^^^^^^^^ + +error: aborting due to previous error; 1 warning emitted + +For more information about this error, try `rustc --explain E0367`. diff --git a/tests/ui/traits/non_lifetime_binders/drop-impl-pred.rs b/tests/ui/traits/non_lifetime_binders/drop-impl-pred.rs new file mode 100644 index 0000000000000..c65b5ea9ba493 --- /dev/null +++ b/tests/ui/traits/non_lifetime_binders/drop-impl-pred.rs @@ -0,0 +1,25 @@ +// revisions: no yes +//[yes] check-pass + +// Issue 110557 + +#![feature(non_lifetime_binders)] +//~^ WARN the feature `non_lifetime_binders` is incomplete + +pub trait Foo {} + +#[cfg(no)] +struct Bar(T) where T: Foo; + +#[cfg(yes)] +struct Bar(T) where for H: Foo; + +impl Drop for Bar +where + for H: Foo, +//[no]~^ ERROR `Drop` impl requires `H: Foo` but the struct it is implemented for does not +{ + fn drop(&mut self) {} +} + +fn main() {} diff --git a/tests/ui/traits/non_lifetime_binders/drop-impl-pred.yes.stderr b/tests/ui/traits/non_lifetime_binders/drop-impl-pred.yes.stderr new file mode 100644 index 0000000000000..165cf2ee13da8 --- /dev/null +++ b/tests/ui/traits/non_lifetime_binders/drop-impl-pred.yes.stderr @@ -0,0 +1,11 @@ +warning: the feature `non_lifetime_binders` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/drop-impl-pred.rs:6:12 + | +LL | #![feature(non_lifetime_binders)] + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #108185 for more information + = note: `#[warn(incomplete_features)]` on by default + +warning: 1 warning emitted + From 2e346b6f3f3be75d0e0b536a6a8cf2f82241b3b4 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 21 Apr 2023 18:04:39 +0000 Subject: [PATCH 07/12] Even more tests --- .../dropck/explicit-drop-bounds.bad1.stderr | 35 +++++++++++++++ .../dropck/explicit-drop-bounds.bad2.stderr | 35 +++++++++++++++ tests/ui/dropck/explicit-drop-bounds.rs | 44 +++++++++++++++++++ .../explicit-implied-outlives.bad1.stderr | 15 +++++++ .../explicit-implied-outlives.bad2.stderr | 15 +++++++ tests/ui/dropck/explicit-implied-outlives.rs | 43 ++++++++++++++++++ tests/ui/dropck/transitive-outlives-2.rs | 18 ++++++++ .../ui/dropck/transitive-outlives.bad.stderr | 15 +++++++ tests/ui/dropck/transitive-outlives.rs | 26 +++++++++++ tests/ui/dropck/trivial-impl-bounds.rs | 34 ++++++++++++++ 10 files changed, 280 insertions(+) create mode 100644 tests/ui/dropck/explicit-drop-bounds.bad1.stderr create mode 100644 tests/ui/dropck/explicit-drop-bounds.bad2.stderr create mode 100644 tests/ui/dropck/explicit-drop-bounds.rs create mode 100644 tests/ui/dropck/explicit-implied-outlives.bad1.stderr create mode 100644 tests/ui/dropck/explicit-implied-outlives.bad2.stderr create mode 100644 tests/ui/dropck/explicit-implied-outlives.rs create mode 100644 tests/ui/dropck/transitive-outlives-2.rs create mode 100644 tests/ui/dropck/transitive-outlives.bad.stderr create mode 100644 tests/ui/dropck/transitive-outlives.rs create mode 100644 tests/ui/dropck/trivial-impl-bounds.rs diff --git a/tests/ui/dropck/explicit-drop-bounds.bad1.stderr b/tests/ui/dropck/explicit-drop-bounds.bad1.stderr new file mode 100644 index 0000000000000..3b506c7e7ec13 --- /dev/null +++ b/tests/ui/dropck/explicit-drop-bounds.bad1.stderr @@ -0,0 +1,35 @@ +error[E0277]: the trait bound `T: Copy` is not satisfied + --> $DIR/explicit-drop-bounds.rs:27:18 + | +LL | impl Drop for DropMe + | ^^^^^^^^^ the trait `Copy` is not implemented for `T` + | +note: required by a bound in `DropMe` + --> $DIR/explicit-drop-bounds.rs:7:18 + | +LL | struct DropMe(T); + | ^^^^ required by this bound in `DropMe` +help: consider further restricting type parameter `T` + | +LL | [T; 1]: Copy, T: std::marker::Copy // But `[T; 1]: Copy` does not imply `T: Copy` + | ~~~~~~~~~~~~~~~~~~~~~~ + +error[E0277]: the trait bound `T: Copy` is not satisfied + --> $DIR/explicit-drop-bounds.rs:32:13 + | +LL | fn drop(&mut self) {} + | ^^^^^^^^^ the trait `Copy` is not implemented for `T` + | +note: required by a bound in `DropMe` + --> $DIR/explicit-drop-bounds.rs:7:18 + | +LL | struct DropMe(T); + | ^^^^ required by this bound in `DropMe` +help: consider further restricting type parameter `T` + | +LL | [T; 1]: Copy, T: std::marker::Copy // But `[T; 1]: Copy` does not imply `T: Copy` + | ~~~~~~~~~~~~~~~~~~~~~~ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/dropck/explicit-drop-bounds.bad2.stderr b/tests/ui/dropck/explicit-drop-bounds.bad2.stderr new file mode 100644 index 0000000000000..832af3e521a9d --- /dev/null +++ b/tests/ui/dropck/explicit-drop-bounds.bad2.stderr @@ -0,0 +1,35 @@ +error[E0277]: the trait bound `T: Copy` is not satisfied + --> $DIR/explicit-drop-bounds.rs:37:18 + | +LL | impl Drop for DropMe + | ^^^^^^^^^ the trait `Copy` is not implemented for `T` + | +note: required by a bound in `DropMe` + --> $DIR/explicit-drop-bounds.rs:7:18 + | +LL | struct DropMe(T); + | ^^^^ required by this bound in `DropMe` +help: consider restricting type parameter `T` + | +LL | impl Drop for DropMe + | +++++++++++++++++++ + +error[E0277]: the trait bound `T: Copy` is not satisfied + --> $DIR/explicit-drop-bounds.rs:40:13 + | +LL | fn drop(&mut self) {} + | ^^^^^^^^^ the trait `Copy` is not implemented for `T` + | +note: required by a bound in `DropMe` + --> $DIR/explicit-drop-bounds.rs:7:18 + | +LL | struct DropMe(T); + | ^^^^ required by this bound in `DropMe` +help: consider restricting type parameter `T` + | +LL | impl Drop for DropMe + | +++++++++++++++++++ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/dropck/explicit-drop-bounds.rs b/tests/ui/dropck/explicit-drop-bounds.rs new file mode 100644 index 0000000000000..ab6f33c09994e --- /dev/null +++ b/tests/ui/dropck/explicit-drop-bounds.rs @@ -0,0 +1,44 @@ +// revisions: good1 good2 bad1 bad2 +//[good1] check-pass +//[good2] check-pass + +use std::ops::Drop; + +struct DropMe(T); + +#[cfg(good1)] +impl Drop for DropMe +where + T: Copy + Clone, +{ + fn drop(&mut self) {} +} + +#[cfg(good2)] +impl Drop for DropMe +where + T: Copy, + [T; 1]: Copy, // Trivial bound implied by `T: Copy` +{ + fn drop(&mut self) {} +} + +#[cfg(bad1)] +impl Drop for DropMe +//[bad1]~^ ERROR the trait bound `T: Copy` is not satisfied +where + [T; 1]: Copy, // But `[T; 1]: Copy` does not imply `T: Copy` +{ + fn drop(&mut self) {} + //[bad1]~^ ERROR the trait bound `T: Copy` is not satisfied +} + +#[cfg(bad2)] +impl Drop for DropMe +//[bad2]~^ ERROR the trait bound `T: Copy` is not satisfied +{ + fn drop(&mut self) {} + //[bad2]~^ ERROR the trait bound `T: Copy` is not satisfied +} + +fn main() {} diff --git a/tests/ui/dropck/explicit-implied-outlives.bad1.stderr b/tests/ui/dropck/explicit-implied-outlives.bad1.stderr new file mode 100644 index 0000000000000..bf6d70e7d3758 --- /dev/null +++ b/tests/ui/dropck/explicit-implied-outlives.bad1.stderr @@ -0,0 +1,15 @@ +error[E0367]: `Drop` impl requires `T: 'static` but the struct it is implemented for does not + --> $DIR/explicit-implied-outlives.rs:28:8 + | +LL | T: 'static, + | ^^^^^^^ + | +note: the implementor must specify the same requirement + --> $DIR/explicit-implied-outlives.rs:7:1 + | +LL | struct DropMe<'a, T>(&'a T); + | ^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0367`. diff --git a/tests/ui/dropck/explicit-implied-outlives.bad2.stderr b/tests/ui/dropck/explicit-implied-outlives.bad2.stderr new file mode 100644 index 0000000000000..27a15170bddb5 --- /dev/null +++ b/tests/ui/dropck/explicit-implied-outlives.bad2.stderr @@ -0,0 +1,15 @@ +error[E0367]: `Drop` impl requires `'a: 'static` but the struct it is implemented for does not + --> $DIR/explicit-implied-outlives.rs:37:9 + | +LL | 'a: 'static, + | ^^^^^^^ + | +note: the implementor must specify the same requirement + --> $DIR/explicit-implied-outlives.rs:7:1 + | +LL | struct DropMe<'a, T>(&'a T); + | ^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0367`. diff --git a/tests/ui/dropck/explicit-implied-outlives.rs b/tests/ui/dropck/explicit-implied-outlives.rs new file mode 100644 index 0000000000000..fa446591f3dc4 --- /dev/null +++ b/tests/ui/dropck/explicit-implied-outlives.rs @@ -0,0 +1,43 @@ +// revisions: good1 good2 bad1 bad2 +//[good1] check-pass +//[good2] check-pass + +use std::ops::Drop; + +struct DropMe<'a, T>(&'a T); + +#[cfg(good1)] +impl<'a, T> Drop for DropMe<'a, T> +where + T: 'a, // Implied by struct, explicit on impl +{ + fn drop(&mut self) {} +} + +#[cfg(good2)] +impl<'a, T> Drop for DropMe<'a, T> +where + 'static: 'a, // Trivial bound +{ + fn drop(&mut self) {} +} + +#[cfg(bad1)] +impl<'a, T> Drop for DropMe<'a, T> +where + T: 'static, + //[bad1]~^ ERROR `Drop` impl requires `T: 'static` +{ + fn drop(&mut self) {} +} + +#[cfg(bad2)] +impl<'a, T> Drop for DropMe<'a, T> +where + 'a: 'static, + //[bad2]~^ ERROR `Drop` impl requires `'a: 'static` +{ + fn drop(&mut self) {} +} + +fn main() {} diff --git a/tests/ui/dropck/transitive-outlives-2.rs b/tests/ui/dropck/transitive-outlives-2.rs new file mode 100644 index 0000000000000..87154e25d4091 --- /dev/null +++ b/tests/ui/dropck/transitive-outlives-2.rs @@ -0,0 +1,18 @@ +// check-pass + +use std::marker::PhantomData; +use std::ops::Drop; + +// a >= b >= c >= a implies a = b = c +struct DropMe<'a: 'b, 'b: 'c, 'c: 'a>( + PhantomData<&'a ()>, + PhantomData<&'b ()>, + PhantomData<&'c ()>, +); + +// a >= b, a >= c, b >= a, c >= a implies a = b = c +impl<'a: 'b + 'c, 'b: 'a, 'c: 'a> Drop for DropMe<'a, 'b, 'c> { + fn drop(&mut self) {} +} + +fn main() {} diff --git a/tests/ui/dropck/transitive-outlives.bad.stderr b/tests/ui/dropck/transitive-outlives.bad.stderr new file mode 100644 index 0000000000000..da5088b27b414 --- /dev/null +++ b/tests/ui/dropck/transitive-outlives.bad.stderr @@ -0,0 +1,15 @@ +error[E0367]: `Drop` impl requires `'a: 'c` but the struct it is implemented for does not + --> $DIR/transitive-outlives.rs:20:9 + | +LL | 'a: 'c, + | ^^ + | +note: the implementor must specify the same requirement + --> $DIR/transitive-outlives.rs:7:1 + | +LL | struct DropMe<'a, 'b: 'a, 'c: 'b>(PhantomData<&'a ()>, PhantomData<&'b ()>, PhantomData<&'c ()>); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0367`. diff --git a/tests/ui/dropck/transitive-outlives.rs b/tests/ui/dropck/transitive-outlives.rs new file mode 100644 index 0000000000000..d071664abdeb6 --- /dev/null +++ b/tests/ui/dropck/transitive-outlives.rs @@ -0,0 +1,26 @@ +// revisions: good bad +//[good] check-pass + +use std::marker::PhantomData; +use std::ops::Drop; + +struct DropMe<'a, 'b: 'a, 'c: 'b>(PhantomData<&'a ()>, PhantomData<&'b ()>, PhantomData<&'c ()>); + +#[cfg(good)] +impl<'a, 'b, 'c> Drop for DropMe<'a, 'b, 'c> +where + 'c: 'a, +{ + fn drop(&mut self) {} +} + +#[cfg(bad)] +impl<'a, 'b, 'c> Drop for DropMe<'a, 'b, 'c> +where + 'a: 'c, + //[bad]~^ ERROR `Drop` impl requires `'a: 'c` +{ + fn drop(&mut self) {} +} + +fn main() {} diff --git a/tests/ui/dropck/trivial-impl-bounds.rs b/tests/ui/dropck/trivial-impl-bounds.rs new file mode 100644 index 0000000000000..a8f5d2c354bc9 --- /dev/null +++ b/tests/ui/dropck/trivial-impl-bounds.rs @@ -0,0 +1,34 @@ +// revisions: good1 good2 good3 +// check-pass + +use std::ops::Drop; + +struct Foo; + +const X: usize = 1; + +#[cfg(good1)] +impl Drop for Foo +where + [(); X]:, // Trivial WF bound +{ + fn drop(&mut self) {} +} + +#[cfg(good2)] +impl Drop for Foo +where + for<'a> &'a (): Copy, // Trivial trait bound +{ + fn drop(&mut self) {} +} + +#[cfg(good3)] +impl Drop for Foo +where + for<'a> &'a (): 'a, // Trivial outlives bound +{ + fn drop(&mut self) {} +} + +fn main() {} From 4b85bea4aeb920ebd2f70eb676ca64d3c7361bac Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Mon, 24 Apr 2023 17:53:51 -0300 Subject: [PATCH 08/12] Add Assert terminator to SMIR --- Cargo.lock | 1 + compiler/rustc_smir/Cargo.toml | 1 + compiler/rustc_smir/src/rustc_smir/mod.rs | 118 ++++++++++++++++-- .../rustc_smir/src/stable_mir/mir/body.rs | 67 +++++++++- tests/ui-fulldeps/stable-mir/crate-info.rs | 15 ++- 5 files changed, 191 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d7806b5daa63a..c7a91ef71166d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4090,6 +4090,7 @@ dependencies = [ name = "rustc_smir" version = "0.0.0" dependencies = [ + "rustc_hir", "rustc_middle", "rustc_span", "tracing", diff --git a/compiler/rustc_smir/Cargo.toml b/compiler/rustc_smir/Cargo.toml index fb97ee5bebe6e..80360a3c73f8d 100644 --- a/compiler/rustc_smir/Cargo.toml +++ b/compiler/rustc_smir/Cargo.toml @@ -4,6 +4,7 @@ version = "0.0.0" edition = "2021" [dependencies] +rustc_hir = { path = "../rustc_hir" } rustc_middle = { path = "../rustc_middle", optional = true } rustc_span = { path = "../rustc_span", optional = true } tracing = "0.1" diff --git a/compiler/rustc_smir/src/rustc_smir/mod.rs b/compiler/rustc_smir/src/rustc_smir/mod.rs index 57ceb89969dd4..efbb3b321d5a7 100644 --- a/compiler/rustc_smir/src/rustc_smir/mod.rs +++ b/compiler/rustc_smir/src/rustc_smir/mod.rs @@ -93,10 +93,10 @@ fn rustc_statement_to_statement( } } -fn rustc_rvalue_to_rvalue(rvalue: &rustc_middle::mir::Rvalue<'_>) -> stable_mir::mir::Operand { +fn rustc_rvalue_to_rvalue(rvalue: &rustc_middle::mir::Rvalue<'_>) -> stable_mir::mir::Rvalue { use rustc_middle::mir::Rvalue::*; match rvalue { - Use(op) => rustc_op_to_op(op), + Use(op) => stable_mir::mir::Rvalue::Use(rustc_op_to_op(op)), Repeat(_, _) => todo!(), Ref(_, _, _) => todo!(), ThreadLocalRef(_) => todo!(), @@ -104,9 +104,15 @@ fn rustc_rvalue_to_rvalue(rvalue: &rustc_middle::mir::Rvalue<'_>) -> stable_mir: Len(_) => todo!(), Cast(_, _, _) => todo!(), BinaryOp(_, _) => todo!(), - CheckedBinaryOp(_, _) => todo!(), + CheckedBinaryOp(bin_op, ops) => stable_mir::mir::Rvalue::CheckedBinaryOp( + rustc_bin_op_to_bin_op(bin_op), + rustc_op_to_op(&ops.0), + rustc_op_to_op(&ops.1), + ), NullaryOp(_, _) => todo!(), - UnaryOp(_, _) => todo!(), + UnaryOp(un_op, op) => { + stable_mir::mir::Rvalue::UnaryOp(rustc_un_op_to_un_op(un_op), rustc_op_to_op(op)) + } Discriminant(_) => todo!(), Aggregate(_, _) => todo!(), ShallowInitBox(_, _) => todo!(), @@ -124,8 +130,10 @@ fn rustc_op_to_op(op: &rustc_middle::mir::Operand<'_>) -> stable_mir::mir::Opera } fn rustc_place_to_place(place: &rustc_middle::mir::Place<'_>) -> stable_mir::mir::Place { - assert_eq!(&place.projection[..], &[]); - stable_mir::mir::Place { local: place.local.as_usize() } + stable_mir::mir::Place { + local: place.local.as_usize(), + projection: format!("{:?}", place.projection), + } } fn rustc_unwind_to_unwind( @@ -140,6 +148,96 @@ fn rustc_unwind_to_unwind( } } +fn rustc_assert_msg_to_msg<'tcx>( + assert_message: &rustc_middle::mir::AssertMessage<'tcx>, +) -> stable_mir::mir::AssertMessage { + use rustc_middle::mir::AssertKind; + match assert_message { + AssertKind::BoundsCheck { len, index } => stable_mir::mir::AssertMessage::BoundsCheck { + len: rustc_op_to_op(len), + index: rustc_op_to_op(index), + }, + AssertKind::Overflow(bin_op, op1, op2) => stable_mir::mir::AssertMessage::Overflow( + rustc_bin_op_to_bin_op(bin_op), + rustc_op_to_op(op1), + rustc_op_to_op(op2), + ), + AssertKind::OverflowNeg(op) => { + stable_mir::mir::AssertMessage::OverflowNeg(rustc_op_to_op(op)) + } + AssertKind::DivisionByZero(op) => { + stable_mir::mir::AssertMessage::DivisionByZero(rustc_op_to_op(op)) + } + AssertKind::RemainderByZero(op) => { + stable_mir::mir::AssertMessage::RemainderByZero(rustc_op_to_op(op)) + } + AssertKind::ResumedAfterReturn(generator) => { + stable_mir::mir::AssertMessage::ResumedAfterReturn(rustc_generator_to_generator( + generator, + )) + } + AssertKind::ResumedAfterPanic(generator) => { + stable_mir::mir::AssertMessage::ResumedAfterPanic(rustc_generator_to_generator( + generator, + )) + } + AssertKind::MisalignedPointerDereference { required, found } => { + stable_mir::mir::AssertMessage::MisalignedPointerDereference { + required: rustc_op_to_op(required), + found: rustc_op_to_op(found), + } + } + } +} + +fn rustc_bin_op_to_bin_op(bin_op: &rustc_middle::mir::BinOp) -> stable_mir::mir::BinOp { + use rustc_middle::mir::BinOp; + match bin_op { + BinOp::Add => stable_mir::mir::BinOp::Add, + BinOp::Sub => stable_mir::mir::BinOp::Sub, + BinOp::Mul => stable_mir::mir::BinOp::Mul, + BinOp::Div => stable_mir::mir::BinOp::Div, + BinOp::Rem => stable_mir::mir::BinOp::Rem, + BinOp::BitXor => stable_mir::mir::BinOp::BitXor, + BinOp::BitAnd => stable_mir::mir::BinOp::BitAnd, + BinOp::BitOr => stable_mir::mir::BinOp::BitOr, + BinOp::Shl => stable_mir::mir::BinOp::Shl, + BinOp::Shr => stable_mir::mir::BinOp::Shr, + BinOp::Eq => stable_mir::mir::BinOp::Eq, + BinOp::Lt => stable_mir::mir::BinOp::Lt, + BinOp::Le => stable_mir::mir::BinOp::Le, + BinOp::Ne => stable_mir::mir::BinOp::Ne, + BinOp::Ge => stable_mir::mir::BinOp::Ge, + BinOp::Gt => stable_mir::mir::BinOp::Gt, + BinOp::Offset => stable_mir::mir::BinOp::Offset, + } +} + +fn rustc_un_op_to_un_op(unary_op: &rustc_middle::mir::UnOp) -> stable_mir::mir::UnOp { + use rustc_middle::mir::UnOp; + match unary_op { + UnOp::Not => stable_mir::mir::UnOp::Not, + UnOp::Neg => stable_mir::mir::UnOp::Neg, + } +} + +fn rustc_generator_to_generator( + generator: &rustc_hir::GeneratorKind, +) -> stable_mir::mir::GeneratorKind { + use rustc_hir::{AsyncGeneratorKind, GeneratorKind}; + match generator { + GeneratorKind::Async(async_gen) => { + let async_gen = match async_gen { + AsyncGeneratorKind::Block => stable_mir::mir::AsyncGeneratorKind::Block, + AsyncGeneratorKind::Closure => stable_mir::mir::AsyncGeneratorKind::Closure, + AsyncGeneratorKind::Fn => stable_mir::mir::AsyncGeneratorKind::Fn, + }; + stable_mir::mir::GeneratorKind::Async(async_gen) + } + GeneratorKind::Gen => stable_mir::mir::GeneratorKind::Gen, + } +} + fn rustc_terminator_to_terminator( terminator: &rustc_middle::mir::Terminator<'_>, ) -> stable_mir::mir::Terminator { @@ -176,7 +274,13 @@ fn rustc_terminator_to_terminator( unwind: rustc_unwind_to_unwind(unwind), } } - Assert { .. } => todo!(), + Assert { cond, expected, msg, target, unwind } => Terminator::Assert { + cond: rustc_op_to_op(cond), + expected: *expected, + msg: rustc_assert_msg_to_msg(msg), + target: target.as_usize(), + unwind: rustc_unwind_to_unwind(unwind), + }, Yield { .. } => todo!(), GeneratorDrop => todo!(), FalseEdge { .. } => todo!(), diff --git a/compiler/rustc_smir/src/stable_mir/mir/body.rs b/compiler/rustc_smir/src/stable_mir/mir/body.rs index b3d0a83573357..699c945738103 100644 --- a/compiler/rustc_smir/src/stable_mir/mir/body.rs +++ b/compiler/rustc_smir/src/stable_mir/mir/body.rs @@ -38,9 +38,9 @@ pub enum Terminator { Assert { cond: Operand, expected: bool, - msg: String, + msg: AssertMessage, target: usize, - cleanup: Option, + unwind: UnwindAction, }, } @@ -52,12 +52,72 @@ pub enum UnwindAction { Cleanup(usize), } +#[derive(Clone, Debug)] +pub enum AssertMessage { + BoundsCheck { len: Operand, index: Operand }, + Overflow(BinOp, Operand, Operand), + OverflowNeg(Operand), + DivisionByZero(Operand), + RemainderByZero(Operand), + ResumedAfterReturn(GeneratorKind), + ResumedAfterPanic(GeneratorKind), + MisalignedPointerDereference { required: Operand, found: Operand }, +} + +#[derive(Clone, Debug)] +pub enum BinOp { + Add, + Sub, + Mul, + Div, + Rem, + BitXor, + BitAnd, + BitOr, + Shl, + Shr, + Eq, + Lt, + Le, + Ne, + Ge, + Gt, + Offset, +} + +#[derive(Clone, Debug)] +pub enum UnOp { + Not, + Neg, +} + +#[derive(Clone, Debug)] +pub enum GeneratorKind { + Async(AsyncGeneratorKind), + Gen, +} + +#[derive(Clone, Debug)] +pub enum AsyncGeneratorKind { + Block, + Closure, + Fn, +} + #[derive(Clone, Debug)] pub enum Statement { - Assign(Place, Operand), + Assign(Place, Rvalue), Nop, } +// FIXME this is incomplete +#[derive(Clone, Debug)] +pub enum Rvalue { + Use(Operand), + CheckedBinaryOp(BinOp, Operand, Operand), + UnaryOp(UnOp, Operand), +} + #[derive(Clone, Debug)] pub enum Operand { Copy(Place), @@ -68,6 +128,7 @@ pub enum Operand { #[derive(Clone, Debug)] pub struct Place { pub local: usize, + pub projection: String, } #[derive(Clone, Debug)] diff --git a/tests/ui-fulldeps/stable-mir/crate-info.rs b/tests/ui-fulldeps/stable-mir/crate-info.rs index f89690e1d1592..1454d6dde6c97 100644 --- a/tests/ui-fulldeps/stable-mir/crate-info.rs +++ b/tests/ui-fulldeps/stable-mir/crate-info.rs @@ -69,6 +69,15 @@ fn test_stable_mir(tcx: TyCtxt<'_>) { stable_mir::mir::Terminator::Drop { .. } => {} other => panic!("{other:?}"), } + + let assert = get_item(tcx, &items, (DefKind::Fn, "assert")).unwrap(); + let body = assert.body(); + assert_eq!(body.blocks.len(), 2); + let block = &body.blocks[0]; + match &block.terminator { + stable_mir::mir::Terminator::Assert { .. } => {} + other => panic!("{other:?}"), + } } // Use internal API to find a function in a crate. @@ -142,7 +151,11 @@ fn generate_input(path: &str) -> std::io::Result<()> { x_64.wrapping_add(y_64) }} - pub fn drop(_: String) {{}}"# + pub fn drop(_: String) {{}} + + pub fn assert(x: i32) -> i32 {{ + x + 1 + }}"# )?; Ok(()) } From 698acc645e08078edfe70ad8651c0874a7933052 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Mon, 24 Apr 2023 18:17:24 -0300 Subject: [PATCH 09/12] Add GeneratorDrop terminator to SMIR --- compiler/rustc_smir/src/rustc_smir/mod.rs | 2 +- compiler/rustc_smir/src/stable_mir/mir/body.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_smir/src/rustc_smir/mod.rs b/compiler/rustc_smir/src/rustc_smir/mod.rs index efbb3b321d5a7..241cd182059ba 100644 --- a/compiler/rustc_smir/src/rustc_smir/mod.rs +++ b/compiler/rustc_smir/src/rustc_smir/mod.rs @@ -282,7 +282,7 @@ fn rustc_terminator_to_terminator( unwind: rustc_unwind_to_unwind(unwind), }, Yield { .. } => todo!(), - GeneratorDrop => todo!(), + GeneratorDrop => Terminator::GeneratorDrop, FalseEdge { .. } => todo!(), FalseUnwind { .. } => todo!(), InlineAsm { .. } => todo!(), diff --git a/compiler/rustc_smir/src/stable_mir/mir/body.rs b/compiler/rustc_smir/src/stable_mir/mir/body.rs index 699c945738103..4baf3f1f75eac 100644 --- a/compiler/rustc_smir/src/stable_mir/mir/body.rs +++ b/compiler/rustc_smir/src/stable_mir/mir/body.rs @@ -42,6 +42,7 @@ pub enum Terminator { target: usize, unwind: UnwindAction, }, + GeneratorDrop, } #[derive(Clone, Debug)] From a183ac6f90dc43a550a8afa5c01fdc797cf3d1d4 Mon Sep 17 00:00:00 2001 From: Zachary Mayhew Date: Thu, 4 May 2023 23:31:04 -0400 Subject: [PATCH 10/12] add hint for =< as <= --- compiler/rustc_parse/src/parser/expr.rs | 13 ++++++++- tests/ui/parser/eq-less-to-less-eq.rs | 33 ++++++++++++++++++++++ tests/ui/parser/eq-less-to-less-eq.stderr | 34 +++++++++++++++++++++++ 3 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 tests/ui/parser/eq-less-to-less-eq.rs create mode 100644 tests/ui/parser/eq-less-to-less-eq.stderr diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index f58f8919e5c9d..dd1b0ddc025d8 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -1448,8 +1448,19 @@ impl<'a> Parser<'a> { } fn parse_expr_path_start(&mut self) -> PResult<'a, P> { + let maybe_eq_tok = self.prev_token.clone(); let (qself, path) = if self.eat_lt() { - let (qself, path) = self.parse_qpath(PathStyle::Expr)?; + let lt_span = self.prev_token.span; + let (qself, path) = self.parse_qpath(PathStyle::Expr).map_err(|mut err| { + // Suggests using '<=' if there is an error parsing qpath when the previous token + // is an '=' token. Only emits suggestion if the '<' token and '=' token are + // directly adjacent (i.e. '=<') + if maybe_eq_tok.kind == TokenKind::Eq && maybe_eq_tok.span.hi() == lt_span.lo() { + let eq_lt = maybe_eq_tok.span.to(lt_span); + err.span_suggestion(eq_lt, "did you mean", "<=", Applicability::Unspecified); + } + err + })?; (Some(qself), path) } else { (None, self.parse_path(PathStyle::Expr)?) diff --git a/tests/ui/parser/eq-less-to-less-eq.rs b/tests/ui/parser/eq-less-to-less-eq.rs new file mode 100644 index 0000000000000..23c6c59d7a62b --- /dev/null +++ b/tests/ui/parser/eq-less-to-less-eq.rs @@ -0,0 +1,33 @@ +fn foo() { + let a = 0; + let b = 4; + if a =< b { //~ERROR + println!("yay!"); + } +} + +fn bar() { + let a = 0; + let b = 4; + if a = ::abs(-4) { //~ERROR: mismatched types + println!("yay!"); + } +} + +fn main() {} diff --git a/tests/ui/parser/eq-less-to-less-eq.stderr b/tests/ui/parser/eq-less-to-less-eq.stderr new file mode 100644 index 0000000000000..4717d8287ff7b --- /dev/null +++ b/tests/ui/parser/eq-less-to-less-eq.stderr @@ -0,0 +1,34 @@ +error: expected one of `!`, `(`, `+`, `::`, `<`, `>`, or `as`, found `{` + --> $DIR/eq-less-to-less-eq.rs:4:15 + | +LL | if a =< b { + | -- ^ expected one of 7 possible tokens + | | + | help: did you mean: `<=` + +error: expected one of `!`, `(`, `+`, `::`, `<`, `>`, or `as`, found `{` + --> $DIR/eq-less-to-less-eq.rs:12:15 + | +LL | if a = `, or `as`, found `{` + --> $DIR/eq-less-to-less-eq.rs:20:16 + | +LL | if a = < b { + | ^ expected one of 7 possible tokens + +error[E0308]: mismatched types + --> $DIR/eq-less-to-less-eq.rs:28:8 + | +LL | if a =< i32>::abs(-4) { + | ^^^^^^^^^^^^^^^^^^ expected `bool`, found `()` + | +help: you might have meant to compare for equality + | +LL | if a ==< i32>::abs(-4) { + | + + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0308`. From 2a1ef34223a17dbe6192ccba13d2ec4bd57a56b9 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 6 May 2023 05:58:00 +0000 Subject: [PATCH 11/12] More robust debug assertions for `Instance::resolve` on built-in traits with custom items --- compiler/rustc_middle/src/ty/instance.rs | 2 +- compiler/rustc_span/src/symbol.rs | 1 + compiler/rustc_ty_utils/src/instance.rs | 84 ++++++++++++++++++++---- 3 files changed, 73 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_middle/src/ty/instance.rs b/compiler/rustc_middle/src/ty/instance.rs index cc86cba6fda72..6c8f4af759434 100644 --- a/compiler/rustc_middle/src/ty/instance.rs +++ b/compiler/rustc_middle/src/ty/instance.rs @@ -385,7 +385,7 @@ impl<'tcx> Instance<'tcx> { /// couldn't complete due to errors elsewhere - this is distinct /// from `Ok(None)` to avoid misleading diagnostics when an error /// has already been/will be emitted, for the original cause - #[instrument(level = "debug", skip(tcx))] + #[instrument(level = "debug", skip(tcx), ret)] pub fn resolve( tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 7969b848fd956..413a958fa2eed 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1203,6 +1203,7 @@ symbols! { require, residual, result, + resume, return_position_impl_trait_in_trait, return_type_notation, rhs, diff --git a/compiler/rustc_ty_utils/src/instance.rs b/compiler/rustc_ty_utils/src/instance.rs index eedf459ce8fbc..b10aaad5f2af4 100644 --- a/compiler/rustc_ty_utils/src/instance.rs +++ b/compiler/rustc_ty_utils/src/instance.rs @@ -177,15 +177,55 @@ fn resolve_associated_item<'tcx>( Some(ty::Instance::new(leaf_def.item.def_id, substs)) } - traits::ImplSource::Generator(generator_data) => Some(Instance { - def: ty::InstanceDef::Item(generator_data.generator_def_id), - substs: generator_data.substs, - }), - traits::ImplSource::Future(future_data) => Some(Instance { - def: ty::InstanceDef::Item(future_data.generator_def_id), - substs: future_data.substs, - }), + traits::ImplSource::Generator(generator_data) => { + if cfg!(debug_assertions) && tcx.item_name(trait_item_id) != sym::resume { + // For compiler developers who'd like to add new items to `Generator`, + // you either need to generate a shim body, or perhaps return + // `InstanceDef::Item` pointing to a trait default method body if + // it is given a default implementation by the trait. + span_bug!( + tcx.def_span(generator_data.generator_def_id), + "no definition for `{trait_ref}::{}` for built-in generator type", + tcx.item_name(trait_item_id) + ) + } + Some(Instance { + def: ty::InstanceDef::Item(generator_data.generator_def_id), + substs: generator_data.substs, + }) + } + traits::ImplSource::Future(future_data) => { + if cfg!(debug_assertions) && tcx.item_name(trait_item_id) != sym::poll { + // For compiler developers who'd like to add new items to `Future`, + // you either need to generate a shim body, or perhaps return + // `InstanceDef::Item` pointing to a trait default method body if + // it is given a default implementation by the trait. + span_bug!( + tcx.def_span(future_data.generator_def_id), + "no definition for `{trait_ref}::{}` for built-in async generator type", + tcx.item_name(trait_item_id) + ) + } + Some(Instance { + def: ty::InstanceDef::Item(future_data.generator_def_id), + substs: future_data.substs, + }) + } traits::ImplSource::Closure(closure_data) => { + if cfg!(debug_assertions) + && ![sym::call, sym::call_mut, sym::call_once] + .contains(&tcx.item_name(trait_item_id)) + { + // For compiler developers who'd like to add new items to `Fn`/`FnMut`/`FnOnce`, + // you either need to generate a shim body, or perhaps return + // `InstanceDef::Item` pointing to a trait default method body if + // it is given a default implementation by the trait. + span_bug!( + tcx.def_span(closure_data.closure_def_id), + "no definition for `{trait_ref}::{}` for built-in closure type", + tcx.item_name(trait_item_id) + ) + } let trait_closure_kind = tcx.fn_trait_kind_from_def_id(trait_id).unwrap(); Instance::resolve_closure( tcx, @@ -195,11 +235,29 @@ fn resolve_associated_item<'tcx>( ) } traits::ImplSource::FnPointer(ref data) => match data.fn_ty.kind() { - ty::FnDef(..) | ty::FnPtr(..) => Some(Instance { - def: ty::InstanceDef::FnPtrShim(trait_item_id, data.fn_ty), - substs: rcvr_substs, - }), - _ => None, + ty::FnDef(..) | ty::FnPtr(..) => { + if cfg!(debug_assertions) + && ![sym::call, sym::call_mut, sym::call_once] + .contains(&tcx.item_name(trait_item_id)) + { + // For compiler developers who'd like to add new items to `Fn`/`FnMut`/`FnOnce`, + // you either need to generate a shim body, or perhaps return + // `InstanceDef::Item` pointing to a trait default method body if + // it is given a default implementation by the trait. + bug!( + "no definition for `{trait_ref}::{}` for built-in fn type", + tcx.item_name(trait_item_id) + ) + } + Some(Instance { + def: ty::InstanceDef::FnPtrShim(trait_item_id, data.fn_ty), + substs: rcvr_substs, + }) + } + _ => bug!( + "no built-in definition for `{trait_ref}::{}` for non-fn type", + tcx.item_name(trait_item_id) + ), }, traits::ImplSource::Object(ref data) => { traits::get_vtable_index_of_object_method(tcx, data, trait_item_id).map(|index| { From bba2a1e07179c59b422e60411813446606b1b7f3 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Sat, 29 Apr 2023 12:01:13 +0100 Subject: [PATCH 12/12] Fix spans in LLVM-generated inline asm errors Previously, incorrect spans were reported if inline assembly contained CRLF (Windows) line endings. Fixes #110885 --- compiler/rustc_codegen_ssa/src/back/write.rs | 12 ++++++++--- compiler/rustc_span/src/lib.rs | 22 ++++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index c80347448cbb2..d0bed71bd12a5 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -1821,9 +1821,15 @@ impl SharedEmitterMain { let source = sess .source_map() .new_source_file(FileName::inline_asm_source_code(&buffer), buffer); - let source_span = Span::with_root_ctxt(source.start_pos, source.end_pos); - let spans: Vec<_> = - spans.iter().map(|sp| source_span.from_inner(*sp)).collect(); + let spans: Vec<_> = spans + .iter() + .map(|sp| { + Span::with_root_ctxt( + source.normalized_byte_pos(sp.start as u32), + source.normalized_byte_pos(sp.end as u32), + ) + }) + .collect(); err.span_note(spans, "instantiated into assembly here"); } diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index 8a900ca427ebe..fa89992f79a1f 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -1745,6 +1745,28 @@ impl SourceFile { BytePos::from_u32(pos.0 - self.start_pos.0 + diff) } + /// Calculates a normalized byte position from a byte offset relative to the + /// start of the file. + /// + /// When we get an inline assembler error from LLVM during codegen, we + /// import the expanded assembly code as a new `SourceFile`, which can then + /// be used for error reporting with spans. However the byte offsets given + /// to us by LLVM are relative to the start of the original buffer, not the + /// normalized one. Hence we need to convert those offsets to the normalized + /// form when constructing spans. + pub fn normalized_byte_pos(&self, offset: u32) -> BytePos { + let diff = match self + .normalized_pos + .binary_search_by(|np| (np.pos.0 + np.diff).cmp(&(self.start_pos.0 + offset))) + { + Ok(i) => self.normalized_pos[i].diff, + Err(i) if i == 0 => 0, + Err(i) => self.normalized_pos[i - 1].diff, + }; + + BytePos::from_u32(self.start_pos.0 + offset - diff) + } + /// Converts an absolute `BytePos` to a `CharPos` relative to the `SourceFile`. pub fn bytepos_to_file_charpos(&self, bpos: BytePos) -> CharPos { // The number of extra bytes due to multibyte chars in the `SourceFile`.