From 2538c0c17095834c994cbfdc4909338a31f83fb7 Mon Sep 17 00:00:00 2001 From: Ibraheem Ahmed Date: Tue, 10 Jan 2023 21:39:02 -0500 Subject: [PATCH 01/16] fix `SyncSender` spinning behavior --- library/std/src/sync/mpmc/utils.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sync/mpmc/utils.rs b/library/std/src/sync/mpmc/utils.rs index e030c55ce8f6..a9b365daeec4 100644 --- a/library/std/src/sync/mpmc/utils.rs +++ b/library/std/src/sync/mpmc/utils.rs @@ -139,6 +139,6 @@ impl Backoff { /// Returns `true` if quadratic backoff has completed and blocking the thread is advised. #[inline] pub fn is_completed(&self) -> bool { - self.step.get() > YIELD_LIMIT + self.step.get() > SPIN_LIMIT } } From f8276c94ac06b272e88fb1bb9c5f6615fc5876ef Mon Sep 17 00:00:00 2001 From: Ibraheem Ahmed Date: Tue, 10 Jan 2023 21:54:53 -0500 Subject: [PATCH 02/16] add `SyncSender::send_timeout` test --- library/std/src/sync/mpmc/mod.rs | 2 +- library/std/src/sync/mpsc/mod.rs | 9 +++++++++ library/std/src/sync/mpsc/sync_tests.rs | 8 ++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/library/std/src/sync/mpmc/mod.rs b/library/std/src/sync/mpmc/mod.rs index cef99c588430..7a602cecd3b8 100644 --- a/library/std/src/sync/mpmc/mod.rs +++ b/library/std/src/sync/mpmc/mod.rs @@ -43,7 +43,7 @@ mod zero; use crate::fmt; use crate::panic::{RefUnwindSafe, UnwindSafe}; use crate::time::{Duration, Instant}; -use error::*; +pub use error::*; /// Creates a channel of unbounded capacity. /// diff --git a/library/std/src/sync/mpsc/mod.rs b/library/std/src/sync/mpsc/mod.rs index adb488d4378f..6e3c28f10bb1 100644 --- a/library/std/src/sync/mpsc/mod.rs +++ b/library/std/src/sync/mpsc/mod.rs @@ -738,6 +738,15 @@ impl SyncSender { pub fn try_send(&self, t: T) -> Result<(), TrySendError> { self.inner.try_send(t) } + + // Attempts to send for a value on this receiver, returning an error if the + // corresponding channel has hung up, or if it waits more than `timeout`. + // + // This method is currently private and only used for tests. + #[allow(unused)] + fn send_timeout(&self, t: T, timeout: Duration) -> Result<(), mpmc::SendTimeoutError> { + self.inner.send_timeout(t, timeout) + } } #[stable(feature = "rust1", since = "1.0.0")] diff --git a/library/std/src/sync/mpsc/sync_tests.rs b/library/std/src/sync/mpsc/sync_tests.rs index 63c79436974d..9d2f92ffc9b1 100644 --- a/library/std/src/sync/mpsc/sync_tests.rs +++ b/library/std/src/sync/mpsc/sync_tests.rs @@ -1,5 +1,6 @@ use super::*; use crate::env; +use crate::sync::mpmc::SendTimeoutError; use crate::thread; use crate::time::Duration; @@ -41,6 +42,13 @@ fn recv_timeout() { assert_eq!(rx.recv_timeout(Duration::from_millis(1)), Ok(1)); } +#[test] +fn send_timeout() { + let (tx, _rx) = sync_channel::(1); + assert_eq!(tx.send_timeout(1, Duration::from_millis(1)), Ok(())); + assert_eq!(tx.send_timeout(1, Duration::from_millis(1)), Err(SendTimeoutError::Timeout(1))); +} + #[test] fn smoke_threads() { let (tx, rx) = sync_channel::(0); From 4e2a3567bc14ad7b7e9d28f31fc1a468976ebc81 Mon Sep 17 00:00:00 2001 From: Yuki Omoto Date: Thu, 12 Jan 2023 00:17:48 +0900 Subject: [PATCH 03/16] Add log-backtrace option to show backtraces along with logging --- Cargo.lock | 1 + compiler/rustc_driver/src/lib.rs | 14 ++++++- compiler/rustc_interface/src/tests.rs | 1 + compiler/rustc_log/Cargo.toml | 1 + compiler/rustc_log/src/lib.rs | 58 ++++++++++++++++++++++++++- compiler/rustc_session/src/options.rs | 2 + tests/rustdoc-ui/z-help.stdout | 1 + tests/ui/attributes/log-backtrace.rs | 9 +++++ 8 files changed, 83 insertions(+), 4 deletions(-) create mode 100644 tests/ui/attributes/log-backtrace.rs diff --git a/Cargo.lock b/Cargo.lock index 2a88152b5194..b0f050a85d2f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4273,6 +4273,7 @@ version = "0.0.0" dependencies = [ "rustc_span", "tracing", + "tracing-core", "tracing-subscriber", "tracing-tree", ] diff --git a/compiler/rustc_driver/src/lib.rs b/compiler/rustc_driver/src/lib.rs index fcb73c64356f..dcce56ba7638 100644 --- a/compiler/rustc_driver/src/lib.rs +++ b/compiler/rustc_driver/src/lib.rs @@ -231,6 +231,10 @@ fn run_compiler( registry: diagnostics_registry(), }; + if !tracing::dispatcher::has_been_set() { + init_rustc_env_logger_with_backtrace_option(&config.opts.unstable_opts.log_backtrace); + } + match make_input(config.opts.error_format, &matches.free) { Err(reported) => return Err(reported), Ok(Some((input, input_file_path))) => { @@ -1299,7 +1303,14 @@ pub fn install_ice_hook() { /// This allows tools to enable rust logging without having to magically match rustc's /// tracing crate version. pub fn init_rustc_env_logger() { - if let Err(error) = rustc_log::init_rustc_env_logger() { + init_rustc_env_logger_with_backtrace_option(&None); +} + +/// This allows tools to enable rust logging without having to magically match rustc's +/// tracing crate version. In contrast to `init_rustc_env_logger` it allows you to +/// choose a target module you wish to show backtraces along with its logging. +pub fn init_rustc_env_logger_with_backtrace_option(backtrace_target: &Option) { + if let Err(error) = rustc_log::init_rustc_env_logger_with_backtrace_option(backtrace_target) { early_error(ErrorOutputType::default(), &error.to_string()); } } @@ -1365,7 +1376,6 @@ mod signal_handler { pub fn main() -> ! { let start_time = Instant::now(); let start_rss = get_resident_set_size(); - init_rustc_env_logger(); signal_handler::install(); let mut callbacks = TimePassesCallbacks::default(); install_ice_hook(); diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index a3b9891ee64e..07b28cc86cee 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -748,6 +748,7 @@ fn test_unstable_options_tracking_hash() { tracked!(link_only, true); tracked!(llvm_plugins, vec![String::from("plugin_name")]); tracked!(location_detail, LocationDetail { file: true, line: false, column: false }); + tracked!(log_backtrace, Some("filter".to_string())); tracked!(maximal_hir_to_mir_coverage, true); tracked!(merge_functions, Some(MergeFunctions::Disabled)); tracked!(mir_emit_retag, true); diff --git a/compiler/rustc_log/Cargo.toml b/compiler/rustc_log/Cargo.toml index 3c50827c1abc..7f955b0a7509 100644 --- a/compiler/rustc_log/Cargo.toml +++ b/compiler/rustc_log/Cargo.toml @@ -7,6 +7,7 @@ edition = "2021" tracing = "0.1.28" tracing-subscriber = { version = "0.3.3", default-features = false, features = ["fmt", "env-filter", "smallvec", "parking_lot", "ansi"] } tracing-tree = "0.2.0" +tracing-core = "0.1.28" [dev-dependencies] rustc_span = { path = "../rustc_span" } diff --git a/compiler/rustc_log/src/lib.rs b/compiler/rustc_log/src/lib.rs index 4cac88aff640..fc1cabd2de95 100644 --- a/compiler/rustc_log/src/lib.rs +++ b/compiler/rustc_log/src/lib.rs @@ -45,16 +45,34 @@ use std::env::{self, VarError}; use std::fmt::{self, Display}; use std::io::{self, IsTerminal}; +use tracing_core::{Event, Subscriber}; use tracing_subscriber::filter::{Directive, EnvFilter, LevelFilter}; +use tracing_subscriber::fmt::{ + format::{self, FormatEvent, FormatFields}, + FmtContext, +}; use tracing_subscriber::layer::SubscriberExt; pub fn init_rustc_env_logger() -> Result<(), Error> { - init_env_logger("RUSTC_LOG") + init_rustc_env_logger_with_backtrace_option(&None) +} + +pub fn init_rustc_env_logger_with_backtrace_option( + backtrace_target: &Option, +) -> Result<(), Error> { + init_env_logger_with_backtrace_option("RUSTC_LOG", backtrace_target) } /// In contrast to `init_rustc_env_logger` this allows you to choose an env var /// other than `RUSTC_LOG`. pub fn init_env_logger(env: &str) -> Result<(), Error> { + init_env_logger_with_backtrace_option(env, &None) +} + +pub fn init_env_logger_with_backtrace_option( + env: &str, + backtrace_target: &Option, +) -> Result<(), Error> { let filter = match env::var(env) { Ok(env) => EnvFilter::new(env), _ => EnvFilter::default().add_directive(Directive::from(LevelFilter::WARN)), @@ -88,11 +106,47 @@ pub fn init_env_logger(env: &str) -> Result<(), Error> { let layer = layer.with_thread_ids(true).with_thread_names(true); let subscriber = tracing_subscriber::Registry::default().with(filter).with(layer); - tracing::subscriber::set_global_default(subscriber).unwrap(); + match backtrace_target { + Some(str) => { + let fmt_layer = tracing_subscriber::fmt::layer() + .with_writer(io::stderr) + .without_time() + .event_format(BacktraceFormatter { backtrace_target: str.to_string() }); + let subscriber = subscriber.with(fmt_layer); + tracing::subscriber::set_global_default(subscriber).unwrap(); + } + None => { + tracing::subscriber::set_global_default(subscriber).unwrap(); + } + }; Ok(()) } +struct BacktraceFormatter { + backtrace_target: String, +} + +impl FormatEvent for BacktraceFormatter +where + S: Subscriber + for<'a> tracing_subscriber::registry::LookupSpan<'a>, + N: for<'a> FormatFields<'a> + 'static, +{ + fn format_event( + &self, + _ctx: &FmtContext<'_, S, N>, + mut writer: format::Writer<'_>, + event: &Event<'_>, + ) -> fmt::Result { + let target = event.metadata().target(); + if !target.contains(&self.backtrace_target) { + return Ok(()); + } + let backtrace = std::backtrace::Backtrace::capture(); + writeln!(writer, "stack backtrace: \n{:?}", backtrace) + } +} + pub fn stdout_isatty() -> bool { io::stdout().is_terminal() } diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index b062b43873b2..7b5fd6cc2a81 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -1411,6 +1411,8 @@ options! { "what location details should be tracked when using caller_location, either \ `none`, or a comma separated list of location details, for which \ valid options are `file`, `line`, and `column` (default: `file,line,column`)"), + log_backtrace: Option = (None, parse_opt_string, [TRACKED], + "add a backtrace along with logging"), ls: bool = (false, parse_bool, [UNTRACKED], "list the symbols defined by a library crate (default: no)"), macro_backtrace: bool = (false, parse_bool, [UNTRACKED], diff --git a/tests/rustdoc-ui/z-help.stdout b/tests/rustdoc-ui/z-help.stdout index 43f30f3d6e80..4bdecdc1b794 100644 --- a/tests/rustdoc-ui/z-help.stdout +++ b/tests/rustdoc-ui/z-help.stdout @@ -76,6 +76,7 @@ -Z llvm-plugins=val -- a list LLVM plugins to enable (space separated) -Z llvm-time-trace=val -- generate JSON tracing data file from LLVM data (default: no) -Z location-detail=val -- what location details should be tracked when using caller_location, either `none`, or a comma separated list of location details, for which valid options are `file`, `line`, and `column` (default: `file,line,column`) + -Z log-backtrace=val -- add a backtrace along with logging -Z ls=val -- list the symbols defined by a library crate (default: no) -Z macro-backtrace=val -- show macro backtraces (default: no) -Z maximal-hir-to-mir-coverage=val -- save as much information as possible about the correspondence between MIR and HIR as source scopes (default: no) diff --git a/tests/ui/attributes/log-backtrace.rs b/tests/ui/attributes/log-backtrace.rs new file mode 100644 index 000000000000..3979d2001fc5 --- /dev/null +++ b/tests/ui/attributes/log-backtrace.rs @@ -0,0 +1,9 @@ +// run-pass +// +// This test makes sure that log-backtrace option doesn't give a compilation error. +// +// dont-check-compiler-stdout +// dont-check-compiler-stderr +// rustc-env:RUSTC_LOG=info +// compile-flags: -Zlog-backtrace=rustc_metadata::creader +fn main() {} From 12ddf778113b291027ff64406ce9d585281debf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 8 Jan 2023 06:54:52 +0000 Subject: [PATCH 04/16] When suggesting writing a fully qualified path probe for appropriate types Fix #46585. --- .../rustc_hir_analysis/src/astconv/mod.rs | 177 ++++++++++++++++-- .../associated-item-duplicate-names-3.stderr | 2 +- ...sociated-types-in-ambiguous-context.stderr | 25 ++- tests/ui/did_you_mean/bad-assoc-ty.stderr | 62 +++++- tests/ui/error-codes/E0223.rs | 4 + tests/ui/error-codes/E0223.stderr | 4 +- tests/ui/impl-trait/impl_trait_projections.rs | 2 +- .../impl-trait/impl_trait_projections.stderr | 9 +- tests/ui/issues/issue-23073.stderr | 7 +- tests/ui/issues/issue-78622.stderr | 7 +- tests/ui/lint/bare-trait-objects-path.stderr | 2 +- .../qualified/qualified-path-params-2.stderr | 7 +- tests/ui/resolve/issue-103202.stderr | 7 +- tests/ui/self/self-impl.stderr | 4 +- .../struct-path-associated-type.stderr | 6 +- .../let-binding-init-expr-as-ty.stderr | 7 +- tests/ui/traits/item-privacy.stderr | 11 +- tests/ui/ufcs/ufcs-partially-resolved.stderr | 7 +- 18 files changed, 295 insertions(+), 55 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/astconv/mod.rs b/compiler/rustc_hir_analysis/src/astconv/mod.rs index 5a7957be318e..7127dadc1e16 100644 --- a/compiler/rustc_hir_analysis/src/astconv/mod.rs +++ b/compiler/rustc_hir_analysis/src/astconv/mod.rs @@ -24,6 +24,7 @@ use rustc_hir::def::{CtorOf, DefKind, Namespace, Res}; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::intravisit::{walk_generics, Visitor as _}; use rustc_hir::{GenericArg, GenericArgs, OpaqueTyOrigin}; +use rustc_infer::infer::TyCtxtInferExt; use rustc_middle::middle::stability::AllowUnstable; use rustc_middle::ty::subst::{self, GenericArgKind, InternalSubsts, SubstsRef}; use rustc_middle::ty::GenericParamDefKind; @@ -1633,8 +1634,8 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { fn report_ambiguous_associated_type( &self, span: Span, - type_str: &str, - trait_str: &str, + types: &[String], + traits: &[String], name: Symbol, ) -> ErrorGuaranteed { let mut err = struct_span_err!(self.tcx().sess, span, E0223, "ambiguous associated type"); @@ -1645,19 +1646,92 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { .keys() .any(|full_span| full_span.contains(span)) { - err.span_suggestion( + err.span_suggestion_verbose( span.shrink_to_lo(), "you are looking for the module in `std`, not the primitive type", "std::", Applicability::MachineApplicable, ); } else { - err.span_suggestion( - span, - "use fully-qualified syntax", - format!("<{} as {}>::{}", type_str, trait_str, name), - Applicability::HasPlaceholders, - ); + match (types, traits) { + ([], []) => { + err.span_suggestion_verbose( + span, + &format!( + "if there were a type named `Type` that implements a trait named \ + `Trait` with associated type `{name}`, you could use the \ + fully-qualified path", + ), + format!("::{name}"), + Applicability::HasPlaceholders, + ); + } + ([], [trait_str]) => { + err.span_suggestion_verbose( + span, + &format!( + "if there were a type named `Example` that implemented `{trait_str}`, \ + you could use the fully-qualified path", + ), + format!("::{name}"), + Applicability::HasPlaceholders, + ); + } + ([], traits) => { + err.span_suggestions( + span, + &format!( + "if there were a type named `Example` that implemented one of the \ + traits with associated type `{name}`, you could use the \ + fully-qualified path", + ), + traits + .iter() + .map(|trait_str| format!("::{name}")) + .collect::>(), + Applicability::HasPlaceholders, + ); + } + ([type_str], []) => { + err.span_suggestion_verbose( + span, + &format!( + "if there were a trait named `Example` with associated type `{name}` \ + implemented for `{type_str}`, you could use the fully-qualified path", + ), + format!("<{type_str} as Example>::{name}"), + Applicability::HasPlaceholders, + ); + } + (types, []) => { + err.span_suggestions( + span, + &format!( + "if there were a trait named `Example` with associated type `{name}` \ + implemented for one of the types, you could use the fully-qualified \ + path", + ), + types + .into_iter() + .map(|type_str| format!("<{type_str} as Example>::{name}")), + Applicability::HasPlaceholders, + ); + } + (types, traits) => { + let mut suggestions = vec![]; + for type_str in types { + for trait_str in traits { + suggestions.push(format!("<{type_str} as {trait_str}>::{name}")); + } + } + err.span_suggestions( + span, + "use the fully-qualified path", + suggestions, + Applicability::MachineApplicable, + ); + } + } } err.emit() } @@ -2040,12 +2114,67 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { err.emit() } else if let Err(reported) = qself_ty.error_reported() { reported + } else if let ty::Alias(ty::Opaque, alias_ty) = qself_ty.kind() { + // `::Assoc` makes no sense. + struct_span_err!( + tcx.sess, + tcx.def_span(alias_ty.def_id), + E0667, + "`impl Trait` is not allowed in path parameters" + ) + .emit() // Already reported in an earlier stage. } else { + // Find all the `impl`s that `qself_ty` has for any trait that has the + // associated type, so that we suggest the right one. + let infcx = tcx.infer_ctxt().build(); + // We create a fresh `ty::ParamEnv` instead of the one for `self.item_def_id()` + // to avoid a cycle error in `src/test/ui/resolve/issue-102946.rs`. + let param_env = ty::ParamEnv::new( + ty::List::empty(), + traits::Reveal::All, + hir::Constness::NotConst, + ); + let traits: Vec<_> = self + .tcx() + .all_traits() + .filter(|trait_def_id| { + // Consider only traits with the associated type + tcx.associated_items(*trait_def_id) + .in_definition_order() + .find(|i| { + i.kind.namespace() == Namespace::TypeNS + && i.ident(tcx).normalize_to_macros_2_0() == assoc_ident + && matches!(i.kind, ty::AssocKind::Type) + }) + .is_some() + // Consider only accessible traits + && tcx.visibility(*trait_def_id) + .is_accessible_from(self.item_def_id(), tcx) + && tcx.all_impls(*trait_def_id) + .filter(|impl_def_id| { + let trait_ref = tcx.impl_trait_ref(impl_def_id); + trait_ref.map_or(false, |impl_| { + infcx + .can_eq( + param_env, + tcx.erase_regions(impl_.self_ty()), + tcx.erase_regions(qself_ty), + ) + .is_ok() + }) + && tcx.impl_polarity(impl_def_id) != ty::ImplPolarity::Negative + }) + .next() + .is_some() + }) + .map(|trait_def_id| tcx.def_path_str(trait_def_id)) + .collect(); + // Don't print `TyErr` to the user. self.report_ambiguous_associated_type( span, - &qself_ty.to_string(), - "Trait", + &[qself_ty.to_string()], + &traits, assoc_ident.name, ) }; @@ -2163,16 +2292,30 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { let is_part_of_self_trait_constraints = def_id == trait_def_id; let is_part_of_fn_in_self_trait = parent_def_id == Some(trait_def_id); - let type_name = if is_part_of_self_trait_constraints || is_part_of_fn_in_self_trait { - "Self" + let type_names = if is_part_of_self_trait_constraints || is_part_of_fn_in_self_trait { + vec!["Self".to_string()] } else { - "Type" + // Find all the types that have an `impl` for the trait. + tcx.all_impls(trait_def_id) + .filter(|impl_def_id| { + // Consider only accessible traits + tcx.visibility(*impl_def_id).is_accessible_from(self.item_def_id(), tcx) + && tcx.impl_polarity(impl_def_id) != ty::ImplPolarity::Negative + }) + .filter_map(|impl_def_id| tcx.impl_trait_ref(impl_def_id)) + .map(|impl_| impl_.self_ty()) + // We don't care about blanket impls. + .filter(|self_ty| !self_ty.has_non_region_infer()) + .map(|self_ty| tcx.erase_regions(self_ty).to_string()) + .collect() }; - + // FIXME: also look at `tcx.generics_of(self.item_def_id()).params` any that + // references the trait. Relevant for the first case in + // `src/test/ui/associated-types/associated-types-in-ambiguous-context.rs` let reported = self.report_ambiguous_associated_type( span, - type_name, - &path_str, + &type_names, + &[path_str], item_segment.ident.name, ); return tcx.ty_error_with_guaranteed(reported) diff --git a/tests/ui/associated-item/associated-item-duplicate-names-3.stderr b/tests/ui/associated-item/associated-item-duplicate-names-3.stderr index bf4bd634cf1d..d0c170620766 100644 --- a/tests/ui/associated-item/associated-item-duplicate-names-3.stderr +++ b/tests/ui/associated-item/associated-item-duplicate-names-3.stderr @@ -13,7 +13,7 @@ error[E0223]: ambiguous associated type --> $DIR/associated-item-duplicate-names-3.rs:18:12 | LL | let x: Baz::Bar = 5; - | ^^^^^^^^ help: use fully-qualified syntax: `::Bar` + | ^^^^^^^^ help: use the fully-qualified path: `::Bar` error: aborting due to 2 previous errors diff --git a/tests/ui/associated-types/associated-types-in-ambiguous-context.stderr b/tests/ui/associated-types/associated-types-in-ambiguous-context.stderr index 289911779ff7..00856b55df5e 100644 --- a/tests/ui/associated-types/associated-types-in-ambiguous-context.stderr +++ b/tests/ui/associated-types/associated-types-in-ambiguous-context.stderr @@ -2,31 +2,46 @@ error[E0223]: ambiguous associated type --> $DIR/associated-types-in-ambiguous-context.rs:6:36 | LL | fn get(x: T, y: U) -> Get::Value {} - | ^^^^^^^^^^ help: use fully-qualified syntax: `::Value` + | ^^^^^^^^^^ + | +help: if there were a type named `Example` that implemented `Get`, you could use the fully-qualified path + | +LL | fn get(x: T, y: U) -> ::Value {} + | ~~~~~~~~~~~~~~~~~~~~~~~ error[E0223]: ambiguous associated type --> $DIR/associated-types-in-ambiguous-context.rs:20:17 | LL | trait Foo where Foo::Assoc: Bar { - | ^^^^^^^^^^ help: use fully-qualified syntax: `::Assoc` + | ^^^^^^^^^^ help: use the fully-qualified path: `::Assoc` error[E0223]: ambiguous associated type --> $DIR/associated-types-in-ambiguous-context.rs:25:10 | LL | type X = std::ops::Deref::Target; - | ^^^^^^^^^^^^^^^^^^^^^^^ help: use fully-qualified syntax: `::Target` + | ^^^^^^^^^^^^^^^^^^^^^^^ + | +help: if there were a type named `Example` that implemented `Deref`, you could use the fully-qualified path + | +LL | type X = ::Target; + | ~~~~~~~~~~~~~~~~~~~~~~~~~~ error[E0223]: ambiguous associated type --> $DIR/associated-types-in-ambiguous-context.rs:11:23 | LL | fn grab(&self) -> Grab::Value; - | ^^^^^^^^^^^ help: use fully-qualified syntax: `::Value` + | ^^^^^^^^^^^ help: use the fully-qualified path: `::Value` error[E0223]: ambiguous associated type --> $DIR/associated-types-in-ambiguous-context.rs:14:22 | LL | fn get(&self) -> Get::Value; - | ^^^^^^^^^^ help: use fully-qualified syntax: `::Value` + | ^^^^^^^^^^ + | +help: if there were a type named `Example` that implemented `Get`, you could use the fully-qualified path + | +LL | fn get(&self) -> ::Value; + | ~~~~~~~~~~~~~~~~~~~~~~~ error: aborting due to 5 previous errors diff --git a/tests/ui/did_you_mean/bad-assoc-ty.stderr b/tests/ui/did_you_mean/bad-assoc-ty.stderr index 21f957ab549a..7cd349c7507f 100644 --- a/tests/ui/did_you_mean/bad-assoc-ty.stderr +++ b/tests/ui/did_you_mean/bad-assoc-ty.stderr @@ -61,25 +61,45 @@ error[E0223]: ambiguous associated type --> $DIR/bad-assoc-ty.rs:1:10 | LL | type A = [u8; 4]::AssocTy; - | ^^^^^^^^^^^^^^^^ help: use fully-qualified syntax: `<[u8; 4] as Trait>::AssocTy` + | ^^^^^^^^^^^^^^^^ + | +help: if there were a trait named `Example` with associated type `AssocTy` implemented for `[u8; 4]`, you could use the fully-qualified path + | +LL | type A = <[u8; 4] as Example>::AssocTy; + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error[E0223]: ambiguous associated type --> $DIR/bad-assoc-ty.rs:5:10 | LL | type B = [u8]::AssocTy; - | ^^^^^^^^^^^^^ help: use fully-qualified syntax: `<[u8] as Trait>::AssocTy` + | ^^^^^^^^^^^^^ + | +help: if there were a trait named `Example` with associated type `AssocTy` implemented for `[u8]`, you could use the fully-qualified path + | +LL | type B = <[u8] as Example>::AssocTy; + | ~~~~~~~~~~~~~~~~~~~~~~~~~~ error[E0223]: ambiguous associated type --> $DIR/bad-assoc-ty.rs:9:10 | LL | type C = (u8)::AssocTy; - | ^^^^^^^^^^^^^ help: use fully-qualified syntax: `::AssocTy` + | ^^^^^^^^^^^^^ + | +help: if there were a trait named `Example` with associated type `AssocTy` implemented for `u8`, you could use the fully-qualified path + | +LL | type C = ::AssocTy; + | ~~~~~~~~~~~~~~~~~~~~~~~~ error[E0223]: ambiguous associated type --> $DIR/bad-assoc-ty.rs:13:10 | LL | type D = (u8, u8)::AssocTy; - | ^^^^^^^^^^^^^^^^^ help: use fully-qualified syntax: `<(u8, u8) as Trait>::AssocTy` + | ^^^^^^^^^^^^^^^^^ + | +help: if there were a trait named `Example` with associated type `AssocTy` implemented for `(u8, u8)`, you could use the fully-qualified path + | +LL | type D = <(u8, u8) as Example>::AssocTy; + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error[E0121]: the placeholder `_` is not allowed within types on item signatures for type aliases --> $DIR/bad-assoc-ty.rs:17:10 @@ -91,13 +111,23 @@ error[E0223]: ambiguous associated type --> $DIR/bad-assoc-ty.rs:21:19 | LL | type F = &'static (u8)::AssocTy; - | ^^^^^^^^^^^^^ help: use fully-qualified syntax: `::AssocTy` + | ^^^^^^^^^^^^^ + | +help: if there were a trait named `Example` with associated type `AssocTy` implemented for `u8`, you could use the fully-qualified path + | +LL | type F = &'static ::AssocTy; + | ~~~~~~~~~~~~~~~~~~~~~~~~ error[E0223]: ambiguous associated type --> $DIR/bad-assoc-ty.rs:27:10 | LL | type G = dyn 'static + (Send)::AssocTy; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use fully-qualified syntax: `<(dyn Send + 'static) as Trait>::AssocTy` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: if there were a trait named `Example` with associated type `AssocTy` implemented for `(dyn Send + 'static)`, you could use the fully-qualified path + | +LL | type G = <(dyn Send + 'static) as Example>::AssocTy; + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ warning: trait objects without an explicit `dyn` are deprecated --> $DIR/bad-assoc-ty.rs:33:10 @@ -117,24 +147,38 @@ error[E0223]: ambiguous associated type --> $DIR/bad-assoc-ty.rs:33:10 | LL | type H = Fn(u8) -> (u8)::Output; - | ^^^^^^^^^^^^^^^^^^^^^^ help: use fully-qualified syntax: `<(dyn Fn(u8) -> u8 + 'static) as Trait>::Output` + | ^^^^^^^^^^^^^^^^^^^^^^ + | +help: if there were a trait named `Example` with associated type `Output` implemented for `(dyn Fn(u8) -> u8 + 'static)`, you could use the fully-qualified path + | +LL | type H = <(dyn Fn(u8) -> u8 + 'static) as Example>::Output; + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error[E0223]: ambiguous associated type --> $DIR/bad-assoc-ty.rs:39:19 | LL | ($ty: ty) => ($ty::AssocTy); - | ^^^^^^^^^^^^ help: use fully-qualified syntax: `::AssocTy` + | ^^^^^^^^^^^^ ... LL | type J = ty!(u8); | ------- in this macro invocation | = note: this error originates in the macro `ty` (in Nightly builds, run with -Z macro-backtrace for more info) +help: if there were a trait named `Example` with associated type `AssocTy` implemented for `u8`, you could use the fully-qualified path + | +LL | ($ty: ty) => (::AssocTy); + | ~~~~~~~~~~~~~~~~~~~~~~~~ error[E0223]: ambiguous associated type --> $DIR/bad-assoc-ty.rs:46:10 | LL | type I = ty!()::AssocTy; - | ^^^^^^^^^^^^^^ help: use fully-qualified syntax: `::AssocTy` + | ^^^^^^^^^^^^^^ + | +help: if there were a trait named `Example` with associated type `AssocTy` implemented for `u8`, you could use the fully-qualified path + | +LL | type I = ::AssocTy; + | ~~~~~~~~~~~~~~~~~~~~~~~~ error[E0121]: the placeholder `_` is not allowed within types on item signatures for functions --> $DIR/bad-assoc-ty.rs:51:13 diff --git a/tests/ui/error-codes/E0223.rs b/tests/ui/error-codes/E0223.rs index 6031b682d72a..2fe252de2566 100644 --- a/tests/ui/error-codes/E0223.rs +++ b/tests/ui/error-codes/E0223.rs @@ -1,4 +1,8 @@ trait MyTrait { type X; } +struct MyStruct; +impl MyTrait for MyStruct { + type X = (); +} fn main() { let foo: MyTrait::X; diff --git a/tests/ui/error-codes/E0223.stderr b/tests/ui/error-codes/E0223.stderr index 726f39e11f13..42945e42f6ea 100644 --- a/tests/ui/error-codes/E0223.stderr +++ b/tests/ui/error-codes/E0223.stderr @@ -1,8 +1,8 @@ error[E0223]: ambiguous associated type - --> $DIR/E0223.rs:4:14 + --> $DIR/E0223.rs:8:14 | LL | let foo: MyTrait::X; - | ^^^^^^^^^^ help: use fully-qualified syntax: `::X` + | ^^^^^^^^^^ help: use the fully-qualified path: `::X` error: aborting due to previous error diff --git a/tests/ui/impl-trait/impl_trait_projections.rs b/tests/ui/impl-trait/impl_trait_projections.rs index fd0986d7c0a9..b3ff2ce5a7bf 100644 --- a/tests/ui/impl-trait/impl_trait_projections.rs +++ b/tests/ui/impl-trait/impl_trait_projections.rs @@ -11,7 +11,7 @@ fn path_parametrized_type_is_allowed() -> option::Option { fn projection_is_disallowed(x: impl Iterator) -> ::Item { //~^ ERROR `impl Trait` is not allowed in path parameters -//~^^ ERROR ambiguous associated type +//~| ERROR `impl Trait` is not allowed in path parameters x.next().unwrap() } diff --git a/tests/ui/impl-trait/impl_trait_projections.stderr b/tests/ui/impl-trait/impl_trait_projections.stderr index 82d2422c4077..4deb24731bc0 100644 --- a/tests/ui/impl-trait/impl_trait_projections.stderr +++ b/tests/ui/impl-trait/impl_trait_projections.stderr @@ -22,13 +22,12 @@ error[E0667]: `impl Trait` is not allowed in path parameters LL | -> as Iterator>::Item | ^^^^^^^^^^ -error[E0223]: ambiguous associated type - --> $DIR/impl_trait_projections.rs:12:50 +error[E0667]: `impl Trait` is not allowed in path parameters + --> $DIR/impl_trait_projections.rs:12:51 | LL | fn projection_is_disallowed(x: impl Iterator) -> ::Item { - | ^^^^^^^^^^^^^^^^^^^^^ help: use fully-qualified syntax: `::Item` + | ^^^^^^^^^^^^^ error: aborting due to 5 previous errors -Some errors have detailed explanations: E0223, E0667. -For more information about an error, try `rustc --explain E0223`. +For more information about this error, try `rustc --explain E0667`. diff --git a/tests/ui/issues/issue-23073.stderr b/tests/ui/issues/issue-23073.stderr index 3a10a1ab11ac..3a9f49ef167d 100644 --- a/tests/ui/issues/issue-23073.stderr +++ b/tests/ui/issues/issue-23073.stderr @@ -2,7 +2,12 @@ error[E0223]: ambiguous associated type --> $DIR/issue-23073.rs:6:17 | LL | type FooT = <::Foo>::T; - | ^^^^^^^^^^^^^^^^^^^^^^^ help: use fully-qualified syntax: `<::Foo as Trait>::T` + | ^^^^^^^^^^^^^^^^^^^^^^^ + | +help: if there were a trait named `Example` with associated type `T` implemented for `::Foo`, you could use the fully-qualified path + | +LL | type FooT = <::Foo as Example>::T; + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: aborting due to previous error diff --git a/tests/ui/issues/issue-78622.stderr b/tests/ui/issues/issue-78622.stderr index f7d44f21d3be..70daf8a2f1a6 100644 --- a/tests/ui/issues/issue-78622.stderr +++ b/tests/ui/issues/issue-78622.stderr @@ -2,7 +2,12 @@ error[E0223]: ambiguous associated type --> $DIR/issue-78622.rs:5:5 | LL | S::A:: {} - | ^^^^ help: use fully-qualified syntax: `::A` + | ^^^^ + | +help: if there were a trait named `Example` with associated type `A` implemented for `S`, you could use the fully-qualified path + | +LL | ::A:: {} + | ~~~~~~~~~~~~~~~~~ error: aborting due to previous error diff --git a/tests/ui/lint/bare-trait-objects-path.stderr b/tests/ui/lint/bare-trait-objects-path.stderr index 8ed303ca6069..a19f4963c239 100644 --- a/tests/ui/lint/bare-trait-objects-path.stderr +++ b/tests/ui/lint/bare-trait-objects-path.stderr @@ -16,7 +16,7 @@ error[E0223]: ambiguous associated type --> $DIR/bare-trait-objects-path.rs:23:12 | LL | let _: Dyn::Ty; - | ^^^^^^^ help: use fully-qualified syntax: `::Ty` + | ^^^^^^^ help: use the fully-qualified path: `::Ty` warning: trait objects without an explicit `dyn` are deprecated --> $DIR/bare-trait-objects-path.rs:14:5 diff --git a/tests/ui/qualified/qualified-path-params-2.stderr b/tests/ui/qualified/qualified-path-params-2.stderr index 948f21fce4bd..b6cf19b8286c 100644 --- a/tests/ui/qualified/qualified-path-params-2.stderr +++ b/tests/ui/qualified/qualified-path-params-2.stderr @@ -2,7 +2,12 @@ error[E0223]: ambiguous associated type --> $DIR/qualified-path-params-2.rs:18:10 | LL | type A = ::A::f; - | ^^^^^^^^^^^^^^^^^^^ help: use fully-qualified syntax: `<::A as Trait>::f` + | ^^^^^^^^^^^^^^^^^^^ + | +help: if there were a trait named `Example` with associated type `f` implemented for `::A`, you could use the fully-qualified path + | +LL | type A = <::A as Example>::f; + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: aborting due to previous error diff --git a/tests/ui/resolve/issue-103202.stderr b/tests/ui/resolve/issue-103202.stderr index 880389371ef7..d4d141fb06f3 100644 --- a/tests/ui/resolve/issue-103202.stderr +++ b/tests/ui/resolve/issue-103202.stderr @@ -2,7 +2,12 @@ error[E0223]: ambiguous associated type --> $DIR/issue-103202.rs:4:17 | LL | fn f(self: &S::x) {} - | ^^^^ help: use fully-qualified syntax: `::x` + | ^^^^ + | +help: if there were a trait named `Example` with associated type `x` implemented for `S`, you could use the fully-qualified path + | +LL | fn f(self: &::x) {} + | ~~~~~~~~~~~~~~~~~ error: aborting due to previous error diff --git a/tests/ui/self/self-impl.stderr b/tests/ui/self/self-impl.stderr index fb47f27e022f..36372b644d6e 100644 --- a/tests/ui/self/self-impl.stderr +++ b/tests/ui/self/self-impl.stderr @@ -2,13 +2,13 @@ error[E0223]: ambiguous associated type --> $DIR/self-impl.rs:23:16 | LL | let _: ::Baz = true; - | ^^^^^^^^^^^ help: use fully-qualified syntax: `::Baz` + | ^^^^^^^^^^^ help: use the fully-qualified path: `::Baz` error[E0223]: ambiguous associated type --> $DIR/self-impl.rs:25:16 | LL | let _: Self::Baz = true; - | ^^^^^^^^^ help: use fully-qualified syntax: `::Baz` + | ^^^^^^^^^ help: use the fully-qualified path: `::Baz` error: aborting due to 2 previous errors diff --git a/tests/ui/structs/struct-path-associated-type.stderr b/tests/ui/structs/struct-path-associated-type.stderr index abb445214f36..ca5f0b7e21e7 100644 --- a/tests/ui/structs/struct-path-associated-type.stderr +++ b/tests/ui/structs/struct-path-associated-type.stderr @@ -48,19 +48,19 @@ error[E0223]: ambiguous associated type --> $DIR/struct-path-associated-type.rs:32:13 | LL | let s = S::A {}; - | ^^^^ help: use fully-qualified syntax: `::A` + | ^^^^ help: use the fully-qualified path: `::A` error[E0223]: ambiguous associated type --> $DIR/struct-path-associated-type.rs:33:13 | LL | let z = S::A:: {}; - | ^^^^ help: use fully-qualified syntax: `::A` + | ^^^^ help: use the fully-qualified path: `::A` error[E0223]: ambiguous associated type --> $DIR/struct-path-associated-type.rs:35:9 | LL | S::A {} => {} - | ^^^^ help: use fully-qualified syntax: `::A` + | ^^^^ help: use the fully-qualified path: `::A` error: aborting due to 8 previous errors diff --git a/tests/ui/suggestions/let-binding-init-expr-as-ty.stderr b/tests/ui/suggestions/let-binding-init-expr-as-ty.stderr index 2bf072ef5217..b90ae051fb77 100644 --- a/tests/ui/suggestions/let-binding-init-expr-as-ty.stderr +++ b/tests/ui/suggestions/let-binding-init-expr-as-ty.stderr @@ -21,7 +21,12 @@ error[E0223]: ambiguous associated type --> $DIR/let-binding-init-expr-as-ty.rs:2:14 | LL | let foo: i32::from_be(num); - | ^^^^^^^^^^^^^^^^^ help: use fully-qualified syntax: `::from_be` + | ^^^^^^^^^^^^^^^^^ + | +help: if there were a trait named `Example` with associated type `from_be` implemented for `i32`, you could use the fully-qualified path + | +LL | let foo: ::from_be; + | ~~~~~~~~~~~~~~~~~~~~~~~~~ error: aborting due to 3 previous errors diff --git a/tests/ui/traits/item-privacy.stderr b/tests/ui/traits/item-privacy.stderr index f137a298a7f4..293cfbda86c4 100644 --- a/tests/ui/traits/item-privacy.stderr +++ b/tests/ui/traits/item-privacy.stderr @@ -148,19 +148,24 @@ error[E0223]: ambiguous associated type --> $DIR/item-privacy.rs:115:12 | LL | let _: S::A; - | ^^^^ help: use fully-qualified syntax: `::A` + | ^^^^ + | +help: if there were a trait named `Example` with associated type `A` implemented for `S`, you could use the fully-qualified path + | +LL | let _: ::A; + | ~~~~~~~~~~~~~~~~~ error[E0223]: ambiguous associated type --> $DIR/item-privacy.rs:116:12 | LL | let _: S::B; - | ^^^^ help: use fully-qualified syntax: `::B` + | ^^^^ help: use the fully-qualified path: `::B` error[E0223]: ambiguous associated type --> $DIR/item-privacy.rs:117:12 | LL | let _: S::C; - | ^^^^ help: use fully-qualified syntax: `::C` + | ^^^^ help: use the fully-qualified path: `::C` error[E0624]: associated type `A` is private --> $DIR/item-privacy.rs:119:12 diff --git a/tests/ui/ufcs/ufcs-partially-resolved.stderr b/tests/ui/ufcs/ufcs-partially-resolved.stderr index 5f7f6aa9f6ec..72fccea8ae39 100644 --- a/tests/ui/ufcs/ufcs-partially-resolved.stderr +++ b/tests/ui/ufcs/ufcs-partially-resolved.stderr @@ -205,7 +205,12 @@ error[E0223]: ambiguous associated type --> $DIR/ufcs-partially-resolved.rs:36:12 | LL | let _: ::Y::NN; - | ^^^^^^^^^^^^^^^^^ help: use fully-qualified syntax: `<::Y as Trait>::NN` + | ^^^^^^^^^^^^^^^^^ + | +help: if there were a trait named `Example` with associated type `NN` implemented for `::Y`, you could use the fully-qualified path + | +LL | let _: <::Y as Example>::NN; + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error[E0599]: no associated item named `NN` found for type `u16` in the current scope --> $DIR/ufcs-partially-resolved.rs:38:20 From 147c9bf4d56d7a9cf5fb70270b3e68c730da7d95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 8 Jan 2023 07:24:36 +0000 Subject: [PATCH 05/16] review comments --- compiler/rustc_hir_analysis/src/astconv/mod.rs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/astconv/mod.rs b/compiler/rustc_hir_analysis/src/astconv/mod.rs index 7127dadc1e16..315a2a2af1bc 100644 --- a/compiler/rustc_hir_analysis/src/astconv/mod.rs +++ b/compiler/rustc_hir_analysis/src/astconv/mod.rs @@ -2129,11 +2129,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { let infcx = tcx.infer_ctxt().build(); // We create a fresh `ty::ParamEnv` instead of the one for `self.item_def_id()` // to avoid a cycle error in `src/test/ui/resolve/issue-102946.rs`. - let param_env = ty::ParamEnv::new( - ty::List::empty(), - traits::Reveal::All, - hir::Constness::NotConst, - ); + let param_env = ty::ParamEnv::empty(); let traits: Vec<_> = self .tcx() .all_traits() @@ -2141,17 +2137,16 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { // Consider only traits with the associated type tcx.associated_items(*trait_def_id) .in_definition_order() - .find(|i| { + .any(|i| { i.kind.namespace() == Namespace::TypeNS && i.ident(tcx).normalize_to_macros_2_0() == assoc_ident && matches!(i.kind, ty::AssocKind::Type) }) - .is_some() // Consider only accessible traits && tcx.visibility(*trait_def_id) .is_accessible_from(self.item_def_id(), tcx) && tcx.all_impls(*trait_def_id) - .filter(|impl_def_id| { + .any(|impl_def_id| { let trait_ref = tcx.impl_trait_ref(impl_def_id); trait_ref.map_or(false, |impl_| { infcx @@ -2164,8 +2159,6 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { }) && tcx.impl_polarity(impl_def_id) != ty::ImplPolarity::Negative }) - .next() - .is_some() }) .map(|trait_def_id| tcx.def_path_str(trait_def_id)) .collect(); @@ -2305,7 +2298,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { .filter_map(|impl_def_id| tcx.impl_trait_ref(impl_def_id)) .map(|impl_| impl_.self_ty()) // We don't care about blanket impls. - .filter(|self_ty| !self_ty.has_non_region_infer()) + .filter(|self_ty| !self_ty.has_non_region_param()) .map(|self_ty| tcx.erase_regions(self_ty).to_string()) .collect() }; From c6f322bf300c7c963a4e4e8bb642c3959b74888a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 8 Jan 2023 20:51:40 +0000 Subject: [PATCH 06/16] review comments: account for generics --- compiler/rustc_hir_analysis/src/astconv/mod.rs | 8 ++++++-- .../ambiguous-associated-type-with-generics.fixed | 14 ++++++++++++++ .../ambiguous-associated-type-with-generics.rs | 14 ++++++++++++++ .../ambiguous-associated-type-with-generics.stderr | 9 +++++++++ tests/ui/did_you_mean/bad-assoc-ty.stderr | 7 +------ 5 files changed, 44 insertions(+), 8 deletions(-) create mode 100644 tests/ui/associated-item/ambiguous-associated-type-with-generics.fixed create mode 100644 tests/ui/associated-item/ambiguous-associated-type-with-generics.rs create mode 100644 tests/ui/associated-item/ambiguous-associated-type-with-generics.stderr diff --git a/compiler/rustc_hir_analysis/src/astconv/mod.rs b/compiler/rustc_hir_analysis/src/astconv/mod.rs index 315a2a2af1bc..5baaaf09edb6 100644 --- a/compiler/rustc_hir_analysis/src/astconv/mod.rs +++ b/compiler/rustc_hir_analysis/src/astconv/mod.rs @@ -2147,8 +2147,12 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { .is_accessible_from(self.item_def_id(), tcx) && tcx.all_impls(*trait_def_id) .any(|impl_def_id| { - let trait_ref = tcx.impl_trait_ref(impl_def_id); - trait_ref.map_or(false, |impl_| { + let trait_ref = tcx.bound_impl_trait_ref(impl_def_id); + trait_ref.map_or(false, |trait_ref| { + let impl_ = trait_ref.subst( + tcx, + infcx.fresh_substs_for_item(span, impl_def_id), + ); infcx .can_eq( param_env, diff --git a/tests/ui/associated-item/ambiguous-associated-type-with-generics.fixed b/tests/ui/associated-item/ambiguous-associated-type-with-generics.fixed new file mode 100644 index 000000000000..23f715200400 --- /dev/null +++ b/tests/ui/associated-item/ambiguous-associated-type-with-generics.fixed @@ -0,0 +1,14 @@ +// run-rustfix +trait Trait {} + +trait Assoc { + type Ty; +} + +impl Assoc for dyn Trait { + type Ty = i32; +} + +fn main() { + let _x: as Assoc>::Ty; //~ ERROR ambiguous associated type +} diff --git a/tests/ui/associated-item/ambiguous-associated-type-with-generics.rs b/tests/ui/associated-item/ambiguous-associated-type-with-generics.rs new file mode 100644 index 000000000000..9c26e339a449 --- /dev/null +++ b/tests/ui/associated-item/ambiguous-associated-type-with-generics.rs @@ -0,0 +1,14 @@ +// run-rustfix +trait Trait {} + +trait Assoc { + type Ty; +} + +impl Assoc for dyn Trait { + type Ty = i32; +} + +fn main() { + let _x: >::Ty; //~ ERROR ambiguous associated type +} diff --git a/tests/ui/associated-item/ambiguous-associated-type-with-generics.stderr b/tests/ui/associated-item/ambiguous-associated-type-with-generics.stderr new file mode 100644 index 000000000000..97088b79fd67 --- /dev/null +++ b/tests/ui/associated-item/ambiguous-associated-type-with-generics.stderr @@ -0,0 +1,9 @@ +error[E0223]: ambiguous associated type + --> $DIR/ambiguous-associated-type-with-generics.rs:13:13 + | +LL | let _x: >::Ty; + | ^^^^^^^^^^^^^^^^^^^^ help: use the fully-qualified path: ` as Assoc>::Ty` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0223`. diff --git a/tests/ui/did_you_mean/bad-assoc-ty.stderr b/tests/ui/did_you_mean/bad-assoc-ty.stderr index 7cd349c7507f..55096e95df7e 100644 --- a/tests/ui/did_you_mean/bad-assoc-ty.stderr +++ b/tests/ui/did_you_mean/bad-assoc-ty.stderr @@ -147,12 +147,7 @@ error[E0223]: ambiguous associated type --> $DIR/bad-assoc-ty.rs:33:10 | LL | type H = Fn(u8) -> (u8)::Output; - | ^^^^^^^^^^^^^^^^^^^^^^ - | -help: if there were a trait named `Example` with associated type `Output` implemented for `(dyn Fn(u8) -> u8 + 'static)`, you could use the fully-qualified path - | -LL | type H = <(dyn Fn(u8) -> u8 + 'static) as Example>::Output; - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + | ^^^^^^^^^^^^^^^^^^^^^^ help: use the fully-qualified path: `<(dyn Fn(u8) -> u8 + 'static) as IntoFuture>::Output` error[E0223]: ambiguous associated type --> $DIR/bad-assoc-ty.rs:39:19 From 8917e9936282f855a08808ed8874c4117210da6e Mon Sep 17 00:00:00 2001 From: Ibraheem Ahmed Date: Wed, 11 Jan 2023 21:29:14 -0500 Subject: [PATCH 07/16] rework and document backoff behavior of `sync::mpsc` --- library/std/src/sync/mpmc/array.rs | 14 +++++++------- library/std/src/sync/mpmc/list.rs | 16 ++++++++-------- library/std/src/sync/mpmc/utils.rs | 29 ++++++++++++++--------------- library/std/src/sync/mpmc/zero.rs | 2 +- 4 files changed, 30 insertions(+), 31 deletions(-) diff --git a/library/std/src/sync/mpmc/array.rs b/library/std/src/sync/mpmc/array.rs index f71edc6c525a..c1e3e48b0446 100644 --- a/library/std/src/sync/mpmc/array.rs +++ b/library/std/src/sync/mpmc/array.rs @@ -168,7 +168,7 @@ impl Channel { return true; } Err(_) => { - backoff.spin(); + backoff.spin_light(); tail = self.tail.load(Ordering::Relaxed); } } @@ -182,11 +182,11 @@ impl Channel { return false; } - backoff.spin(); + backoff.spin_light(); tail = self.tail.load(Ordering::Relaxed); } else { // Snooze because we need to wait for the stamp to get updated. - backoff.snooze(); + backoff.spin_heavy(); tail = self.tail.load(Ordering::Relaxed); } } @@ -251,7 +251,7 @@ impl Channel { return true; } Err(_) => { - backoff.spin(); + backoff.spin_light(); head = self.head.load(Ordering::Relaxed); } } @@ -273,11 +273,11 @@ impl Channel { } } - backoff.spin(); + backoff.spin_light(); head = self.head.load(Ordering::Relaxed); } else { // Snooze because we need to wait for the stamp to get updated. - backoff.snooze(); + backoff.spin_heavy(); head = self.head.load(Ordering::Relaxed); } } @@ -330,7 +330,7 @@ impl Channel { if backoff.is_completed() { break; } else { - backoff.spin(); + backoff.spin_light(); } } diff --git a/library/std/src/sync/mpmc/list.rs b/library/std/src/sync/mpmc/list.rs index 2d5b2fb3b231..ec6c0726ac79 100644 --- a/library/std/src/sync/mpmc/list.rs +++ b/library/std/src/sync/mpmc/list.rs @@ -46,7 +46,7 @@ impl Slot { fn wait_write(&self) { let backoff = Backoff::new(); while self.state.load(Ordering::Acquire) & WRITE == 0 { - backoff.snooze(); + backoff.spin_heavy(); } } } @@ -82,7 +82,7 @@ impl Block { if !next.is_null() { return next; } - backoff.snooze(); + backoff.spin_heavy(); } } @@ -191,7 +191,7 @@ impl Channel { // If we reached the end of the block, wait until the next one is installed. if offset == BLOCK_CAP { - backoff.snooze(); + backoff.spin_heavy(); tail = self.tail.index.load(Ordering::Acquire); block = self.tail.block.load(Ordering::Acquire); continue; @@ -247,7 +247,7 @@ impl Channel { return true; }, Err(_) => { - backoff.spin(); + backoff.spin_light(); tail = self.tail.index.load(Ordering::Acquire); block = self.tail.block.load(Ordering::Acquire); } @@ -286,7 +286,7 @@ impl Channel { // If we reached the end of the block, wait until the next one is installed. if offset == BLOCK_CAP { - backoff.snooze(); + backoff.spin_heavy(); head = self.head.index.load(Ordering::Acquire); block = self.head.block.load(Ordering::Acquire); continue; @@ -320,7 +320,7 @@ impl Channel { // The block can be null here only if the first message is being sent into the channel. // In that case, just wait until it gets initialized. if block.is_null() { - backoff.snooze(); + backoff.spin_heavy(); head = self.head.index.load(Ordering::Acquire); block = self.head.block.load(Ordering::Acquire); continue; @@ -351,7 +351,7 @@ impl Channel { return true; }, Err(_) => { - backoff.spin(); + backoff.spin_light(); head = self.head.index.load(Ordering::Acquire); block = self.head.block.load(Ordering::Acquire); } @@ -542,7 +542,7 @@ impl Channel { // New updates to tail will be rejected by MARK_BIT and aborted unless it's // at boundary. We need to wait for the updates take affect otherwise there // can be memory leaks. - backoff.snooze(); + backoff.spin_heavy(); tail = self.tail.index.load(Ordering::Acquire); } diff --git a/library/std/src/sync/mpmc/utils.rs b/library/std/src/sync/mpmc/utils.rs index a9b365daeec4..cfe42750d523 100644 --- a/library/std/src/sync/mpmc/utils.rs +++ b/library/std/src/sync/mpmc/utils.rs @@ -91,9 +91,8 @@ impl DerefMut for CachePadded { } const SPIN_LIMIT: u32 = 6; -const YIELD_LIMIT: u32 = 10; -/// Performs exponential backoff in spin loops. +/// Performs quadratic backoff in spin loops. pub struct Backoff { step: Cell, } @@ -104,25 +103,27 @@ impl Backoff { Backoff { step: Cell::new(0) } } - /// Backs off in a lock-free loop. + /// Backs off using lightweight spinning. /// - /// This method should be used when we need to retry an operation because another thread made - /// progress. + /// This method should be used for: + /// - Retrying an operation because another thread made progress. i.e. on CAS failure. + /// - Waiting for an operation to complete by spinning optimistically for a few iterations + /// before falling back to parking the thread (see `Backoff::is_completed`). #[inline] - pub fn spin(&self) { + pub fn spin_light(&self) { let step = self.step.get().min(SPIN_LIMIT); for _ in 0..step.pow(2) { crate::hint::spin_loop(); } - if self.step.get() <= SPIN_LIMIT { - self.step.set(self.step.get() + 1); - } + self.step.set(self.step.get() + 1); } - /// Backs off in a blocking loop. + /// Backs off using heavyweight spinning. + /// + /// This method should be used in blocking loops where parking the thread is not an option. #[inline] - pub fn snooze(&self) { + pub fn spin_heavy(&self) { if self.step.get() <= SPIN_LIMIT { for _ in 0..self.step.get().pow(2) { crate::hint::spin_loop() @@ -131,12 +132,10 @@ impl Backoff { crate::thread::yield_now(); } - if self.step.get() <= YIELD_LIMIT { - self.step.set(self.step.get() + 1); - } + self.step.set(self.step.get() + 1); } - /// Returns `true` if quadratic backoff has completed and blocking the thread is advised. + /// Returns `true` if quadratic backoff has completed and parking the thread is advised. #[inline] pub fn is_completed(&self) -> bool { self.step.get() > SPIN_LIMIT diff --git a/library/std/src/sync/mpmc/zero.rs b/library/std/src/sync/mpmc/zero.rs index fccd6c29a7e4..33f768dcbe90 100644 --- a/library/std/src/sync/mpmc/zero.rs +++ b/library/std/src/sync/mpmc/zero.rs @@ -57,7 +57,7 @@ impl Packet { fn wait_ready(&self) { let backoff = Backoff::new(); while !self.ready.load(Ordering::Acquire) { - backoff.snooze(); + backoff.spin_heavy(); } } } From c82545955ea526c0bb6878ee12fa9959e8de3b89 Mon Sep 17 00:00:00 2001 From: yukang Date: Sat, 7 Jan 2023 14:02:59 +0800 Subject: [PATCH 08/16] Provide help on closures capturing self causing borrow checker errors --- .../src/diagnostics/conflict_errors.rs | 150 +++++++++++++++++- .../src/diagnostics/mutability_errors.rs | 2 +- ...ssue-105761-suggest-self-for-closure.fixed | 28 ++++ .../issue-105761-suggest-self-for-closure.rs | 28 ++++ ...sue-105761-suggest-self-for-closure.stderr | 49 ++++++ 5 files changed, 252 insertions(+), 5 deletions(-) create mode 100644 tests/ui/suggestions/issue-105761-suggest-self-for-closure.fixed create mode 100644 tests/ui/suggestions/issue-105761-suggest-self-for-closure.rs create mode 100644 tests/ui/suggestions/issue-105761-suggest-self-for-closure.stderr diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 6658ee89ad6f..04bbceadd5a5 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -1,4 +1,6 @@ +use crate::diagnostics::mutability_errors::mut_borrow_of_mutable_ref; use either::Either; +use hir::Closure; use rustc_const_eval::util::CallKind; use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::FxHashSet; @@ -20,7 +22,7 @@ use rustc_middle::ty::{self, suggest_constraining_type_params, PredicateKind, Ty use rustc_mir_dataflow::move_paths::{InitKind, MoveOutIndex, MovePathIndex}; use rustc_span::def_id::LocalDefId; use rustc_span::hygiene::DesugaringKind; -use rustc_span::symbol::sym; +use rustc_span::symbol::{kw, sym}; use rustc_span::{BytePos, Span, Symbol}; use rustc_trait_selection::infer::InferCtxtExt; @@ -39,6 +41,8 @@ use super::{ DescribePlaceOpt, RegionName, RegionNameSource, UseSpans, }; +use rustc_hir::def::Res; + #[derive(Debug)] struct MoveSite { /// Index of the "move out" that we found. The `MoveData` can @@ -356,7 +360,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { if let Some(hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(_, _, body_id), .. - })) = hir.find(hir.local_def_id_to_hir_id(self.mir_def_id())) + })) = hir.find(self.mir_hir_id()) && let Some(hir::Node::Expr(expr)) = hir.find(body_id.hir_id) { let place = &self.move_data.move_paths[mpi].place; @@ -948,7 +952,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } (BorrowKind::Mut { .. }, BorrowKind::Shared) => { first_borrow_desc = "immutable "; - self.cannot_reborrow_already_borrowed( + let mut err = self.cannot_reborrow_already_borrowed( span, &desc_place, &msg_place, @@ -958,7 +962,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { "immutable", &msg_borrow, None, - ) + ); + self.suggest_binding_for_closure_capture_self( + &mut err, + issued_borrow.borrowed_place, + &issued_spans, + ); + err } (BorrowKind::Mut { .. }, BorrowKind::Mut { .. }) => { @@ -1240,6 +1250,138 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } + fn suggest_binding_for_closure_capture_self( + &self, + err: &mut Diagnostic, + borrowed_place: Place<'tcx>, + issued_spans: &UseSpans<'tcx>, + ) { + let UseSpans::ClosureUse { capture_kind_span, .. } = issued_spans else { return }; + let hir = self.infcx.tcx.hir(); + + // check whether the borrowed place is capturing `self` by mut reference + let local = borrowed_place.local; + let Some(_) = self + .body + .local_decls + .get(local) + .map(|l| mut_borrow_of_mutable_ref(l, self.local_names[local])) else { return }; + + struct ExpressionFinder<'hir> { + capture_span: Span, + closure_change_spans: Vec, + closure_arg_span: Option, + in_closure: bool, + suggest_arg: String, + hir: rustc_middle::hir::map::Map<'hir>, + closure_local_id: Option, + closure_call_changes: Vec<(Span, String)>, + } + impl<'hir> Visitor<'hir> for ExpressionFinder<'hir> { + fn visit_expr(&mut self, e: &'hir hir::Expr<'hir>) { + if e.span.contains(self.capture_span) { + if let hir::ExprKind::Closure(&Closure { + movability: None, + body, + fn_arg_span, + fn_decl: hir::FnDecl{ inputs, .. }, + .. + }) = e.kind && + let Some(hir::Node::Expr(body )) = self.hir.find(body.hir_id) { + self.suggest_arg = "this: &Self".to_string(); + if inputs.len() > 0 { + self.suggest_arg.push_str(", "); + } + self.in_closure = true; + self.closure_arg_span = fn_arg_span; + self.visit_expr(body); + self.in_closure = false; + } + } + if let hir::Expr { kind: hir::ExprKind::Path(path), .. } = e { + if let hir::QPath::Resolved(_, hir::Path { segments: [seg], ..}) = path && + seg.ident.name == kw::SelfLower && self.in_closure { + self.closure_change_spans.push(e.span); + } + } + hir::intravisit::walk_expr(self, e); + } + + fn visit_local(&mut self, local: &'hir hir::Local<'hir>) { + if let hir::Pat { kind: hir::PatKind::Binding(_, hir_id, _ident, _), .. } = local.pat && + let Some(init) = local.init + { + if let hir::Expr { kind: hir::ExprKind::Closure(&Closure { + movability: None, + .. + }), .. } = init && + init.span.contains(self.capture_span) { + self.closure_local_id = Some(*hir_id); + } + } + hir::intravisit::walk_local(self, local); + } + + fn visit_stmt(&mut self, s: &'hir hir::Stmt<'hir>) { + if let hir::StmtKind::Semi(e) = s.kind && + let hir::ExprKind::Call(hir::Expr { kind: hir::ExprKind::Path(path), ..}, args) = e.kind && + let hir::QPath::Resolved(_, hir::Path { segments: [seg], ..}) = path && + let Res::Local(hir_id) = seg.res && + Some(hir_id) == self.closure_local_id { + let mut arg_str = "self".to_string(); + if args.len() > 0 { + arg_str.push_str(", "); + } + self.closure_call_changes.push((seg.ident.span, arg_str)); + } + hir::intravisit::walk_stmt(self, s); + } + } + + if let Some(hir::Node::ImplItem( + hir::ImplItem { kind: hir::ImplItemKind::Fn(_fn_sig, body_id), .. } + )) = hir.find(self.mir_hir_id()) && + let Some(hir::Node::Expr(expr)) = hir.find(body_id.hir_id) { + let mut finder = ExpressionFinder { + capture_span: *capture_kind_span, + closure_change_spans: vec![], + closure_arg_span: None, + in_closure: false, + suggest_arg: String::new(), + closure_local_id: None, + closure_call_changes: vec![], + hir, + }; + finder.visit_expr(expr); + + if finder.closure_change_spans.is_empty() || finder.closure_call_changes.is_empty() { + return; + } + + let mut sugg = vec![]; + let sm = self.infcx.tcx.sess.source_map(); + + if let Some(span) = finder.closure_arg_span { + sugg.push((sm.next_point(span.shrink_to_lo()).shrink_to_hi(), finder.suggest_arg)); + } + for span in finder.closure_change_spans { + sugg.push((span, "this".to_string())); + } + + for (span, suggest) in finder.closure_call_changes { + if let Ok(span) = sm.span_extend_while(span, |c| c != '(') { + sugg.push((sm.next_point(span).shrink_to_hi(), suggest)); + } + } + + err.multipart_suggestion_verbose( + "try explicitly pass `&Self` into the Closure as an argument", + sugg, + Applicability::MachineApplicable, + ); + } + } + /// Returns the description of the root place for a conflicting borrow and the full /// descriptions of the places that caused the conflict. /// diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index b9cfc7e69610..45b15c2c5bd7 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -1094,7 +1094,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { } } -fn mut_borrow_of_mutable_ref(local_decl: &LocalDecl<'_>, local_name: Option) -> bool { +pub fn mut_borrow_of_mutable_ref(local_decl: &LocalDecl<'_>, local_name: Option) -> bool { debug!("local_info: {:?}, ty.kind(): {:?}", local_decl.local_info, local_decl.ty.kind()); match local_decl.local_info.as_deref() { diff --git a/tests/ui/suggestions/issue-105761-suggest-self-for-closure.fixed b/tests/ui/suggestions/issue-105761-suggest-self-for-closure.fixed new file mode 100644 index 000000000000..78e48364bba0 --- /dev/null +++ b/tests/ui/suggestions/issue-105761-suggest-self-for-closure.fixed @@ -0,0 +1,28 @@ +//run-rustfix +#![allow(unused)] + +struct S; +impl S { + fn foo(&mut self) { + let x = |this: &Self, v: i32| { + this.bar(); + this.hel(); + }; + self.qux(); //~ ERROR cannot borrow `*self` as mutable because it is also borrowed as immutable + x(self, 1); + x(self, 3); + } + fn bar(&self) {} + fn hel(&self) {} + fn qux(&mut self) {} + + fn hello(&mut self) { + let y = |this: &Self| { + this.bar(); + }; + self.qux(); //~ ERROR cannot borrow `*self` as mutable because it is also borrowed as immutable + y(self); + } +} + +fn main() {} diff --git a/tests/ui/suggestions/issue-105761-suggest-self-for-closure.rs b/tests/ui/suggestions/issue-105761-suggest-self-for-closure.rs new file mode 100644 index 000000000000..6d8a9ffc12d3 --- /dev/null +++ b/tests/ui/suggestions/issue-105761-suggest-self-for-closure.rs @@ -0,0 +1,28 @@ +//run-rustfix +#![allow(unused)] + +struct S; +impl S { + fn foo(&mut self) { + let x = |v: i32| { + self.bar(); + self.hel(); + }; + self.qux(); //~ ERROR cannot borrow `*self` as mutable because it is also borrowed as immutable + x(1); + x(3); + } + fn bar(&self) {} + fn hel(&self) {} + fn qux(&mut self) {} + + fn hello(&mut self) { + let y = || { + self.bar(); + }; + self.qux(); //~ ERROR cannot borrow `*self` as mutable because it is also borrowed as immutable + y(); + } +} + +fn main() {} diff --git a/tests/ui/suggestions/issue-105761-suggest-self-for-closure.stderr b/tests/ui/suggestions/issue-105761-suggest-self-for-closure.stderr new file mode 100644 index 000000000000..bc97d32ebb6e --- /dev/null +++ b/tests/ui/suggestions/issue-105761-suggest-self-for-closure.stderr @@ -0,0 +1,49 @@ +error[E0502]: cannot borrow `*self` as mutable because it is also borrowed as immutable + --> $DIR/issue-105761-suggest-self-for-closure.rs:11:9 + | +LL | let x = |v: i32| { + | -------- immutable borrow occurs here +LL | self.bar(); + | ---- first borrow occurs due to use of `self` in closure +... +LL | self.qux(); + | ^^^^^^^^^^ mutable borrow occurs here +LL | x(1); + | - immutable borrow later used here + | +help: try explicitly pass `&Self` into the Closure as an argument + | +LL ~ let x = |this: &Self, v: i32| { +LL ~ this.bar(); +LL ~ this.hel(); +LL | }; +LL | self.qux(); +LL ~ x(self, 1); +LL ~ x(self, 3); + | + +error[E0502]: cannot borrow `*self` as mutable because it is also borrowed as immutable + --> $DIR/issue-105761-suggest-self-for-closure.rs:23:9 + | +LL | let y = || { + | -- immutable borrow occurs here +LL | self.bar(); + | ---- first borrow occurs due to use of `self` in closure +LL | }; +LL | self.qux(); + | ^^^^^^^^^^ mutable borrow occurs here +LL | y(); + | - immutable borrow later used here + | +help: try explicitly pass `&Self` into the Closure as an argument + | +LL ~ let y = |this: &Self| { +LL ~ this.bar(); +LL | }; +LL | self.qux(); +LL ~ y(self); + | + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0502`. From eafbca916632ef8a92570fe3bf2c88ac6fbb3665 Mon Sep 17 00:00:00 2001 From: yukang Date: Sun, 8 Jan 2023 00:32:40 +0800 Subject: [PATCH 09/16] take care when there is no args in method call --- .../src/diagnostics/conflict_errors.rs | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 04bbceadd5a5..968c1f49b95c 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -1,6 +1,4 @@ -use crate::diagnostics::mutability_errors::mut_borrow_of_mutable_ref; use either::Either; -use hir::Closure; use rustc_const_eval::util::CallKind; use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::FxHashSet; @@ -8,6 +6,7 @@ use rustc_errors::{ struct_span_err, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, MultiSpan, }; use rustc_hir as hir; +use rustc_hir::def::Res; use rustc_hir::intravisit::{walk_block, walk_expr, Visitor}; use rustc_hir::{AsyncGeneratorKind, GeneratorKind, LangItem}; use rustc_infer::infer::TyCtxtInferExt; @@ -31,6 +30,7 @@ use crate::borrowck_errors; use crate::diagnostics::conflict_errors::StorageDeadOrDrop::LocalStorageDead; use crate::diagnostics::find_all_local_uses; +use crate::diagnostics::mutability_errors::mut_borrow_of_mutable_ref; use crate::{ borrow_set::BorrowData, diagnostics::Instance, prefixes::IsPrefixOf, InitializationRequiringAction, MirBorrowckCtxt, PrefixSet, WriteKind, @@ -41,8 +41,6 @@ use super::{ DescribePlaceOpt, RegionName, RegionNameSource, UseSpans, }; -use rustc_hir::def::Res; - #[derive(Debug)] struct MoveSite { /// Index of the "move out" that we found. The `MoveData` can @@ -1280,7 +1278,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { impl<'hir> Visitor<'hir> for ExpressionFinder<'hir> { fn visit_expr(&mut self, e: &'hir hir::Expr<'hir>) { if e.span.contains(self.capture_span) { - if let hir::ExprKind::Closure(&Closure { + if let hir::ExprKind::Closure(&hir::Closure { movability: None, body, fn_arg_span, @@ -1311,7 +1309,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { if let hir::Pat { kind: hir::PatKind::Binding(_, hir_id, _ident, _), .. } = local.pat && let Some(init) = local.init { - if let hir::Expr { kind: hir::ExprKind::Closure(&Closure { + if let hir::Expr { kind: hir::ExprKind::Closure(&hir::Closure { movability: None, .. }), .. } = init && @@ -1328,11 +1326,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let hir::QPath::Resolved(_, hir::Path { segments: [seg], ..}) = path && let Res::Local(hir_id) = seg.res && Some(hir_id) == self.closure_local_id { - let mut arg_str = "self".to_string(); - if args.len() > 0 { - arg_str.push_str(", "); - } - self.closure_call_changes.push((seg.ident.span, arg_str)); + let (span, arg_str) = if args.len() > 0 { + (args[0].span.shrink_to_lo(), "self, ".to_string()) + } else { + let span = e.span.trim_start(seg.ident.span).unwrap_or(e.span); + (span, "(self)".to_string()) + }; + self.closure_call_changes.push((span, arg_str)); } hir::intravisit::walk_stmt(self, s); } @@ -1369,9 +1369,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } for (span, suggest) in finder.closure_call_changes { - if let Ok(span) = sm.span_extend_while(span, |c| c != '(') { - sugg.push((sm.next_point(span).shrink_to_hi(), suggest)); - } + sugg.push((span, suggest)); } err.multipart_suggestion_verbose( From 54571407b2ac4bd29f6509b680a19b49d7fbaa16 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 4 Jan 2023 20:55:37 +0000 Subject: [PATCH 10/16] Bump IMPLIED_BOUNDS_ENTAILMENT to Deny + ReportNow --- compiler/rustc_lint_defs/src/builtin.rs | 4 ++-- ...plied-bounds-compatibility-unnormalized.stderr | 15 +++++++++++++++ .../impl-implied-bounds-compatibility.stderr | 15 +++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 28317d6cea02..6cdf50970836 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -4033,10 +4033,10 @@ declare_lint! { /// /// This can be used to implement an unsound API if used incorrectly. pub IMPLIED_BOUNDS_ENTAILMENT, - Warn, + Deny, "impl method assumes more implied bounds than its corresponding trait method", @future_incompatible = FutureIncompatibleInfo { reference: "issue #105572 ", - reason: FutureIncompatibilityReason::FutureReleaseError, + reason: FutureIncompatibilityReason::FutureReleaseErrorReportNow, }; } diff --git a/tests/ui/implied-bounds/impl-implied-bounds-compatibility-unnormalized.stderr b/tests/ui/implied-bounds/impl-implied-bounds-compatibility-unnormalized.stderr index 0ac31c642eb1..961ff573e504 100644 --- a/tests/ui/implied-bounds/impl-implied-bounds-compatibility-unnormalized.stderr +++ b/tests/ui/implied-bounds/impl-implied-bounds-compatibility-unnormalized.stderr @@ -14,3 +14,18 @@ LL | #![deny(implied_bounds_entailment)] error: aborting due to previous error +Future incompatibility report: Future breakage diagnostic: +error: impl method assumes more implied bounds than the corresponding trait method + --> $DIR/impl-implied-bounds-compatibility-unnormalized.rs:13:5 + | +LL | fn get<'s>(s: &'s str, _: <&'static &'s () as Project>::Ty) -> &'static str { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #105572 +note: the lint level is defined here + --> $DIR/impl-implied-bounds-compatibility-unnormalized.rs:1:9 + | +LL | #![deny(implied_bounds_entailment)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + diff --git a/tests/ui/implied-bounds/impl-implied-bounds-compatibility.stderr b/tests/ui/implied-bounds/impl-implied-bounds-compatibility.stderr index 0dfa8167a996..4841f2b17993 100644 --- a/tests/ui/implied-bounds/impl-implied-bounds-compatibility.stderr +++ b/tests/ui/implied-bounds/impl-implied-bounds-compatibility.stderr @@ -14,3 +14,18 @@ LL | #![deny(implied_bounds_entailment)] error: aborting due to previous error +Future incompatibility report: Future breakage diagnostic: +error: impl method assumes more implied bounds than the corresponding trait method + --> $DIR/impl-implied-bounds-compatibility.rs:14:5 + | +LL | fn listeners<'b>(&'b self) -> &'a MessageListeners<'b> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #105572 +note: the lint level is defined here + --> $DIR/impl-implied-bounds-compatibility.rs:1:9 + | +LL | #![deny(implied_bounds_entailment)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + From eaa7cc84d358a0113c063d445e5d0d7caeeea94c Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 12 Jan 2023 20:43:44 +0000 Subject: [PATCH 11/16] Add logic to make IMPLIED_BOUNDS_ENTAILMENT easier to understand --- .../src/check/compare_impl_item.rs | 160 ++++++++++++++++-- ...d-bounds-compatibility-unnormalized.stderr | 8 +- .../impl-implied-bounds-compatibility.stderr | 8 +- 3 files changed, 156 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs index 7af89934d142..2cdf75794713 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs @@ -2,7 +2,9 @@ use super::potentially_plural_count; use crate::errors::LifetimesOrBoundsMismatchOnTrait; use hir::def_id::{DefId, LocalDefId}; use rustc_data_structures::fx::{FxHashMap, FxIndexSet}; -use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticId, ErrorGuaranteed}; +use rustc_errors::{ + pluralize, struct_span_err, Applicability, DiagnosticId, ErrorGuaranteed, MultiSpan, +}; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; use rustc_hir::intravisit; @@ -320,15 +322,6 @@ fn compare_method_predicate_entailment<'tcx>( ty::Binder::dummy(ty::PredicateKind::WellFormed(unnormalized_impl_fty.into())), )); } - let emit_implied_wf_lint = || { - infcx.tcx.struct_span_lint_hir( - rustc_session::lint::builtin::IMPLIED_BOUNDS_ENTAILMENT, - impl_m_hir_id, - infcx.tcx.def_span(impl_m.def_id), - "impl method assumes more implied bounds than the corresponding trait method", - |lint| lint, - ); - }; // Check that all obligations are satisfied by the implementation's // version. @@ -346,7 +339,7 @@ fn compare_method_predicate_entailment<'tcx>( ) .map(|()| { // If the skip-mode was successful, emit a lint. - emit_implied_wf_lint(); + emit_implied_wf_lint(infcx.tcx, impl_m, impl_m_hir_id, vec![]); }); } CheckImpliedWfMode::Skip => { @@ -382,8 +375,16 @@ fn compare_method_predicate_entailment<'tcx>( CheckImpliedWfMode::Skip, ) .map(|()| { + let bad_args = extract_bad_args_for_implies_lint( + tcx, + &errors, + (trait_m, trait_sig), + // Unnormalized impl sig corresponds to the HIR types written + (impl_m, unnormalized_impl_sig), + impl_m_hir_id, + ); // If the skip-mode was successful, emit a lint. - emit_implied_wf_lint(); + emit_implied_wf_lint(tcx, impl_m, impl_m_hir_id, bad_args); }); } CheckImpliedWfMode::Skip => { @@ -400,6 +401,141 @@ fn compare_method_predicate_entailment<'tcx>( Ok(()) } +fn extract_bad_args_for_implies_lint<'tcx>( + tcx: TyCtxt<'tcx>, + errors: &[infer::RegionResolutionError<'tcx>], + (trait_m, trait_sig): (&ty::AssocItem, ty::FnSig<'tcx>), + (impl_m, impl_sig): (&ty::AssocItem, ty::FnSig<'tcx>), + hir_id: hir::HirId, +) -> Vec<(Span, Option)> { + let mut blame_generics = vec![]; + for error in errors { + // Look for the subregion origin that contains an input/output type + let origin = match error { + infer::RegionResolutionError::ConcreteFailure(o, ..) => o, + infer::RegionResolutionError::GenericBoundFailure(o, ..) => o, + infer::RegionResolutionError::SubSupConflict(_, _, o, ..) => o, + infer::RegionResolutionError::UpperBoundUniverseConflict(.., o, _) => o, + }; + // Extract (possible) input/output types from origin + match origin { + infer::SubregionOrigin::Subtype(trace) => { + if let Some((a, b)) = trace.values.ty() { + blame_generics.extend([a, b]); + } + } + infer::SubregionOrigin::RelateParamBound(_, ty, _) => blame_generics.push(*ty), + infer::SubregionOrigin::ReferenceOutlivesReferent(ty, _) => blame_generics.push(*ty), + _ => {} + } + } + + let fn_decl = tcx.hir().fn_decl_by_hir_id(hir_id).unwrap(); + let opt_ret_ty = match fn_decl.output { + hir::FnRetTy::DefaultReturn(_) => None, + hir::FnRetTy::Return(ty) => Some(ty), + }; + + // Map late-bound regions from trait to impl, so the names are right. + let mapping = std::iter::zip( + tcx.fn_sig(trait_m.def_id).bound_vars(), + tcx.fn_sig(impl_m.def_id).bound_vars(), + ) + .filter_map(|(impl_bv, trait_bv)| { + if let ty::BoundVariableKind::Region(impl_bv) = impl_bv + && let ty::BoundVariableKind::Region(trait_bv) = trait_bv + { + Some((impl_bv, trait_bv)) + } else { + None + } + }) + .collect(); + + // For each arg, see if it was in the "blame" of any of the region errors. + // If so, then try to produce a suggestion to replace the argument type with + // one from the trait. + let mut bad_args = vec![]; + for (idx, (ty, hir_ty)) in + std::iter::zip(impl_sig.inputs_and_output, fn_decl.inputs.iter().chain(opt_ret_ty)) + .enumerate() + { + let expected_ty = trait_sig.inputs_and_output[idx] + .fold_with(&mut RemapLateBound { tcx, mapping: &mapping }); + if blame_generics.iter().any(|blame| ty.contains(*blame)) { + let expected_ty_sugg = expected_ty.to_string(); + bad_args.push(( + hir_ty.span, + // Only suggest something if it actually changed. + (expected_ty_sugg != ty.to_string()).then_some(expected_ty_sugg), + )); + } + } + + bad_args +} + +struct RemapLateBound<'a, 'tcx> { + tcx: TyCtxt<'tcx>, + mapping: &'a FxHashMap, +} + +impl<'tcx> TypeFolder<'tcx> for RemapLateBound<'_, 'tcx> { + fn tcx(&self) -> TyCtxt<'tcx> { + self.tcx + } + + fn fold_region(&mut self, r: ty::Region<'tcx>) -> ty::Region<'tcx> { + if let ty::ReFree(fr) = *r { + self.tcx.mk_region(ty::ReFree(ty::FreeRegion { + bound_region: self + .mapping + .get(&fr.bound_region) + .copied() + .unwrap_or(fr.bound_region), + ..fr + })) + } else { + r + } + } +} + +fn emit_implied_wf_lint<'tcx>( + tcx: TyCtxt<'tcx>, + impl_m: &ty::AssocItem, + hir_id: hir::HirId, + bad_args: Vec<(Span, Option)>, +) { + let span: MultiSpan = if bad_args.is_empty() { + tcx.def_span(impl_m.def_id).into() + } else { + bad_args.iter().map(|(span, _)| *span).collect::>().into() + }; + tcx.struct_span_lint_hir( + rustc_session::lint::builtin::IMPLIED_BOUNDS_ENTAILMENT, + hir_id, + span, + "impl method assumes more implied bounds than the corresponding trait method", + |lint| { + let bad_args: Vec<_> = + bad_args.into_iter().filter_map(|(span, sugg)| Some((span, sugg?))).collect(); + if !bad_args.is_empty() { + lint.multipart_suggestion( + format!( + "replace {} type{} to make the impl signature compatible", + pluralize!("this", bad_args.len()), + pluralize!(bad_args.len()) + ), + bad_args, + Applicability::MaybeIncorrect, + ); + } + lint + }, + ); +} + #[derive(Debug, PartialEq, Eq)] enum CheckImpliedWfMode { /// Checks implied well-formedness of the impl method. If it fails, we will diff --git a/tests/ui/implied-bounds/impl-implied-bounds-compatibility-unnormalized.stderr b/tests/ui/implied-bounds/impl-implied-bounds-compatibility-unnormalized.stderr index 961ff573e504..ebe07027d2fa 100644 --- a/tests/ui/implied-bounds/impl-implied-bounds-compatibility-unnormalized.stderr +++ b/tests/ui/implied-bounds/impl-implied-bounds-compatibility-unnormalized.stderr @@ -1,8 +1,8 @@ error: impl method assumes more implied bounds than the corresponding trait method - --> $DIR/impl-implied-bounds-compatibility-unnormalized.rs:13:5 + --> $DIR/impl-implied-bounds-compatibility-unnormalized.rs:13:31 | LL | fn get<'s>(s: &'s str, _: <&'static &'s () as Project>::Ty) -> &'static str { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace this type to make the impl signature compatible: `()` | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #105572 @@ -16,10 +16,10 @@ error: aborting due to previous error Future incompatibility report: Future breakage diagnostic: error: impl method assumes more implied bounds than the corresponding trait method - --> $DIR/impl-implied-bounds-compatibility-unnormalized.rs:13:5 + --> $DIR/impl-implied-bounds-compatibility-unnormalized.rs:13:31 | LL | fn get<'s>(s: &'s str, _: <&'static &'s () as Project>::Ty) -> &'static str { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace this type to make the impl signature compatible: `()` | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #105572 diff --git a/tests/ui/implied-bounds/impl-implied-bounds-compatibility.stderr b/tests/ui/implied-bounds/impl-implied-bounds-compatibility.stderr index 4841f2b17993..43d3e058ffeb 100644 --- a/tests/ui/implied-bounds/impl-implied-bounds-compatibility.stderr +++ b/tests/ui/implied-bounds/impl-implied-bounds-compatibility.stderr @@ -1,8 +1,8 @@ error: impl method assumes more implied bounds than the corresponding trait method - --> $DIR/impl-implied-bounds-compatibility.rs:14:5 + --> $DIR/impl-implied-bounds-compatibility.rs:14:35 | LL | fn listeners<'b>(&'b self) -> &'a MessageListeners<'b> { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace this type to make the impl signature compatible: `&'b MessageListeners<'b>` | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #105572 @@ -16,10 +16,10 @@ error: aborting due to previous error Future incompatibility report: Future breakage diagnostic: error: impl method assumes more implied bounds than the corresponding trait method - --> $DIR/impl-implied-bounds-compatibility.rs:14:5 + --> $DIR/impl-implied-bounds-compatibility.rs:14:35 | LL | fn listeners<'b>(&'b self) -> &'a MessageListeners<'b> { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace this type to make the impl signature compatible: `&'b MessageListeners<'b>` | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #105572 From 95ef76b8aa0563192d549314a375913ce68b4902 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Thu, 12 Jan 2023 21:28:20 -0500 Subject: [PATCH 12/16] Normalize test output more thoroughly This prevents differences in local environments, which may (for example) end up with a longer backtrace with more digits in the backtrace prefix, as happened to me. While we're at it, clean more of the output up, including the exact location of the error in the compiler. --- tests/ui/chalkify/bugs/async.rs | 17 ++++++-- tests/ui/chalkify/bugs/async.stderr | 60 +++++++++++++++++++---------- 2 files changed, 52 insertions(+), 25 deletions(-) diff --git a/tests/ui/chalkify/bugs/async.rs b/tests/ui/chalkify/bugs/async.rs index 86ce42631b43..1c69b07e3d4a 100644 --- a/tests/ui/chalkify/bugs/async.rs +++ b/tests/ui/chalkify/bugs/async.rs @@ -2,12 +2,21 @@ // known-bug // unset-rustc-env:RUST_BACKTRACE // compile-flags:-Z trait-solver=chalk --edition=2021 -// error-pattern:stack backtrace: +// error-pattern:internal compiler error // failure-status:101 -// normalize-stderr-test "note: .*" -> "" -// normalize-stderr-test "thread 'rustc' .*" -> "" -// normalize-stderr-test " .*\n" -> "" // normalize-stderr-test "DefId([^)]*)" -> "..." +// normalize-stderr-test "\nerror: internal compiler error.*\n\n" -> "" +// normalize-stderr-test "note:.*unexpectedly panicked.*\n\n" -> "" +// normalize-stderr-test "note: we would appreciate a bug report.*\n\n" -> "" +// normalize-stderr-test "note: compiler flags.*\n\n" -> "" +// normalize-stderr-test "note: rustc.*running on.*\n\n" -> "" +// normalize-stderr-test "thread.*panicked.*\n" -> "" +// normalize-stderr-test "stack backtrace:\n" -> "" +// normalize-stderr-test "\s\d{1,}: .*\n" -> "" +// normalize-stderr-test "\s at .*\n" -> "" +// normalize-stderr-test ".*note: Some details.*\n" -> "" +// normalize-stderr-test "\n\n[ ]*\n" -> "" +// normalize-stderr-test "compiler/.*: projection" -> "projection" fn main() -> () {} diff --git a/tests/ui/chalkify/bugs/async.stderr b/tests/ui/chalkify/bugs/async.stderr index 7e2466dece43..d1508cb17001 100644 --- a/tests/ui/chalkify/bugs/async.stderr +++ b/tests/ui/chalkify/bugs/async.stderr @@ -1,29 +1,47 @@ -error[E0277]: `[async fn body@$DIR/async.rs:14:29: 16:2]` is not a future -LL |LL | |LL | | } +error[E0277]: `[async fn body@$DIR/async.rs:23:29: 25:2]` is not a future + --> $DIR/async.rs:23:29 + | +LL | async fn foo(x: u32) -> u32 { + | _____________________________- +LL | | x +LL | | } + | | ^ + | | | + | |_`[async fn body@$DIR/async.rs:23:29: 25:2]` is not a future + | required by a bound introduced by this call + | + = help: the trait `Future` is not implemented for `[async fn body@$DIR/async.rs:23:29: 25:2]` + = note: [async fn body@$DIR/async.rs:23:29: 25:2] must be a future or must implement `IntoFuture` to be awaited +note: required by a bound in `identity_future` + --> $SRC_DIR/core/src/future/mod.rs:LL:COL +error[E0277]: the size for values of type `<[async fn body@$DIR/async.rs:23:29: 25:2] as Future>::Output` cannot be known at compilation time + --> $DIR/async.rs:23:29 + | +LL | async fn foo(x: u32) -> u32 { + | _____________________________^ +LL | | x +LL | | } + | |_^ doesn't have a size known at compile-time + | + = help: the trait `Sized` is not implemented for `<[async fn body@$DIR/async.rs:23:29: 25:2] as Future>::Output` +note: required by a bound in `identity_future` + --> $SRC_DIR/core/src/future/mod.rs:LL:COL -error[E0277]: the size for values of type `<[async fn body@$DIR/async.rs:14:29: 16:2] as Future>::Output` cannot be known at compilation time -LL |LL | |LL | | } - - -error[E0277]: `[async fn body@$DIR/async.rs:14:29: 16:2]` is not a future +error[E0277]: `[async fn body@$DIR/async.rs:23:29: 25:2]` is not a future + --> $DIR/async.rs:23:25 + | LL | async fn foo(x: u32) -> u32 { + | ^^^ `[async fn body@$DIR/async.rs:23:29: 25:2]` is not a future + | + = help: the trait `Future` is not implemented for `[async fn body@$DIR/async.rs:23:29: 25:2]` + = note: [async fn body@$DIR/async.rs:23:29: 25:2] must be a future or must implement `IntoFuture` to be awaited -error: internal compiler error: compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs:1114:25: projection clauses should be implied from elsewhere. obligation: `Obligation(predicate=Binder(ProjectionPredicate(AliasTy { substs: [[async fn body@$DIR/async.rs:14:29: 16:2]], def_id: ...), _use_mk_alias_ty_instead: () }, Term::Ty(u32)), []), depth=0)` +error: internal compiler error: projection clauses should be implied from elsewhere. obligation: `Obligation(predicate=Binder(ProjectionPredicate(AliasTy { substs: [[async fn body@$DIR/async.rs:23:29: 25:2]], def_id: ...), _use_mk_alias_ty_instead: () }, Term::Ty(u32)), []), depth=0)` + --> $DIR/async.rs:23:25 + | LL | async fn foo(x: u32) -> u32 { - - -stack backtrace: - - - - - - - - - -query stack during panic: + | ^^^query stack during panic: #0 [typeck] type-checking `foo` #1 [thir_body] building THIR for `foo` #2 [mir_built] building MIR for `foo` From 138a1d26b53cab16066b0faa3846722358a2c09f Mon Sep 17 00:00:00 2001 From: Fawaz Date: Mon, 9 Jan 2023 00:17:58 -0800 Subject: [PATCH 13/16] riscv: Fix ELF header flags The previous version added both `EF_RISCV_FLOAT_ABI_DOUBLE` and `EF_RISCV_RVC` if the "D" extension was enabled on riscv64 targets. riscv32 targets were not accounted for. This patch changes this so that: - Only add `EF_RISCV_RVC` if the "C" extension is enabled - Add `EF_RISCV_FLOAT_ABI_SINGLE` if the "F" extension is enabled and the "D" extension is not - Add these ELF flags for riscv32 as well --- .../rustc_codegen_ssa/src/back/metadata.rs | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/metadata.rs b/compiler/rustc_codegen_ssa/src/back/metadata.rs index 51c5c375d519..5ad2744f61de 100644 --- a/compiler/rustc_codegen_ssa/src/back/metadata.rs +++ b/compiler/rustc_codegen_ssa/src/back/metadata.rs @@ -165,11 +165,23 @@ pub(crate) fn create_object_file(sess: &Session) -> Option { - // copied from `riscv64-linux-gnu-gcc foo.c -c`, note though - // that the `+d` target feature represents whether the double - // float abi is enabled. - let e_flags = elf::EF_RISCV_RVC | elf::EF_RISCV_FLOAT_ABI_DOUBLE; + Architecture::Riscv32 | Architecture::Riscv64 => { + // Source: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/079772828bd10933d34121117a222b4cc0ee2200/riscv-elf.adoc + let mut e_flags: u32 = 0x0; + let features = &sess.target.options.features; + // Check if compressed is enabled + if features.contains("+c") { + e_flags |= elf::EF_RISCV_RVC; + } + + // Select the appropriate floating-point ABI + if features.contains("+d") { + e_flags |= elf::EF_RISCV_FLOAT_ABI_DOUBLE; + } else if features.contains("+f") { + e_flags |= elf::EF_RISCV_FLOAT_ABI_SINGLE; + } else { + e_flags |= elf::EF_RISCV_FLOAT_ABI_SOFT; + } e_flags } _ => 0, From 549ece703393bf78c7567605862496435cae7202 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 10 Jan 2023 13:57:42 +0100 Subject: [PATCH 14/16] Warn when using panic-strategy abort for proc-macro crates --- compiler/rustc_error_messages/locales/en-US/interface.ftl | 3 +++ compiler/rustc_interface/Cargo.toml | 1 + compiler/rustc_interface/src/errors.rs | 4 ++++ compiler/rustc_interface/src/passes.rs | 8 +++++++- tests/ui/proc-macro/panic-abort.rs | 4 ++++ tests/ui/proc-macro/panic-abort.stderr | 4 ++++ 6 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 tests/ui/proc-macro/panic-abort.rs create mode 100644 tests/ui/proc-macro/panic-abort.stderr diff --git a/compiler/rustc_error_messages/locales/en-US/interface.ftl b/compiler/rustc_error_messages/locales/en-US/interface.ftl index bbcb8fc28cff..688b04472226 100644 --- a/compiler/rustc_error_messages/locales/en-US/interface.ftl +++ b/compiler/rustc_error_messages/locales/en-US/interface.ftl @@ -41,3 +41,6 @@ interface_rustc_error_unexpected_annotation = interface_failed_writing_file = failed to write file {$path}: {$error}" + +interface_proc_macro_crate_panic_abort = + building proc macro crate with `panic=abort` may crash the compiler should the proc-macro panic diff --git a/compiler/rustc_interface/Cargo.toml b/compiler/rustc_interface/Cargo.toml index e67dec31dcee..f817c5bc1cd7 100644 --- a/compiler/rustc_interface/Cargo.toml +++ b/compiler/rustc_interface/Cargo.toml @@ -45,6 +45,7 @@ rustc_plugin_impl = { path = "../rustc_plugin_impl" } rustc_privacy = { path = "../rustc_privacy" } rustc_query_impl = { path = "../rustc_query_impl" } rustc_resolve = { path = "../rustc_resolve" } +rustc_target = { path = "../rustc_target" } rustc_trait_selection = { path = "../rustc_trait_selection" } rustc_ty_utils = { path = "../rustc_ty_utils" } diff --git a/compiler/rustc_interface/src/errors.rs b/compiler/rustc_interface/src/errors.rs index f5135c78dc83..15d7e977bbe8 100644 --- a/compiler/rustc_interface/src/errors.rs +++ b/compiler/rustc_interface/src/errors.rs @@ -87,3 +87,7 @@ pub struct FailedWritingFile<'a> { pub path: &'a Path, pub error: io::Error, } + +#[derive(Diagnostic)] +#[diag(interface_proc_macro_crate_panic_abort)] +pub struct ProcMacroCratePanicAbort; diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index 86d56385bc96..c8085e91dfae 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -1,7 +1,8 @@ use crate::errors::{ CantEmitMIR, EmojiIdentifier, ErrorWritingDependencies, FerrisIdentifier, GeneratedFileConflictsWithDirectory, InputFileWouldBeOverWritten, MixedBinCrate, - MixedProcMacroCrate, OutDirError, ProcMacroDocWithoutArg, TempsDirError, + MixedProcMacroCrate, OutDirError, ProcMacroCratePanicAbort, ProcMacroDocWithoutArg, + TempsDirError, }; use crate::interface::{Compiler, Result}; use crate::proc_macro_decls; @@ -36,6 +37,7 @@ use rustc_session::search_paths::PathKind; use rustc_session::{Limit, Session}; use rustc_span::symbol::{sym, Symbol}; use rustc_span::FileName; +use rustc_target::spec::PanicStrategy; use rustc_trait_selection::traits; use std::any::Any; @@ -380,6 +382,10 @@ pub fn configure_and_expand( } } + if is_proc_macro_crate && sess.panic_strategy() == PanicStrategy::Abort { + sess.emit_warning(ProcMacroCratePanicAbort); + } + // For backwards compatibility, we don't try to run proc macro injection // if rustdoc is run on a proc macro crate without '--crate-type proc-macro' being // specified. This should only affect users who manually invoke 'rustdoc', as diff --git a/tests/ui/proc-macro/panic-abort.rs b/tests/ui/proc-macro/panic-abort.rs new file mode 100644 index 000000000000..ad312a875e39 --- /dev/null +++ b/tests/ui/proc-macro/panic-abort.rs @@ -0,0 +1,4 @@ +// error-pattern: building proc macro crate with `panic=abort` may crash the compiler should the proc-macro panic +// compile-flags: --crate-type proc-macro -Cpanic=abort +// force-host +// check-pass diff --git a/tests/ui/proc-macro/panic-abort.stderr b/tests/ui/proc-macro/panic-abort.stderr new file mode 100644 index 000000000000..a6e18614f8f0 --- /dev/null +++ b/tests/ui/proc-macro/panic-abort.stderr @@ -0,0 +1,4 @@ +warning: building proc macro crate with `panic=abort` may crash the compiler should the proc-macro panic + +warning: 1 warning emitted + From 4aca7beab0fd7e45dc4f901db5aa10c46ba9cf1b Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 8 Dec 2022 09:39:48 +0000 Subject: [PATCH 15/16] Remove redundant session field --- compiler/rustc_resolve/src/lib.rs | 6 ++++- src/librustdoc/lib.rs | 3 --- .../passes/collect_intra_doc_links/early.rs | 22 +++++++++---------- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 2182b7369377..e5c9169358d3 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -1133,7 +1133,7 @@ impl<'a, 'b> DefIdTree for &'a Resolver<'b> { } } -impl Resolver<'_> { +impl<'a> Resolver<'a> { fn opt_local_def_id(&self, node: NodeId) -> Option { self.node_id_to_def_id.get(&node).copied() } @@ -1190,6 +1190,10 @@ impl Resolver<'_> { self.cstore().item_generics_num_lifetimes(def_id, self.session) } } + + pub fn sess(&self) -> &'a Session { + self.session + } } impl<'a> Resolver<'a> { diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index ed77de200a9b..86454e1f2eb7 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -772,7 +772,6 @@ fn main_args(at_args: &[String]) -> MainResult { let crate_version = options.crate_version.clone(); let output_format = options.output_format; - let externs = options.externs.clone(); let scrape_examples_options = options.scrape_examples_options.clone(); let bin_crate = options.bin_crate; @@ -805,9 +804,7 @@ fn main_args(at_args: &[String]) -> MainResult { let resolver_caches = resolver.borrow_mut().access(|resolver| { collect_intra_doc_links::early_resolve_intra_doc_links( resolver, - sess, krate, - externs, render_options.document_private, ) }); diff --git a/src/librustdoc/passes/collect_intra_doc_links/early.rs b/src/librustdoc/passes/collect_intra_doc_links/early.rs index 1b373cfe5bb7..42677bd84974 100644 --- a/src/librustdoc/passes/collect_intra_doc_links/early.rs +++ b/src/librustdoc/passes/collect_intra_doc_links/early.rs @@ -12,8 +12,6 @@ use rustc_hir::def_id::{DefId, DefIdMap, DefIdSet, CRATE_DEF_ID}; use rustc_hir::TraitCandidate; use rustc_middle::ty::{DefIdTree, Visibility}; use rustc_resolve::{ParentScope, Resolver}; -use rustc_session::config::Externs; -use rustc_session::Session; use rustc_span::symbol::sym; use rustc_span::{Symbol, SyntaxContext}; @@ -22,16 +20,13 @@ use std::mem; pub(crate) fn early_resolve_intra_doc_links( resolver: &mut Resolver<'_>, - sess: &Session, krate: &ast::Crate, - externs: Externs, document_private_items: bool, ) -> ResolverCaches { let parent_scope = ParentScope::module(resolver.expect_module(CRATE_DEF_ID.to_def_id()), resolver); let mut link_resolver = EarlyDocLinkResolver { resolver, - sess, parent_scope, visited_mods: Default::default(), markdown_links: Default::default(), @@ -52,7 +47,9 @@ pub(crate) fn early_resolve_intra_doc_links( // the known necessary crates. Load them all unconditionally until we find a way to fix this. // DO NOT REMOVE THIS without first testing on the reproducer in // https://github.com/jyn514/objr/commit/edcee7b8124abf0e4c63873e8422ff81beb11ebb - for (extern_name, _) in externs.iter().filter(|(_, entry)| entry.add_prelude) { + for (extern_name, _) in + link_resolver.resolver.sess().opts.externs.iter().filter(|(_, entry)| entry.add_prelude) + { link_resolver.resolver.resolve_rustdoc_path(extern_name, TypeNS, parent_scope); } @@ -73,7 +70,6 @@ fn doc_attrs<'a>(attrs: impl Iterator) -> Attributes struct EarlyDocLinkResolver<'r, 'ra> { resolver: &'r mut Resolver<'ra>, - sess: &'r Session, parent_scope: ParentScope<'ra>, visited_mods: DefIdSet, markdown_links: FxHashMap>, @@ -166,7 +162,7 @@ impl<'ra> EarlyDocLinkResolver<'_, 'ra> { fn resolve_doc_links_extern_impl(&mut self, def_id: DefId, is_inherent: bool) { self.resolve_doc_links_extern_outer_fixme(def_id, def_id); let assoc_item_def_ids = Vec::from_iter( - self.resolver.cstore().associated_item_def_ids_untracked(def_id, self.sess), + self.resolver.cstore().associated_item_def_ids_untracked(def_id, self.resolver.sess()), ); for assoc_def_id in assoc_item_def_ids { if !is_inherent || self.resolver.cstore().visibility_untracked(assoc_def_id).is_public() @@ -191,7 +187,9 @@ impl<'ra> EarlyDocLinkResolver<'_, 'ra> { if !self.resolver.cstore().may_have_doc_links_untracked(def_id) { return; } - let attrs = Vec::from_iter(self.resolver.cstore().item_attrs_untracked(def_id, self.sess)); + let attrs = Vec::from_iter( + self.resolver.cstore().item_attrs_untracked(def_id, self.resolver.sess()), + ); let parent_scope = ParentScope::module( self.resolver.get_nearest_non_block_module( self.resolver.opt_parent(scope_id).unwrap_or(scope_id), @@ -205,7 +203,9 @@ impl<'ra> EarlyDocLinkResolver<'_, 'ra> { if !self.resolver.cstore().may_have_doc_links_untracked(def_id) { return; } - let attrs = Vec::from_iter(self.resolver.cstore().item_attrs_untracked(def_id, self.sess)); + let attrs = Vec::from_iter( + self.resolver.cstore().item_attrs_untracked(def_id, self.resolver.sess()), + ); let parent_scope = ParentScope::module(self.resolver.expect_module(def_id), self.resolver); self.resolve_doc_links(doc_attrs(attrs.iter()), parent_scope); } @@ -321,7 +321,7 @@ impl<'ra> EarlyDocLinkResolver<'_, 'ra> { let field_def_ids = Vec::from_iter( self.resolver .cstore() - .associated_item_def_ids_untracked(def_id, self.sess), + .associated_item_def_ids_untracked(def_id, self.resolver.sess()), ); for field_def_id in field_def_ids { self.resolve_doc_links_extern_outer(field_def_id, scope_id); From 3bc2970a2e5f80f04ac2c1b1c1e4ec06a001f151 Mon Sep 17 00:00:00 2001 From: Jonathan Schwender Date: Thu, 5 Jan 2023 14:14:23 +0100 Subject: [PATCH 16/16] Improve linker-flavor detection Linker drivers such as gcc, clang or lld often have a version postfix, e.g clang-12. The previous logic would not account for this and would fall back to guessing the linker flavor to be the default linker flavor for the target, which causes linker errors when this is not the case. By accounting for the possible version postfix and also considering g++ and clang++, we considerably reduce the amount of times the fallback guess has to be used. To simplify matching check for a version postfix and match against the linker stem without any version postfix. In contrast to gcc, clang supports all architectures in one binary. This means there are no variants like `aarch64-linux-gnu-clang` and there is no need to check for `-clang` variants. --- compiler/rustc_codegen_ssa/src/back/link.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 8ca7103ed482..342abf81f6a7 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -1231,12 +1231,21 @@ pub fn linker_and_flavor(sess: &Session) -> (PathBuf, LinkerFlavor) { sess.emit_fatal(errors::LinkerFileStem); }); + // Remove any version postfix. + let stem = stem + .rsplit_once('-') + .and_then(|(lhs, rhs)| rhs.chars().all(char::is_numeric).then_some(lhs)) + .unwrap_or(stem); + + // GCC can have an optional target prefix. let flavor = if stem == "emcc" { LinkerFlavor::EmCc } else if stem == "gcc" || stem.ends_with("-gcc") + || stem == "g++" + || stem.ends_with("-g++") || stem == "clang" - || stem.ends_with("-clang") + || stem == "clang++" { LinkerFlavor::from_cli(LinkerFlavorCli::Gcc, &sess.target) } else if stem == "wasm-ld" || stem.ends_with("-wasm-ld") {