From 483ee1f14770343122e9fadfffc054a119587dbf Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 26 Jun 2022 06:19:32 -0500 Subject: [PATCH 01/13] Add a `-Zdump-drop-tracking-cfg` debugging flag This is useful for debugging drop-tracking; previously, you had to recompile rustc from source and manually add a call to `write_graph_to_file`. This makes the option more discoverable and configurable at runtime. I also took the liberty of making the labels for the CFG nodes much easier to read: previously, they looked like `id(2), local_id: 48`, now they look like ``` expr from_config (hir_id=HirId { owner: DefId(0:10 ~ default_struct_update[79f9]::foo), local_id: 2}) ``` --- compiler/rustc_interface/src/tests.rs | 1 + compiler/rustc_session/src/options.rs | 2 + .../drop_ranges/cfg_build.rs | 3 ++ .../drop_ranges/cfg_visualize.rs | 46 +++++++++++-------- 4 files changed, 33 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 30a29ed6ed38f..dd7ef360cc18b 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -649,6 +649,7 @@ fn test_debugging_options_tracking_hash() { untracked!(dlltool, Some(PathBuf::from("custom_dlltool.exe"))); untracked!(dont_buffer_diagnostics, true); untracked!(dump_dep_graph, true); + untracked!(dump_drop_tracking_cfg, Some("cfg.dot".to_string())); untracked!(dump_mir, Some(String::from("abc"))); untracked!(dump_mir_dataflow, true); untracked!(dump_mir_dir, String::from("abc")); diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 441e1f9f6a2b8..2638faa9d6a47 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -1245,6 +1245,8 @@ options! { dump_dep_graph: bool = (false, parse_bool, [UNTRACKED], "dump the dependency graph to $RUST_DEP_GRAPH (default: /tmp/dep_graph.gv) \ (default: no)"), + dump_drop_tracking_cfg: Option = (None, parse_opt_string, [UNTRACKED], + "dump drop-tracking control-flow graph as a `.dot` file (default: no)"), dump_mir: Option = (None, parse_opt_string, [UNTRACKED], "dump MIR state to file. `val` is used to select which passes and functions to dump. For example: diff --git a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_build.rs b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_build.rs index dbc309b29ff11..0af866359e847 100644 --- a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_build.rs +++ b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_build.rs @@ -33,6 +33,9 @@ pub(super) fn build_control_flow_graph<'tcx>( intravisit::walk_body(&mut drop_range_visitor, body); drop_range_visitor.drop_ranges.process_deferred_edges(); + if let Some(filename) = &tcx.sess.opts.debugging_opts.dump_drop_tracking_cfg { + super::cfg_visualize::write_graph_to_file(&drop_range_visitor.drop_ranges, filename, tcx); + } (drop_range_visitor.drop_ranges, drop_range_visitor.places.borrowed_temporaries) } diff --git a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_visualize.rs b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_visualize.rs index 20aad7aedf775..afca7f7b531f4 100644 --- a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_visualize.rs +++ b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_visualize.rs @@ -2,6 +2,7 @@ //! flow graph when needed for debugging. use rustc_graphviz as dot; +use rustc_middle::ty::TyCtxt; use super::{DropRangesBuilder, PostOrderId}; @@ -9,22 +10,35 @@ use super::{DropRangesBuilder, PostOrderId}; /// /// It is not normally called, but is kept around to easily add debugging /// code when needed. -#[allow(dead_code)] -pub(super) fn write_graph_to_file(drop_ranges: &DropRangesBuilder, filename: &str) { - dot::render(drop_ranges, &mut std::fs::File::create(filename).unwrap()).unwrap(); +pub(super) fn write_graph_to_file( + drop_ranges: &DropRangesBuilder, + filename: &str, + tcx: TyCtxt<'_>, +) { + dot::render( + &DropRangesGraph { drop_ranges, tcx }, + &mut std::fs::File::create(filename).unwrap(), + ) + .unwrap(); } -impl<'a> dot::GraphWalk<'a> for DropRangesBuilder { +struct DropRangesGraph<'a, 'tcx> { + drop_ranges: &'a DropRangesBuilder, + tcx: TyCtxt<'tcx>, +} + +impl<'a> dot::GraphWalk<'a> for DropRangesGraph<'_, '_> { type Node = PostOrderId; type Edge = (PostOrderId, PostOrderId); fn nodes(&'a self) -> dot::Nodes<'a, Self::Node> { - self.nodes.iter_enumerated().map(|(i, _)| i).collect() + self.drop_ranges.nodes.iter_enumerated().map(|(i, _)| i).collect() } fn edges(&'a self) -> dot::Edges<'a, Self::Edge> { - self.nodes + self.drop_ranges + .nodes .iter_enumerated() .flat_map(|(i, node)| { if node.successors.len() == 0 { @@ -45,7 +59,7 @@ impl<'a> dot::GraphWalk<'a> for DropRangesBuilder { } } -impl<'a> dot::Labeller<'a> for DropRangesBuilder { +impl<'a> dot::Labeller<'a> for DropRangesGraph<'_, '_> { type Node = PostOrderId; type Edge = (PostOrderId, PostOrderId); @@ -60,18 +74,12 @@ impl<'a> dot::Labeller<'a> for DropRangesBuilder { fn node_label(&'a self, n: &Self::Node) -> dot::LabelText<'a> { dot::LabelText::LabelStr( - format!( - "{:?}, local_id: {}", - n, - self.post_order_map - .iter() - .find(|(_hir_id, &post_order_id)| post_order_id == *n) - .map_or("".into(), |(hir_id, _)| format!( - "{}", - hir_id.local_id.index() - )) - ) - .into(), + self.drop_ranges + .post_order_map + .iter() + .find(|(_hir_id, &post_order_id)| post_order_id == *n) + .map_or("".into(), |(hir_id, _)| self.tcx.hir().node_to_string(*hir_id)) + .into(), ) } } From 9e76fcc468f57a1040a85cc195374923be414401 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 29 Jun 2022 10:43:47 +1000 Subject: [PATCH 02/13] Change `Search::infcx` to `tcx`. Because the `infcx` isn't needed. This removes one lifetime from `Search`. --- .../src/traits/structural_match.rs | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/structural_match.rs b/compiler/rustc_trait_selection/src/traits/structural_match.rs index bc2ce31df6d93..8db1d4cfbe8b8 100644 --- a/compiler/rustc_trait_selection/src/traits/structural_match.rs +++ b/compiler/rustc_trait_selection/src/traits/structural_match.rs @@ -60,7 +60,7 @@ pub fn search_for_structural_match_violation<'tcx>( ) -> Option> { // FIXME: we should instead pass in an `infcx` from the outside. tcx.infer_ctxt().enter(|infcx| { - ty.visit_with(&mut Search { infcx, span, seen: FxHashSet::default() }).break_value() + ty.visit_with(&mut Search { tcx: infcx.tcx, span, seen: FxHashSet::default() }).break_value() }) } @@ -114,27 +114,23 @@ fn type_marked_structural<'tcx>( /// This implements the traversal over the structure of a given type to try to /// find instances of ADTs (specifically structs or enums) that do not implement /// the structural-match traits (`StructuralPartialEq` and `StructuralEq`). -struct Search<'a, 'tcx> { +struct Search<'tcx> { span: Span, - infcx: InferCtxt<'a, 'tcx>, + tcx: TyCtxt<'tcx>, /// Tracks ADTs previously encountered during search, so that /// we will not recur on them again. seen: FxHashSet, } -impl<'a, 'tcx> Search<'a, 'tcx> { - fn tcx(&self) -> TyCtxt<'tcx> { - self.infcx.tcx - } - +impl<'tcx> Search<'tcx> { fn type_marked_structural(&self, adt_ty: Ty<'tcx>) -> bool { - adt_ty.is_structural_eq_shallow(self.tcx()) + adt_ty.is_structural_eq_shallow(self.tcx) } } -impl<'a, 'tcx> TypeVisitor<'tcx> for Search<'a, 'tcx> { +impl<'tcx> TypeVisitor<'tcx> for Search<'tcx> { type BreakTy = NonStructuralMatchTy<'tcx>; fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow { @@ -193,7 +189,7 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for Search<'a, 'tcx> { return ControlFlow::CONTINUE; } ty::Array(_, n) - if { n.try_eval_usize(self.tcx(), ty::ParamEnv::reveal_all()) == Some(0) } => + if { n.try_eval_usize(self.tcx, ty::ParamEnv::reveal_all()) == Some(0) } => { // rust-lang/rust#62336: ignore type of contents // for empty array. @@ -214,7 +210,7 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for Search<'a, 'tcx> { bug!("unexpected type during structural-match checking: {:?}", ty); } ty::Error(_) => { - self.tcx().sess.delay_span_bug(self.span, "ty::Error in structural-match check"); + self.tcx.sess.delay_span_bug(self.span, "ty::Error in structural-match check"); // We still want to check other types after encountering an error, // as this may still emit relevant errors. return ControlFlow::CONTINUE; @@ -244,9 +240,9 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for Search<'a, 'tcx> { // even though we skip super_visit_with, we must recur on // fields of ADT. - let tcx = self.tcx(); + let tcx = self.tcx; adt_def.all_fields().map(|field| field.ty(tcx, substs)).try_for_each(|field_ty| { - let ty = self.tcx().normalize_erasing_regions(ty::ParamEnv::empty(), field_ty); + let ty = self.tcx.normalize_erasing_regions(ty::ParamEnv::empty(), field_ty); debug!("structural-match ADT: field_ty={:?}, ty={:?}", field_ty, ty); ty.visit_with(self) }) From 687e391bc3a9682b6fe956a2e39a8a3935047158 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 29 Jun 2022 11:56:28 +1000 Subject: [PATCH 03/13] Avoid constructing an unnecessary `InferCtxt`. Currently, `search_for_structural_match_violation` constructs an `infcx` from a `tcx` and then only uses the `tcx` within the `infcx`. This is wasteful because `infcx` is a big type. This commit changes it to use the `tcx` directly. When compiling `pest-2.1.3`, this changes the memcpy stats reported by DHAT for a `check full` build from this: ``` 433,008,916 bytes (100%, 99,787.93/Minstr) in 2,148,668 blocks (100%, 495.17/Minstr), avg size 201.52 bytes ``` to this: ``` 101,422,347 bytes (99.98%, 25,243.59/Minstr) in 1,318,407 blocks (99.96%, 328.15/Minstr), avg size 76.93 bytes ``` This translates to a 4.3% reduction in instruction counts. --- .../rustc_trait_selection/src/traits/structural_match.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/structural_match.rs b/compiler/rustc_trait_selection/src/traits/structural_match.rs index 8db1d4cfbe8b8..725df69974c9c 100644 --- a/compiler/rustc_trait_selection/src/traits/structural_match.rs +++ b/compiler/rustc_trait_selection/src/traits/structural_match.rs @@ -58,10 +58,7 @@ pub fn search_for_structural_match_violation<'tcx>( tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, ) -> Option> { - // FIXME: we should instead pass in an `infcx` from the outside. - tcx.infer_ctxt().enter(|infcx| { - ty.visit_with(&mut Search { tcx: infcx.tcx, span, seen: FxHashSet::default() }).break_value() - }) + ty.visit_with(&mut Search { tcx, span, seen: FxHashSet::default() }).break_value() } /// This method returns true if and only if `adt_ty` itself has been marked as From 3164c2aa15a17ddbc4378b375ac822b6fef154b5 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 1 Jul 2022 00:11:39 -0500 Subject: [PATCH 04/13] Make logging for drop-tracking easier to read. Some of these are a little questionable because the output is so much longer, but I would really love to keep the bit that adds the pretty-printed expression to the generated CFG .dot file. Before: ``` DEBUG rustc_typeck::check::generator_interior::drop_ranges::record_consumed_borrow consume PlaceWithHirId { hir_id: HirId { owner: DefId(0:7 ~ default_struct_update[79f9]::foo), local_id: 15 }, place: Place { base_ty: impl std::future::Future, base: Rvalue, projections: [] } }; diag_expr_id=HirId { owner: DefId(0:7 ~ default_struct_update[79f9]::foo), local_id: 15 }, using parent expr HirId { owner: DefId(0:7 ~ default_struct_update[79f9]::foo), local_id: 49 } ``` After: ``` DEBUG rustc_typeck::check::generator_interior::drop_ranges::record_consumed_borrow consume PlaceWithHirId { hir_id: HirId { owner: DefId(0:7 ~ default_struct_update[79f9]::foo), local_id: 15 }, place: Place { base_ty: impl std::future::Future, base: Rvalue, projections: [] } }; diag_expr_id=expr from_config(Config { nickname: None, ..Default::default() }) (hir_id=HirId { owner: DefId(0:7 ~ default_struct_update[79f9]::foo), local_id: 15 }), using parent expr .await (hir_id=HirId { owner: DefId(0:7 ~ default_struct_update[79f9]::foo), local_id: 49 }) ``` --- .../check/generator_interior/drop_ranges.rs | 19 +++++++++++++++++-- .../drop_ranges/cfg_build.rs | 3 ++- .../drop_ranges/cfg_visualize.rs | 18 ++++++++++++------ .../drop_ranges/record_consumed_borrow.rs | 10 +++++++--- 4 files changed, 38 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges.rs b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges.rs index c2b4c478ba387..887c791af76c2 100644 --- a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges.rs +++ b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges.rs @@ -109,7 +109,7 @@ rustc_index::newtype_index! { } /// Identifies a value whose drop state we need to track. -#[derive(PartialEq, Eq, Hash, Debug, Clone, Copy)] +#[derive(PartialEq, Eq, Hash, Clone, Copy)] enum TrackedValue { /// Represents a named variable, such as a let binding, parameter, or upvar. /// @@ -121,6 +121,21 @@ enum TrackedValue { Temporary(HirId), } +impl Debug for TrackedValue { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + ty::tls::with_opt(|opt_tcx| { + if let Some(tcx) = opt_tcx { + write!(f, "{}", tcx.hir().node_to_string(self.hir_id())) + } else { + match self { + Self::Variable(hir_id) => write!(f, "Variable({:?})", hir_id), + Self::Temporary(hir_id) => write!(f, "Temporary({:?})", hir_id), + } + } + }) + } +} + impl TrackedValue { fn hir_id(&self) -> HirId { match self { @@ -148,7 +163,7 @@ enum TrackedValueConversionError { /// Place projects are not currently supported. /// /// The reasoning around these is kind of subtle, so we choose to be more - /// conservative around these for now. There is not reason in theory we + /// conservative around these for now. There is no reason in theory we /// cannot support these, we just have not implemented it yet. PlaceProjectionsNotSupported, } diff --git a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_build.rs b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_build.rs index 0af866359e847..5e363ef6f8444 100644 --- a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_build.rs +++ b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_build.rs @@ -129,13 +129,14 @@ impl<'a, 'tcx> DropRangeVisitor<'a, 'tcx> { /// ExprUseVisitor's consume callback doesn't go deep enough for our purposes in all /// expressions. This method consumes a little deeper into the expression when needed. fn consume_expr(&mut self, expr: &hir::Expr<'_>) { - debug!("consuming expr {:?}, count={:?}", expr.hir_id, self.expr_index); + debug!("consuming expr {:?}, count={:?}", expr.kind, self.expr_index); let places = self .places .consumed .get(&expr.hir_id) .map_or(vec![], |places| places.iter().cloned().collect()); for place in places { + trace!(?place, "consuming place"); for_each_consumable(self.hir, place, |value| self.record_drop(value)); } } diff --git a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_visualize.rs b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_visualize.rs index afca7f7b531f4..c0a0bfe8e1c00 100644 --- a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_visualize.rs +++ b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_visualize.rs @@ -74,12 +74,18 @@ impl<'a> dot::Labeller<'a> for DropRangesGraph<'_, '_> { fn node_label(&'a self, n: &Self::Node) -> dot::LabelText<'a> { dot::LabelText::LabelStr( - self.drop_ranges - .post_order_map - .iter() - .find(|(_hir_id, &post_order_id)| post_order_id == *n) - .map_or("".into(), |(hir_id, _)| self.tcx.hir().node_to_string(*hir_id)) - .into(), + format!( + "{n:?}: {}", + self.drop_ranges + .post_order_map + .iter() + .find(|(_hir_id, &post_order_id)| post_order_id == *n) + .map_or("".into(), |(hir_id, _)| self + .tcx + .hir() + .node_to_string(*hir_id)) + ) + .into(), ) } } diff --git a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs index b22b791f629d7..67cc46f21f00b 100644 --- a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs +++ b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs @@ -75,6 +75,7 @@ impl<'tcx> ExprUseDelegate<'tcx> { if !self.places.consumed.contains_key(&consumer) { self.places.consumed.insert(consumer, <_>::default()); } + debug!(?consumer, ?target, "mark_consumed"); self.places.consumed.get_mut(&consumer).map(|places| places.insert(target)); } @@ -136,13 +137,16 @@ impl<'tcx> expr_use_visitor::Delegate<'tcx> for ExprUseDelegate<'tcx> { place_with_id: &expr_use_visitor::PlaceWithHirId<'tcx>, diag_expr_id: HirId, ) { - let parent = match self.tcx.hir().find_parent_node(place_with_id.hir_id) { + let hir = self.tcx.hir(); + let parent = match hir.find_parent_node(place_with_id.hir_id) { Some(parent) => parent, None => place_with_id.hir_id, }; debug!( - "consume {:?}; diag_expr_id={:?}, using parent {:?}", - place_with_id, diag_expr_id, parent + "consume {:?}; diag_expr_id={}, using parent {}", + place_with_id, + hir.node_to_string(diag_expr_id), + hir.node_to_string(parent) ); place_with_id .try_into() From 34d6f08f4d320efbefbe121319eed3928874e039 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 28 Jun 2022 21:03:14 -0700 Subject: [PATCH 05/13] Use dashes instead of underscores in fluent names --- .../rustc_error_messages/locales/en-US/builtin_macros.ftl | 4 ++-- compiler/rustc_macros/src/diagnostics/fluent.rs | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_error_messages/locales/en-US/builtin_macros.ftl b/compiler/rustc_error_messages/locales/en-US/builtin_macros.ftl index 4a42d52f71004..1d3e33c81851f 100644 --- a/compiler/rustc_error_messages/locales/en-US/builtin_macros.ftl +++ b/compiler/rustc_error_messages/locales/en-US/builtin_macros.ftl @@ -1,5 +1,5 @@ -builtin_macros-requires-cfg-pattern = +builtin-macros-requires-cfg-pattern = macro requires a cfg-pattern as an argument .label = cfg-pattern required -builtin_macros-expected-one-cfg-pattern = expected 1 cfg-pattern +builtin-macros-expected-one-cfg-pattern = expected 1 cfg-pattern diff --git a/compiler/rustc_macros/src/diagnostics/fluent.rs b/compiler/rustc_macros/src/diagnostics/fluent.rs index 2317186e65502..2758fcd1310fe 100644 --- a/compiler/rustc_macros/src/diagnostics/fluent.rs +++ b/compiler/rustc_macros/src/diagnostics/fluent.rs @@ -189,9 +189,13 @@ pub(crate) fn fluent_messages(input: proc_macro::TokenStream) -> proc_macro::Tok if let Entry::Message(Message { id: Identifier { name }, attributes, .. }) = entry { let _ = previous_defns.entry(name.to_string()).or_insert(ident_span); - // `typeck-foo-bar` => `foo_bar` + // `typeck-foo-bar` => `foo_bar` (in `typeck.ftl`) + // `const-eval-baz` => `baz` (in `const_eval.ftl`) let snake_name = Ident::new( - &name.replace(&format!("{}-", res.ident), "").replace("-", "_"), + // FIXME: should probably trim prefix, not replace all occurrences + &name + .replace(&format!("{}-", res.ident).replace("_", "-"), "") + .replace("-", "_"), span, ); constants.extend(quote! { From 934079fd9e3df668895ed6d64db7221d03f3b027 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 28 Jun 2022 21:26:05 -0700 Subject: [PATCH 06/13] Migrate unstable-in-stable diagnostic --- compiler/rustc_const_eval/src/errors.rs | 21 +++++++++++++++++ compiler/rustc_const_eval/src/lib.rs | 1 + .../src/transform/check_consts/check.rs | 23 +++---------------- .../locales/en-US/const_eval.ftl | 4 ++++ compiler/rustc_error_messages/src/lib.rs | 1 + 5 files changed, 30 insertions(+), 20 deletions(-) create mode 100644 compiler/rustc_const_eval/src/errors.rs create mode 100644 compiler/rustc_error_messages/locales/en-US/const_eval.ftl diff --git a/compiler/rustc_const_eval/src/errors.rs b/compiler/rustc_const_eval/src/errors.rs new file mode 100644 index 0000000000000..f19e8046d422d --- /dev/null +++ b/compiler/rustc_const_eval/src/errors.rs @@ -0,0 +1,21 @@ +use rustc_macros::SessionDiagnostic; +use rustc_span::Span; + +#[derive(SessionDiagnostic)] +#[error(const_eval::unstable_in_stable)] +pub(crate) struct UnstableInStable { + pub gate: String, + #[primary_span] + pub span: Span, + #[suggestion( + const_eval::unstable_sugg, + code = "#[rustc_const_unstable(feature = \"...\", issue = \"...\")]\n", + applicability = "has-placeholders" + )] + #[suggestion( + const_eval::bypass_sugg, + code = "#[rustc_allow_const_fn_unstable({gate})]\n", + applicability = "has-placeholders" + )] + pub attr_span: Span, +} diff --git a/compiler/rustc_const_eval/src/lib.rs b/compiler/rustc_const_eval/src/lib.rs index 2d42ae236ad9d..d65d4f7eb720e 100644 --- a/compiler/rustc_const_eval/src/lib.rs +++ b/compiler/rustc_const_eval/src/lib.rs @@ -31,6 +31,7 @@ extern crate tracing; extern crate rustc_middle; pub mod const_eval; +mod errors; pub mod interpret; pub mod transform; pub mod util; diff --git a/compiler/rustc_const_eval/src/transform/check_consts/check.rs b/compiler/rustc_const_eval/src/transform/check_consts/check.rs index f87de4f6a08a4..3dcd96df33cf5 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/check.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/check.rs @@ -1,6 +1,6 @@ //! The `Visitor` responsible for actually checking a `mir::Body` for invalid operations. -use rustc_errors::{Applicability, Diagnostic, ErrorGuaranteed}; +use rustc_errors::{Diagnostic, ErrorGuaranteed}; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_index::bit_set::BitSet; @@ -24,6 +24,7 @@ use super::qualifs::{self, CustomEq, HasMutInterior, NeedsDrop, NeedsNonConstDro use super::resolver::FlowSensitiveAnalysis; use super::{ConstCx, Qualif}; use crate::const_eval::is_unstable_const_fn; +use crate::errors::UnstableInStable; type QualifResults<'mir, 'tcx, Q> = rustc_mir_dataflow::ResultsCursor<'mir, 'tcx, FlowSensitiveAnalysis<'mir, 'mir, 'tcx, Q>>; @@ -1026,23 +1027,5 @@ fn is_int_bool_or_char(ty: Ty<'_>) -> bool { fn emit_unstable_in_stable_error(ccx: &ConstCx<'_, '_>, span: Span, gate: Symbol) { let attr_span = ccx.tcx.def_span(ccx.def_id()).shrink_to_lo(); - ccx.tcx - .sess - .struct_span_err( - span, - &format!("const-stable function cannot use `#[feature({})]`", gate.as_str()), - ) - .span_suggestion( - attr_span, - "if it is not part of the public API, make this function unstably const", - concat!(r#"#[rustc_const_unstable(feature = "...", issue = "...")]"#, '\n'), - Applicability::HasPlaceholders, - ) - .span_suggestion( - attr_span, - "otherwise `#[rustc_allow_const_fn_unstable]` can be used to bypass stability checks", - format!("#[rustc_allow_const_fn_unstable({})]\n", gate), - Applicability::MaybeIncorrect, - ) - .emit(); + ccx.tcx.sess.emit_err(UnstableInStable { gate: gate.to_string(), span, attr_span }); } diff --git a/compiler/rustc_error_messages/locales/en-US/const_eval.ftl b/compiler/rustc_error_messages/locales/en-US/const_eval.ftl new file mode 100644 index 0000000000000..a022e596efb7b --- /dev/null +++ b/compiler/rustc_error_messages/locales/en-US/const_eval.ftl @@ -0,0 +1,4 @@ +const-eval-unstable-in-stable = + const-stable function cannot use `#[feature({$gate})]` + .unstable-sugg = if it is not part of the public API, make this function unstably const + .bypass-sugg = otherwise `#[rustc_allow_const_fn_unstable]` can be used to bypass stability checks diff --git a/compiler/rustc_error_messages/src/lib.rs b/compiler/rustc_error_messages/src/lib.rs index 563d0534d8f38..5a482bc5b2c39 100644 --- a/compiler/rustc_error_messages/src/lib.rs +++ b/compiler/rustc_error_messages/src/lib.rs @@ -37,6 +37,7 @@ fluent_messages! { parser => "../locales/en-US/parser.ftl", privacy => "../locales/en-US/privacy.ftl", typeck => "../locales/en-US/typeck.ftl", + const_eval => "../locales/en-US/const_eval.ftl", } pub use fluent_generated::{self as fluent, DEFAULT_LOCALE_RESOURCES}; From 1c4afbd1de7a0aa548a4b8f184aede5c5954e63a Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 28 Jun 2022 21:46:01 -0700 Subject: [PATCH 07/13] Migrate NonConstOp diagnostic --- compiler/rustc_const_eval/src/errors.rs | 7 +++++++ .../rustc_const_eval/src/transform/check_consts/ops.rs | 9 ++------- .../rustc_error_messages/locales/en-US/const_eval.ftl | 3 +++ 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_const_eval/src/errors.rs b/compiler/rustc_const_eval/src/errors.rs index f19e8046d422d..40398011f1f77 100644 --- a/compiler/rustc_const_eval/src/errors.rs +++ b/compiler/rustc_const_eval/src/errors.rs @@ -19,3 +19,10 @@ pub(crate) struct UnstableInStable { )] pub attr_span: Span, } + +#[derive(SessionDiagnostic)] +#[error(const_eval::thread_local_access, code = "E0625")] +pub(crate) struct NonConstOpErr { + #[primary_span] + pub span: Span, +} diff --git a/compiler/rustc_const_eval/src/transform/check_consts/ops.rs b/compiler/rustc_const_eval/src/transform/check_consts/ops.rs index 9574661282ba1..01080560021cc 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/ops.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/ops.rs @@ -20,6 +20,7 @@ use rustc_span::{BytePos, Pos, Span, Symbol}; use rustc_trait_selection::traits::SelectionContext; use super::ConstCx; +use crate::errors::NonConstOpErr; use crate::util::{call_kind, CallDesugaringKind, CallKind}; #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -760,13 +761,7 @@ impl<'tcx> NonConstOp<'tcx> for ThreadLocalAccess { ccx: &ConstCx<'_, 'tcx>, span: Span, ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { - struct_span_err!( - ccx.tcx.sess, - span, - E0625, - "thread-local statics cannot be \ - accessed at compile-time" - ) + ccx.tcx.sess.create_err(NonConstOpErr { span }) } } diff --git a/compiler/rustc_error_messages/locales/en-US/const_eval.ftl b/compiler/rustc_error_messages/locales/en-US/const_eval.ftl index a022e596efb7b..897ddce7e99ba 100644 --- a/compiler/rustc_error_messages/locales/en-US/const_eval.ftl +++ b/compiler/rustc_error_messages/locales/en-US/const_eval.ftl @@ -2,3 +2,6 @@ const-eval-unstable-in-stable = const-stable function cannot use `#[feature({$gate})]` .unstable-sugg = if it is not part of the public API, make this function unstably const .bypass-sugg = otherwise `#[rustc_allow_const_fn_unstable]` can be used to bypass stability checks + +const-eval-thread-local-access = + thread-local statics cannot be accessed at compile-time \ No newline at end of file From ff9fd36aa4ed90051424560268108370bbb3749b Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 28 Jun 2022 22:09:59 -0700 Subject: [PATCH 08/13] Implement IntoDiagnosticArg for hir::ConstContext --- Cargo.lock | 1 + compiler/rustc_errors/Cargo.toml | 1 + compiler/rustc_errors/src/diagnostic.rs | 11 +++++++++++ compiler/rustc_hir/src/hir.rs | 3 +++ 4 files changed, 16 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 86936b25d8ad9..268af59aa891c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3813,6 +3813,7 @@ dependencies = [ "atty", "rustc_data_structures", "rustc_error_messages", + "rustc_hir", "rustc_lint_defs", "rustc_macros", "rustc_serialize", diff --git a/compiler/rustc_errors/Cargo.toml b/compiler/rustc_errors/Cargo.toml index 8955762605713..7d7e92c522922 100644 --- a/compiler/rustc_errors/Cargo.toml +++ b/compiler/rustc_errors/Cargo.toml @@ -13,6 +13,7 @@ rustc_serialize = { path = "../rustc_serialize" } rustc_span = { path = "../rustc_span" } rustc_macros = { path = "../rustc_macros" } rustc_data_structures = { path = "../rustc_data_structures" } +rustc_hir = { path = "../rustc_hir" } rustc_lint_defs = { path = "../rustc_lint_defs" } unicode-width = "0.1.4" atty = "0.2" diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 0d1d017d87458..9429ad1a89745 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -5,6 +5,7 @@ use crate::{ }; use rustc_data_structures::stable_map::FxHashMap; use rustc_error_messages::FluentValue; +use rustc_hir as hir; use rustc_lint_defs::{Applicability, LintExpectationId}; use rustc_span::edition::LATEST_STABLE_EDITION; use rustc_span::symbol::{Ident, Symbol}; @@ -160,6 +161,16 @@ impl<'source> Into> for DiagnosticArgValue<'source> { } } +impl IntoDiagnosticArg for hir::ConstContext { + fn into_diagnostic_arg(self) -> DiagnosticArgValue<'static> { + DiagnosticArgValue::Str(Cow::Borrowed(match self { + hir::ConstContext::ConstFn => "constant function", + hir::ConstContext::Static(_) => "static", + hir::ConstContext::Const => "constant", + })) + } +} + /// Trait implemented by error types. This should not be implemented manually. Instead, use /// `#[derive(SessionSubdiagnostic)]` -- see [rustc_macros::SessionSubdiagnostic]. #[rustc_diagnostic_item = "AddSubdiagnostic"] diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index a2ef158ce8d32..acd77f5d2ee91 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -1595,6 +1595,9 @@ impl fmt::Display for ConstContext { } } +// NOTE: `IntoDiagnosticArg` impl for `ConstContext` lives in `rustc_errors` +// due to a cyclical dependency between hir that crate. + /// A literal. pub type Lit = Spanned; From c48f4828131d7d326b8a4a91ae9931736deda26f Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 28 Jun 2022 22:32:32 -0700 Subject: [PATCH 09/13] Migrate StaticAccess diagnostic --- compiler/rustc_const_eval/src/errors.rs | 13 +++++++++ .../src/transform/check_consts/ops.rs | 27 ++++++------------- .../locales/en-US/const_eval.ftl | 12 ++++++++- 3 files changed, 32 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_const_eval/src/errors.rs b/compiler/rustc_const_eval/src/errors.rs index 40398011f1f77..3a498f84ec58e 100644 --- a/compiler/rustc_const_eval/src/errors.rs +++ b/compiler/rustc_const_eval/src/errors.rs @@ -1,3 +1,4 @@ +use rustc_hir::ConstContext; use rustc_macros::SessionDiagnostic; use rustc_span::Span; @@ -26,3 +27,15 @@ pub(crate) struct NonConstOpErr { #[primary_span] pub span: Span, } + +#[derive(SessionDiagnostic)] +#[error(const_eval::static_access, code = "E0013")] +#[help] +pub(crate) struct StaticAccessErr { + #[primary_span] + pub span: Span, + pub kind: ConstContext, + #[note(const_eval::teach_note)] + #[help(const_eval::teach_help)] + pub teach: Option<()>, +} diff --git a/compiler/rustc_const_eval/src/transform/check_consts/ops.rs b/compiler/rustc_const_eval/src/transform/check_consts/ops.rs index 01080560021cc..d104fdd59c5ee 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/ops.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/ops.rs @@ -1,7 +1,9 @@ //! Concrete error types for all operations which may be invalid in a certain const context. use hir::def_id::LocalDefId; -use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder, ErrorGuaranteed}; +use rustc_errors::{ + error_code, struct_span_err, Applicability, DiagnosticBuilder, ErrorGuaranteed, +}; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_infer::infer::TyCtxtInferExt; @@ -20,7 +22,7 @@ use rustc_span::{BytePos, Pos, Span, Symbol}; use rustc_trait_selection::traits::SelectionContext; use super::ConstCx; -use crate::errors::NonConstOpErr; +use crate::errors::{NonConstOpErr, StaticAccessErr}; use crate::util::{call_kind, CallDesugaringKind, CallKind}; #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -731,24 +733,11 @@ impl<'tcx> NonConstOp<'tcx> for StaticAccess { ccx: &ConstCx<'_, 'tcx>, span: Span, ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { - let mut err = struct_span_err!( - ccx.tcx.sess, + ccx.tcx.sess.create_err(StaticAccessErr { span, - E0013, - "{}s cannot refer to statics", - ccx.const_kind() - ); - err.help( - "consider extracting the value of the `static` to a `const`, and referring to that", - ); - if ccx.tcx.sess.teach(&err.get_code().unwrap()) { - err.note( - "`static` and `const` variables can refer to other `const` variables. \ - A `const` variable, however, cannot refer to a `static` variable.", - ); - err.help("To fix this, the value can be extracted to a `const` and then used."); - } - err + kind: ccx.const_kind(), + teach: ccx.tcx.sess.teach(&error_code!(E0013)).then_some(()), + }) } } diff --git a/compiler/rustc_error_messages/locales/en-US/const_eval.ftl b/compiler/rustc_error_messages/locales/en-US/const_eval.ftl index 897ddce7e99ba..66058aa1769ed 100644 --- a/compiler/rustc_error_messages/locales/en-US/const_eval.ftl +++ b/compiler/rustc_error_messages/locales/en-US/const_eval.ftl @@ -4,4 +4,14 @@ const-eval-unstable-in-stable = .bypass-sugg = otherwise `#[rustc_allow_const_fn_unstable]` can be used to bypass stability checks const-eval-thread-local-access = - thread-local statics cannot be accessed at compile-time \ No newline at end of file + thread-local statics cannot be accessed at compile-time + +const-eval-static-access = + { $kind -> + [constant function] constant functions + [static] statics + *[constant] constants + } cannot refer to statics + .help = consider extracting the value of the `static` to a `const`, and referring to that + .teach-note = `static` and `const` variables can refer to other `const` variables. A `const` variable, however, cannot refer to a `static` variable. + .teach-help = To fix this, the value can be extracted to a `const` and then used. From 584e5d4c4ff1e24c7cb32e2a7526b4f629728daf Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 28 Jun 2022 22:46:25 -0700 Subject: [PATCH 10/13] Migrate PanicNonStr, RawPtrComparison, RawPtrToInt diagnostics --- compiler/rustc_const_eval/src/errors.rs | 24 +++++++++++++++ .../src/transform/check_consts/ops.rs | 29 ++++--------------- .../locales/en-US/const_eval.ftl | 11 +++++++ 3 files changed, 41 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_const_eval/src/errors.rs b/compiler/rustc_const_eval/src/errors.rs index 3a498f84ec58e..20382a81f14df 100644 --- a/compiler/rustc_const_eval/src/errors.rs +++ b/compiler/rustc_const_eval/src/errors.rs @@ -39,3 +39,27 @@ pub(crate) struct StaticAccessErr { #[help(const_eval::teach_help)] pub teach: Option<()>, } + +#[derive(SessionDiagnostic)] +#[error(const_eval::raw_ptr_to_int)] +#[note] +#[note(const_eval::note2)] +pub(crate) struct RawPtrToIntErr { + #[primary_span] + pub span: Span, +} + +#[derive(SessionDiagnostic)] +#[error(const_eval::raw_ptr_comparison)] +#[note] +pub(crate) struct RawPtrComparisonErr { + #[primary_span] + pub span: Span, +} + +#[derive(SessionDiagnostic)] +#[error(const_eval::panic_non_str)] +pub(crate) struct PanicNonStrErr { + #[primary_span] + pub span: Span, +} diff --git a/compiler/rustc_const_eval/src/transform/check_consts/ops.rs b/compiler/rustc_const_eval/src/transform/check_consts/ops.rs index d104fdd59c5ee..181dbacdc820e 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/ops.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/ops.rs @@ -22,7 +22,9 @@ use rustc_span::{BytePos, Pos, Span, Symbol}; use rustc_trait_selection::traits::SelectionContext; use super::ConstCx; -use crate::errors::{NonConstOpErr, StaticAccessErr}; +use crate::errors::{ + NonConstOpErr, PanicNonStrErr, RawPtrComparisonErr, RawPtrToIntErr, StaticAccessErr, +}; use crate::util::{call_kind, CallDesugaringKind, CallKind}; #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -642,10 +644,7 @@ impl<'tcx> NonConstOp<'tcx> for PanicNonStr { ccx: &ConstCx<'_, 'tcx>, span: Span, ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { - ccx.tcx.sess.struct_span_err( - span, - "argument to `panic!()` in a const context must have type `&str`", - ) + ccx.tcx.sess.create_err(PanicNonStrErr { span }) } } @@ -660,15 +659,7 @@ impl<'tcx> NonConstOp<'tcx> for RawPtrComparison { ccx: &ConstCx<'_, 'tcx>, span: Span, ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { - let mut err = ccx - .tcx - .sess - .struct_span_err(span, "pointers cannot be reliably compared during const eval"); - err.note( - "see issue #53020 \ - for more information", - ); - err + ccx.tcx.sess.create_err(RawPtrComparisonErr { span }) } } @@ -704,15 +695,7 @@ impl<'tcx> NonConstOp<'tcx> for RawPtrToIntCast { ccx: &ConstCx<'_, 'tcx>, span: Span, ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { - let mut err = ccx - .tcx - .sess - .struct_span_err(span, "pointers cannot be cast to integers during const eval"); - err.note("at compile-time, pointers do not have an integer value"); - err.note( - "avoiding this restriction via `transmute`, `union`, or raw pointers leads to compile-time undefined behavior", - ); - err + ccx.tcx.sess.create_err(RawPtrToIntErr { span }) } } diff --git a/compiler/rustc_error_messages/locales/en-US/const_eval.ftl b/compiler/rustc_error_messages/locales/en-US/const_eval.ftl index 66058aa1769ed..30de7d5a24f50 100644 --- a/compiler/rustc_error_messages/locales/en-US/const_eval.ftl +++ b/compiler/rustc_error_messages/locales/en-US/const_eval.ftl @@ -15,3 +15,14 @@ const-eval-static-access = .help = consider extracting the value of the `static` to a `const`, and referring to that .teach-note = `static` and `const` variables can refer to other `const` variables. A `const` variable, however, cannot refer to a `static` variable. .teach-help = To fix this, the value can be extracted to a `const` and then used. + +const-eval-raw-ptr-to-int = + pointers cannot be cast to integers during const eval + .note = at compile-time, pointers do not have an integer value + .note2 = avoiding this restriction via `transmute`, `union`, or raw pointers leads to compile-time undefined behavior + +const-eval-raw-ptr-comparison = + pointers cannot be reliably compared during const eval + .note = see issue #53020 for more information + +const-eval-panic-non-str = argument to `panic!()` in a const context must have type `&str` From f97f2a47ffd1e232f5b6dec17118fcbadef90692 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 28 Jun 2022 23:22:15 -0700 Subject: [PATCH 11/13] Migrate MutDeref, TransientMutBorrow diagnostics --- compiler/rustc_const_eval/src/errors.rs | 24 +++++++++++++ .../src/transform/check_consts/ops.rs | 34 +++++++++---------- .../locales/en-US/const_eval.ftl | 19 +++++++++++ compiler/rustc_session/src/session.rs | 11 +++++- 4 files changed, 69 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_const_eval/src/errors.rs b/compiler/rustc_const_eval/src/errors.rs index 20382a81f14df..a463fe7b970f4 100644 --- a/compiler/rustc_const_eval/src/errors.rs +++ b/compiler/rustc_const_eval/src/errors.rs @@ -63,3 +63,27 @@ pub(crate) struct PanicNonStrErr { #[primary_span] pub span: Span, } + +#[derive(SessionDiagnostic)] +#[error(const_eval::mut_deref, code = "E0658")] +pub(crate) struct MutDerefErr { + #[primary_span] + pub span: Span, + pub kind: ConstContext, +} + +#[derive(SessionDiagnostic)] +#[error(const_eval::transient_mut_borrow, code = "E0658")] +pub(crate) struct TransientMutBorrowErr { + #[primary_span] + pub span: Span, + pub kind: ConstContext, +} + +#[derive(SessionDiagnostic)] +#[error(const_eval::transient_mut_borrow_raw, code = "E0658")] +pub(crate) struct TransientMutBorrowErrRaw { + #[primary_span] + pub span: Span, + pub kind: ConstContext, +} diff --git a/compiler/rustc_const_eval/src/transform/check_consts/ops.rs b/compiler/rustc_const_eval/src/transform/check_consts/ops.rs index 181dbacdc820e..17376e59e09cc 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/ops.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/ops.rs @@ -23,7 +23,8 @@ use rustc_trait_selection::traits::SelectionContext; use super::ConstCx; use crate::errors::{ - NonConstOpErr, PanicNonStrErr, RawPtrComparisonErr, RawPtrToIntErr, StaticAccessErr, + MutDerefErr, NonConstOpErr, PanicNonStrErr, RawPtrComparisonErr, RawPtrToIntErr, + StaticAccessErr, TransientMutBorrowErr, TransientMutBorrowErrRaw, }; use crate::util::{call_kind, CallDesugaringKind, CallKind}; @@ -595,17 +596,17 @@ impl<'tcx> NonConstOp<'tcx> for TransientMutBorrow { ccx: &ConstCx<'_, 'tcx>, span: Span, ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { - let raw = match self.0 { - hir::BorrowKind::Raw => "raw ", - hir::BorrowKind::Ref => "", - }; - - feature_err( - &ccx.tcx.sess.parse_sess, - sym::const_mut_refs, - span, - &format!("{}mutable references are not allowed in {}s", raw, ccx.const_kind()), - ) + let kind = ccx.const_kind(); + match self.0 { + hir::BorrowKind::Raw => ccx + .tcx + .sess + .create_feature_err(TransientMutBorrowErrRaw { span, kind }, sym::const_mut_refs), + hir::BorrowKind::Ref => ccx + .tcx + .sess + .create_feature_err(TransientMutBorrowErr { span, kind }, sym::const_mut_refs), + } } } @@ -626,12 +627,9 @@ impl<'tcx> NonConstOp<'tcx> for MutDeref { ccx: &ConstCx<'_, 'tcx>, span: Span, ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { - feature_err( - &ccx.tcx.sess.parse_sess, - sym::const_mut_refs, - span, - &format!("mutation through a reference is not allowed in {}s", ccx.const_kind()), - ) + ccx.tcx + .sess + .create_feature_err(MutDerefErr { span, kind: ccx.const_kind() }, sym::const_mut_refs) } } diff --git a/compiler/rustc_error_messages/locales/en-US/const_eval.ftl b/compiler/rustc_error_messages/locales/en-US/const_eval.ftl index 30de7d5a24f50..f1a8c8c47ccc3 100644 --- a/compiler/rustc_error_messages/locales/en-US/const_eval.ftl +++ b/compiler/rustc_error_messages/locales/en-US/const_eval.ftl @@ -26,3 +26,22 @@ const-eval-raw-ptr-comparison = .note = see issue #53020 for more information const-eval-panic-non-str = argument to `panic!()` in a const context must have type `&str` + +const-eval-mut-deref = + mutation through a reference is not allowed in { $kind -> + [constant function] constant functions + [static] statics + *[constant] constants + } + +const-eval-transient-mut-borrow = mutable references are not allowed in { $kind -> + [constant function] constant functions + [static] statics + *[constant] constants + } + +const-eval-transient-mut-borrow-raw = raw mutable references are not allowed in { $kind -> + [constant function] constant functions + [static] statics + *[constant] constants + } diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 2a5ddd4e9e420..edce70d19cc4c 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -2,7 +2,7 @@ use crate::cgu_reuse_tracker::CguReuseTracker; use crate::code_stats::CodeStats; pub use crate::code_stats::{DataTypeKind, FieldInfo, SizeKind, VariantInfo}; use crate::config::{self, CrateType, OutputType, SwitchWithOptPath}; -use crate::parse::ParseSess; +use crate::parse::{add_feature_diagnostics, ParseSess}; use crate::search_paths::{PathKind, SearchPath}; use crate::{filesearch, lint}; @@ -458,6 +458,15 @@ impl Session { ) -> DiagnosticBuilder<'a, ErrorGuaranteed> { self.parse_sess.create_err(err) } + pub fn create_feature_err<'a>( + &'a self, + err: impl SessionDiagnostic<'a>, + feature: Symbol, + ) -> DiagnosticBuilder<'a, ErrorGuaranteed> { + let mut err = self.parse_sess.create_err(err); + add_feature_diagnostics(&mut err, &self.parse_sess, feature); + err + } pub fn emit_err<'a>(&'a self, err: impl SessionDiagnostic<'a>) -> ErrorGuaranteed { self.parse_sess.emit_err(err) } From 20583337800524516ab814e1cff080c0ca9f200b Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 8 Jul 2022 03:46:18 +0000 Subject: [PATCH 12/13] simplify plurals in fluent messages using hir::ConstContext --- .../locales/en-US/const_eval.ftl | 28 ++++--------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_error_messages/locales/en-US/const_eval.ftl b/compiler/rustc_error_messages/locales/en-US/const_eval.ftl index f1a8c8c47ccc3..3f2ff86100160 100644 --- a/compiler/rustc_error_messages/locales/en-US/const_eval.ftl +++ b/compiler/rustc_error_messages/locales/en-US/const_eval.ftl @@ -7,11 +7,7 @@ const-eval-thread-local-access = thread-local statics cannot be accessed at compile-time const-eval-static-access = - { $kind -> - [constant function] constant functions - [static] statics - *[constant] constants - } cannot refer to statics + {$kind}s cannot refer to statics .help = consider extracting the value of the `static` to a `const`, and referring to that .teach-note = `static` and `const` variables can refer to other `const` variables. A `const` variable, however, cannot refer to a `static` variable. .teach-help = To fix this, the value can be extracted to a `const` and then used. @@ -28,20 +24,8 @@ const-eval-raw-ptr-comparison = const-eval-panic-non-str = argument to `panic!()` in a const context must have type `&str` const-eval-mut-deref = - mutation through a reference is not allowed in { $kind -> - [constant function] constant functions - [static] statics - *[constant] constants - } - -const-eval-transient-mut-borrow = mutable references are not allowed in { $kind -> - [constant function] constant functions - [static] statics - *[constant] constants - } - -const-eval-transient-mut-borrow-raw = raw mutable references are not allowed in { $kind -> - [constant function] constant functions - [static] statics - *[constant] constants - } + mutation through a reference is not allowed in {$kind}s + +const-eval-transient-mut-borrow = mutable references are not allowed in {$kind}s + +const-eval-transient-mut-borrow-raw = raw mutable references are not allowed in {$kind}s From 08135254dcf22be0d5661ea8f75e703b29a83514 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 2 Jul 2022 01:30:07 +0000 Subject: [PATCH 13/13] Highlight conflicting param-env candidates --- compiler/rustc_middle/src/traits/mod.rs | 10 ++++- .../src/traits/error_reporting/mod.rs | 41 +++++++++++++----- .../src/traits/select/candidate_assembly.rs | 43 ++++++++++++++++++- src/test/ui/issues/issue-21974.stderr | 8 +++- src/test/ui/issues/issue-24424.stderr | 6 ++- src/test/ui/lifetimes/issue-34979.stderr | 8 +++- src/test/ui/traits/issue-85735.stderr | 9 +++- .../ui/type/type-check/issue-40294.stderr | 8 +++- 8 files changed, 114 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index eee44df8645ef..fe7f72024d358 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -546,9 +546,17 @@ pub enum SelectionError<'tcx> { ErrorReporting, /// Multiple applicable `impl`s where found. The `DefId`s correspond to /// all the `impl`s' Items. - Ambiguous(Vec), + Ambiguous(Vec), } +#[derive(Copy, Clone, Debug)] +pub enum AmbiguousSelection { + Impl(DefId), + ParamEnv(Span), +} + +TrivialTypeTraversalAndLiftImpls! { AmbiguousSelection, } + /// When performing resolution, it is typically the case that there /// can be one of three outcomes: /// diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs index 8efefd476abc9..aa1c91362891b 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -23,7 +23,7 @@ use rustc_hir::GenericParam; use rustc_hir::Item; use rustc_hir::Node; use rustc_infer::infer::error_reporting::same_type_modulo_infer; -use rustc_infer::traits::TraitEngine; +use rustc_infer::traits::{AmbiguousSelection, TraitEngine}; use rustc_middle::thir::abstract_const::NotConstEvaluatable; use rustc_middle::traits::select::OverflowError; use rustc_middle::ty::error::ExpectedFound; @@ -1404,7 +1404,7 @@ trait InferCtxtPrivExt<'hir, 'tcx> { fn annotate_source_of_ambiguity( &self, err: &mut Diagnostic, - impls: &[DefId], + impls: &[AmbiguousSelection], predicate: ty::Predicate<'tcx>, ); @@ -2020,6 +2020,14 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> { ); match selcx.select_from_obligation(&obligation) { Err(SelectionError::Ambiguous(impls)) if impls.len() > 1 => { + if self.is_tainted_by_errors() && subst.is_none() { + // If `subst.is_none()`, then this is probably two param-env + // candidates or impl candidates that are equal modulo lifetimes. + // Therefore, if we've already emitted an error, just skip this + // one, since it's not particularly actionable. + err.cancel(); + return; + } self.annotate_source_of_ambiguity(&mut err, &impls, predicate); } _ => { @@ -2170,24 +2178,35 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> { fn annotate_source_of_ambiguity( &self, err: &mut Diagnostic, - impls: &[DefId], + impls: &[AmbiguousSelection], predicate: ty::Predicate<'tcx>, ) { let mut spans = vec![]; let mut crates = vec![]; let mut post = vec![]; - for def_id in impls { - match self.tcx.span_of_impl(*def_id) { - Ok(span) => spans.push(span), - Err(name) => { - crates.push(name); - if let Some(header) = to_pretty_impl_header(self.tcx, *def_id) { - post.push(header); + let mut or_where_clause = false; + for ambig in impls { + match ambig { + AmbiguousSelection::Impl(def_id) => match self.tcx.span_of_impl(*def_id) { + Ok(span) => spans.push(span), + Err(name) => { + crates.push(name); + if let Some(header) = to_pretty_impl_header(self.tcx, *def_id) { + post.push(header); + } } + }, + AmbiguousSelection::ParamEnv(span) => { + or_where_clause = true; + spans.push(*span); } } } - let msg = format!("multiple `impl`s satisfying `{}` found", predicate); + let msg = format!( + "multiple `impl`s{} satisfying `{}` found", + if or_where_clause { " or `where` clauses" } else { "" }, + predicate + ); let mut crate_names: Vec<_> = crates.iter().map(|n| format!("`{}`", n)).collect(); crate_names.sort(); crate_names.dedup(); diff --git a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs index 0f7af41cfe3ff..21e14eae0ee27 100644 --- a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs +++ b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs @@ -6,9 +6,11 @@ //! //! [rustc dev guide]:https://rustc-dev-guide.rust-lang.org/traits/resolution.html#candidate-assembly use hir::LangItem; +use rustc_data_structures::fx::FxHashMap; use rustc_hir as hir; use rustc_hir::def_id::DefId; -use rustc_infer::traits::TraitEngine; +use rustc_infer::traits::util::elaborate_predicates_with_span; +use rustc_infer::traits::{AmbiguousSelection, TraitEngine}; use rustc_infer::traits::{Obligation, SelectionError, TraitObligation}; use rustc_lint_defs::builtin::DEREF_INTO_DYN_SUPERTRAIT; use rustc_middle::ty::print::with_no_trimmed_paths; @@ -199,11 +201,48 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { // and report ambiguity. if i > 1 { debug!("multiple matches, ambig"); + + // Collect a list of (probable) spans that point to a param-env candidate + let tcx = self.infcx.tcx; + let owner = stack.obligation.cause.body_id.owner.to_def_id(); + let predicates = tcx.predicates_of(owner).instantiate_identity(tcx); + let param_env_spans: FxHashMap<_, _> = elaborate_predicates_with_span( + tcx, + std::iter::zip(predicates.predicates, predicates.spans), + ) + .filter_map(|obligation| { + let kind = obligation.predicate.kind(); + if let ty::PredicateKind::Trait(trait_pred) = kind.skip_binder() { + if trait_pred.trait_ref + == ty::TraitRef::identity(tcx, trait_pred.def_id()) + .skip_binder() + { + // HACK: Remap the `Self: Trait` predicate that every trait has to a more useful span + Some(( + kind.rebind(trait_pred), + tcx.def_span(trait_pred.def_id()), + )) + } else { + Some((kind.rebind(trait_pred), obligation.cause.span)) + } + } else { + None + } + }) + .collect(); + return Err(Ambiguous( candidates .into_iter() .filter_map(|c| match c.candidate { - SelectionCandidate::ImplCandidate(def_id) => Some(def_id), + SelectionCandidate::ImplCandidate(def_id) => { + Some(AmbiguousSelection::Impl(def_id)) + } + SelectionCandidate::ParamCandidate(predicate) => { + Some(AmbiguousSelection::ParamEnv( + *param_env_spans.get(&predicate)?, + )) + } _ => None, }) .collect(), diff --git a/src/test/ui/issues/issue-21974.stderr b/src/test/ui/issues/issue-21974.stderr index 4e010a13653e7..2d60b18b1f208 100644 --- a/src/test/ui/issues/issue-21974.stderr +++ b/src/test/ui/issues/issue-21974.stderr @@ -4,7 +4,13 @@ error[E0283]: type annotations needed: cannot satisfy `&'a T: Foo` LL | where &'a T : Foo, | ^^^ | - = note: cannot satisfy `&'a T: Foo` +note: multiple `impl`s or `where` clauses satisfying `&'a T: Foo` found + --> $DIR/issue-21974.rs:11:19 + | +LL | where &'a T : Foo, + | ^^^ +LL | &'b T : Foo + | ^^^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-24424.stderr b/src/test/ui/issues/issue-24424.stderr index 8f3b2ac73199c..50d7f988e194c 100644 --- a/src/test/ui/issues/issue-24424.stderr +++ b/src/test/ui/issues/issue-24424.stderr @@ -4,7 +4,11 @@ error[E0283]: type annotations needed: cannot satisfy `T0: Trait0<'l0>` LL | impl <'l0, 'l1, T0> Trait1<'l0, T0> for bool where T0 : Trait0<'l0>, T0 : Trait0<'l1> {} | ^^^^^^^^^^^ | - = note: cannot satisfy `T0: Trait0<'l0>` +note: multiple `impl`s or `where` clauses satisfying `T0: Trait0<'l0>` found + --> $DIR/issue-24424.rs:4:57 + | +LL | impl <'l0, 'l1, T0> Trait1<'l0, T0> for bool where T0 : Trait0<'l0>, T0 : Trait0<'l1> {} + | ^^^^^^^^^^^ ^^^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/lifetimes/issue-34979.stderr b/src/test/ui/lifetimes/issue-34979.stderr index 5832c4d173c10..a78b3eaf6258d 100644 --- a/src/test/ui/lifetimes/issue-34979.stderr +++ b/src/test/ui/lifetimes/issue-34979.stderr @@ -4,7 +4,13 @@ error[E0283]: type annotations needed: cannot satisfy `&'a (): Foo` LL | &'a (): Foo, | ^^^ | - = note: cannot satisfy `&'a (): Foo` +note: multiple `impl`s or `where` clauses satisfying `&'a (): Foo` found + --> $DIR/issue-34979.rs:6:13 + | +LL | &'a (): Foo, + | ^^^ +LL | &'static (): Foo; + | ^^^ error: aborting due to previous error diff --git a/src/test/ui/traits/issue-85735.stderr b/src/test/ui/traits/issue-85735.stderr index fa280135beb2d..9e80497ca6e92 100644 --- a/src/test/ui/traits/issue-85735.stderr +++ b/src/test/ui/traits/issue-85735.stderr @@ -4,7 +4,14 @@ error[E0283]: type annotations needed: cannot satisfy `T: FnMut<(&'a (),)>` LL | T: FnMut(&'a ()), | ^^^^^^^^^^^^^ | - = note: cannot satisfy `T: FnMut<(&'a (),)>` +note: multiple `impl`s or `where` clauses satisfying `T: FnMut<(&'a (),)>` found + --> $DIR/issue-85735.rs:7:8 + | +LL | T: FnMut(&'a ()), + | ^^^^^^^^^^^^^ +LL | +LL | T: FnMut(&'b ()), + | ^^^^^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/type/type-check/issue-40294.stderr b/src/test/ui/type/type-check/issue-40294.stderr index 75feb5698eb63..d15fd23418bb0 100644 --- a/src/test/ui/type/type-check/issue-40294.stderr +++ b/src/test/ui/type/type-check/issue-40294.stderr @@ -4,7 +4,13 @@ error[E0283]: type annotations needed: cannot satisfy `&'a T: Foo` LL | where &'a T : Foo, | ^^^ | - = note: cannot satisfy `&'a T: Foo` +note: multiple `impl`s or `where` clauses satisfying `&'a T: Foo` found + --> $DIR/issue-40294.rs:6:19 + | +LL | where &'a T : Foo, + | ^^^ +LL | &'b T : Foo + | ^^^ error: aborting due to previous error