From ddf7f18190674b4c5446e21d5da89cb67817fc95 Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Thu, 3 Apr 2025 20:59:05 -0400 Subject: [PATCH 1/6] Extend `QueryStability` to handle `IntoIterator` implementations --- compiler/rustc_lint/src/internal.rs | 127 +++++++++++++++++- .../query_stability_into_iter.rs | 11 ++ .../query_stability_into_iter.stderr | 15 +++ 3 files changed, 147 insertions(+), 6 deletions(-) create mode 100644 tests/ui/internal-lints/query_stability_into_iter.rs create mode 100644 tests/ui/internal-lints/query_stability_into_iter.stderr diff --git a/compiler/rustc_lint/src/internal.rs b/compiler/rustc_lint/src/internal.rs index 1d4be24ea9fa0..f9d96ce01bbe5 100644 --- a/compiler/rustc_lint/src/internal.rs +++ b/compiler/rustc_lint/src/internal.rs @@ -1,10 +1,12 @@ //! Some lints that are only useful in the compiler or crates that use compiler internals, such as //! Clippy. -use rustc_hir::HirId; use rustc_hir::def::Res; use rustc_hir::def_id::DefId; -use rustc_middle::ty::{self, GenericArgsRef, Ty as MiddleTy}; +use rustc_hir::{Expr, ExprKind, HirId}; +use rustc_middle::ty::{ + self, ClauseKind, GenericArgsRef, ParamTy, ProjectionPredicate, TraitPredicate, Ty as MiddleTy, +}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::hygiene::{ExpnKind, MacroKind}; use rustc_span::{Span, sym}; @@ -101,10 +103,11 @@ declare_tool_lint! { declare_lint_pass!(QueryStability => [POTENTIAL_QUERY_INSTABILITY, UNTRACKED_QUERY_INFORMATION]); -impl LateLintPass<'_> for QueryStability { - fn check_expr(&mut self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) { - let Some((span, def_id, args)) = typeck_results_of_method_fn(cx, expr) else { return }; - if let Ok(Some(instance)) = ty::Instance::try_resolve(cx.tcx, cx.typing_env(), def_id, args) +impl<'tcx> LateLintPass<'tcx> for QueryStability { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + if let Some((span, def_id, args)) = typeck_results_of_method_fn(cx, expr) + && let Ok(Some(instance)) = + ty::Instance::try_resolve(cx.tcx, cx.typing_env(), def_id, args) { let def_id = instance.def_id(); if cx.tcx.has_attr(def_id, sym::rustc_lint_query_instability) { @@ -122,7 +125,119 @@ impl LateLintPass<'_> for QueryStability { ); } } + check_into_iter_stability(cx, expr); + } +} + +fn check_into_iter_stability<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + let Some(into_iterator_def_id) = cx.tcx.get_diagnostic_item(sym::IntoIterator) else { + return; + }; + // Is `expr` a function or method call? + let Some((callee_def_id, generic_args, recv, args)) = + get_callee_generic_args_and_args(cx, expr) + else { + return; + }; + let fn_sig = cx.tcx.fn_sig(callee_def_id).instantiate_identity().skip_binder(); + let (_, inputs) = fn_sig.inputs_and_output.as_slice().split_last().unwrap(); + for (arg_index, &input) in inputs.iter().enumerate() { + let &ty::Param(ParamTy { index: param_index, .. }) = input.kind() else { + continue; + }; + let (trait_predicates, _) = get_input_traits_and_projections(cx, callee_def_id, input); + for TraitPredicate { trait_ref, .. } in trait_predicates { + // Does the function or method require any of its arguments to implement `IntoIterator`? + if trait_ref.def_id != into_iterator_def_id { + continue; + } + let self_ty = generic_args[param_index as usize].expect_ty(); + let Some(self_ty_adt_def) = self_ty.peel_refs().ty_adt_def() else { + return; + }; + cx.tcx.for_each_relevant_impl(into_iterator_def_id, self_ty, |impl_id| { + let impl_ty = cx.tcx.type_of(impl_id).skip_binder(); + let Some(impl_ty_adt_def) = impl_ty.peel_refs().ty_adt_def() else { + return; + }; + // To reduce false positives, verify that `self_ty` and `impl_ty` refer to the same ADT. + if self_ty_adt_def != impl_ty_adt_def { + return; + } + let Some(into_iter_item) = cx + .tcx + .associated_items(impl_id) + .filter_by_name_unhygienic(sym::into_iter) + .next() + else { + return; + }; + // Does the input type's `IntoIterator` implementation have the + // `rustc_lint_query_instability` attribute on its `into_iter` method? + if !cx.tcx.has_attr(into_iter_item.def_id, sym::rustc_lint_query_instability) { + return; + } + let span = if let Some(recv) = recv { + if arg_index == 0 { recv.span } else { args[arg_index - 1].span } + } else { + args[arg_index].span + }; + cx.emit_span_lint( + POTENTIAL_QUERY_INSTABILITY, + span, + QueryInstability { query: cx.tcx.item_name(into_iter_item.def_id) }, + ); + }); + } + } +} + +/// Checks whether an expression is a function or method call and, if so, returns its `DefId`, +/// `GenericArgs`, and arguments. +fn get_callee_generic_args_and_args<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'tcx>, +) -> Option<(DefId, GenericArgsRef<'tcx>, Option<&'tcx Expr<'tcx>>, &'tcx [Expr<'tcx>])> { + if let ExprKind::Call(callee, args) = expr.kind + && let callee_ty = cx.typeck_results().expr_ty(callee) + && let ty::FnDef(callee_def_id, _) = callee_ty.kind() + { + let generic_args = cx.typeck_results().node_args(callee.hir_id); + return Some((*callee_def_id, generic_args, None, args)); + } + if let ExprKind::MethodCall(_, recv, args, _) = expr.kind + && let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) + { + let generic_args = cx.typeck_results().node_args(expr.hir_id); + return Some((method_def_id, generic_args, Some(recv), args)); + } + None +} + +/// Returns the `TraitPredicate`s and `ProjectionPredicate`s for a function's input type. +fn get_input_traits_and_projections<'tcx>( + cx: &LateContext<'tcx>, + callee_def_id: DefId, + input: MiddleTy<'tcx>, +) -> (Vec>, Vec>) { + let mut trait_predicates = Vec::new(); + let mut projection_predicates = Vec::new(); + for predicate in cx.tcx.param_env(callee_def_id).caller_bounds() { + match predicate.kind().skip_binder() { + ClauseKind::Trait(trait_predicate) => { + if trait_predicate.trait_ref.self_ty() == input { + trait_predicates.push(trait_predicate); + } + } + ClauseKind::Projection(projection_predicate) => { + if projection_predicate.projection_term.self_ty() == input { + projection_predicates.push(projection_predicate); + } + } + _ => {} + } } + (trait_predicates, projection_predicates) } declare_tool_lint! { diff --git a/tests/ui/internal-lints/query_stability_into_iter.rs b/tests/ui/internal-lints/query_stability_into_iter.rs new file mode 100644 index 0000000000000..c674786f09c69 --- /dev/null +++ b/tests/ui/internal-lints/query_stability_into_iter.rs @@ -0,0 +1,11 @@ +//@ compile-flags: -Z unstable-options + +#![deny(rustc::potential_query_instability)] + +use std::collections::HashSet; + +fn main() { + let set = HashSet::::default(); + HashSet::::default().extend(set); + //~^ ERROR using `into_iter` can result in unstable query results +} diff --git a/tests/ui/internal-lints/query_stability_into_iter.stderr b/tests/ui/internal-lints/query_stability_into_iter.stderr new file mode 100644 index 0000000000000..08caf79624120 --- /dev/null +++ b/tests/ui/internal-lints/query_stability_into_iter.stderr @@ -0,0 +1,15 @@ +error: using `into_iter` can result in unstable query results + --> $DIR/query_stability_into_iter.rs:9:38 + | +LL | HashSet::::default().extend(set); + | ^^^ + | + = note: if you believe this case to be fine, allow this lint and add a comment explaining your rationale +note: the lint level is defined here + --> $DIR/query_stability_into_iter.rs:3:9 + | +LL | #![deny(rustc::potential_query_instability)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + From 1a2c5a35b9d4a8258aa7f5f9996da842f93a7d20 Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Thu, 3 Apr 2025 21:12:58 -0400 Subject: [PATCH 2/6] Fix adjacent code --- compiler/rustc_errors/src/emitter.rs | 4 ++-- compiler/rustc_interface/src/interface.rs | 4 +++- compiler/rustc_resolve/src/late.rs | 2 ++ src/librustdoc/formats/cache.rs | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index fe01e289334cc..b1e02fedf230c 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -17,7 +17,7 @@ use std::path::Path; use std::sync::Arc; use derive_setters::Setters; -use rustc_data_structures::fx::{FxHashMap, FxIndexMap, FxIndexSet}; +use rustc_data_structures::fx::{FxIndexMap, FxIndexSet}; use rustc_data_structures::sync::{DynSend, IntoDynSyncSend}; use rustc_error_messages::{FluentArgs, SpanLabel}; use rustc_lexer; @@ -1840,7 +1840,7 @@ impl HumanEmitter { !is_cont && line_idx + 1 == annotated_file.lines.len(), ); - let mut to_add = FxHashMap::default(); + let mut to_add = FxIndexMap::default(); for (depth, style) in depths { // FIXME(#120456) - is `swap_remove` correct? diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index 3f87b1a547be5..048c00f9d9498 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -275,7 +275,9 @@ pub(crate) fn parse_check_cfg(dcx: DiagCtxtHandle<'_>, specs: Vec) -> Ch .expecteds .entry(name.name) .and_modify(|v| match v { - ExpectedValues::Some(v) if !values_any_specified => { + ExpectedValues::Some(v) if !values_any_specified => + { + #[allow(rustc::potential_query_instability)] v.extend(values.clone()) } ExpectedValues::Some(_) => *v = ExpectedValues::Any, diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 20e19caf9096b..bbad1278486b8 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -3934,12 +3934,14 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { // Move up the non-overlapping bindings to the or-pattern. // Existing bindings just get "merged". let collected = bindings.pop().unwrap().1; + #[allow(rustc::potential_query_instability)] // FIXME bindings.last_mut().unwrap().1.extend(collected); } // This or-pattern itself can itself be part of a product, // e.g. `(V1(a) | V2(a), a)` or `(a, V1(a) | V2(a))`. // Both cases bind `a` again in a product pattern and must be rejected. let collected = bindings.pop().unwrap().1; + #[allow(rustc::potential_query_instability)] // FIXME bindings.last_mut().unwrap().1.extend(collected); // Prevent visiting `ps` as we've already done so above. diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index 19402004ed59c..17f4b032ad276 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -47,7 +47,7 @@ pub(crate) struct Cache { /// Similar to `paths`, but only holds external paths. This is only used for /// generating explicit hyperlinks to other crates. - pub(crate) external_paths: FxHashMap, ItemType)>, + pub(crate) external_paths: FxIndexMap, ItemType)>, /// Maps local `DefId`s of exported types to fully qualified paths. /// Unlike 'paths', this mapping ignores any renames that occur From 420b9864b74bee3f7b8f3915aff10047742fe873 Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Fri, 4 Apr 2025 15:22:09 -0400 Subject: [PATCH 3/6] Fix duplicate warning; merge test into `tests/ui-fulldeps/internal-lints` --- compiler/rustc_lint/src/internal.rs | 3 +++ .../ui-fulldeps/internal-lints/query_stability.rs | 3 +++ .../internal-lints/query_stability.stderr | 10 +++++++++- .../internal-lints/query_stability_into_iter.rs | 11 ----------- .../query_stability_into_iter.stderr | 15 --------------- 5 files changed, 15 insertions(+), 27 deletions(-) delete mode 100644 tests/ui/internal-lints/query_stability_into_iter.rs delete mode 100644 tests/ui/internal-lints/query_stability_into_iter.stderr diff --git a/compiler/rustc_lint/src/internal.rs b/compiler/rustc_lint/src/internal.rs index f9d96ce01bbe5..f51a6050e969c 100644 --- a/compiler/rustc_lint/src/internal.rs +++ b/compiler/rustc_lint/src/internal.rs @@ -133,6 +133,9 @@ fn check_into_iter_stability<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx let Some(into_iterator_def_id) = cx.tcx.get_diagnostic_item(sym::IntoIterator) else { return; }; + if expr.span.from_expansion() { + return; + }; // Is `expr` a function or method call? let Some((callee_def_id, generic_args, recv, args)) = get_callee_generic_args_and_args(cx, expr) diff --git a/tests/ui-fulldeps/internal-lints/query_stability.rs b/tests/ui-fulldeps/internal-lints/query_stability.rs index 7b897fabd2d80..f136dde7349b9 100644 --- a/tests/ui-fulldeps/internal-lints/query_stability.rs +++ b/tests/ui-fulldeps/internal-lints/query_stability.rs @@ -34,4 +34,7 @@ fn main() { //~^ ERROR using `values_mut` can result in unstable query results *val = *val + 10; } + + FxHashMap::::default().extend(x); + //~^ ERROR using `into_iter` can result in unstable query results } diff --git a/tests/ui-fulldeps/internal-lints/query_stability.stderr b/tests/ui-fulldeps/internal-lints/query_stability.stderr index 43b156dc20a3c..deedd7b1b91f1 100644 --- a/tests/ui-fulldeps/internal-lints/query_stability.stderr +++ b/tests/ui-fulldeps/internal-lints/query_stability.stderr @@ -59,5 +59,13 @@ LL | for val in x.values_mut() { | = note: if you believe this case to be fine, allow this lint and add a comment explaining your rationale -error: aborting due to 7 previous errors +error: using `into_iter` can result in unstable query results + --> $DIR/query_stability.rs:38:45 + | +LL | FxHashMap::::default().extend(x); + | ^ + | + = note: if you believe this case to be fine, allow this lint and add a comment explaining your rationale + +error: aborting due to 8 previous errors diff --git a/tests/ui/internal-lints/query_stability_into_iter.rs b/tests/ui/internal-lints/query_stability_into_iter.rs deleted file mode 100644 index c674786f09c69..0000000000000 --- a/tests/ui/internal-lints/query_stability_into_iter.rs +++ /dev/null @@ -1,11 +0,0 @@ -//@ compile-flags: -Z unstable-options - -#![deny(rustc::potential_query_instability)] - -use std::collections::HashSet; - -fn main() { - let set = HashSet::::default(); - HashSet::::default().extend(set); - //~^ ERROR using `into_iter` can result in unstable query results -} diff --git a/tests/ui/internal-lints/query_stability_into_iter.stderr b/tests/ui/internal-lints/query_stability_into_iter.stderr deleted file mode 100644 index 08caf79624120..0000000000000 --- a/tests/ui/internal-lints/query_stability_into_iter.stderr +++ /dev/null @@ -1,15 +0,0 @@ -error: using `into_iter` can result in unstable query results - --> $DIR/query_stability_into_iter.rs:9:38 - | -LL | HashSet::::default().extend(set); - | ^^^ - | - = note: if you believe this case to be fine, allow this lint and add a comment explaining your rationale -note: the lint level is defined here - --> $DIR/query_stability_into_iter.rs:3:9 - | -LL | #![deny(rustc::potential_query_instability)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -error: aborting due to 1 previous error - From dfec3c025c053cf555678a6f412acf23c681afaa Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Wed, 30 Apr 2025 07:46:57 -0400 Subject: [PATCH 4/6] Use `rustc_middle::ty::FnSig::inputs` --- compiler/rustc_lint/src/internal.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/rustc_lint/src/internal.rs b/compiler/rustc_lint/src/internal.rs index f51a6050e969c..f56cbcffdcb7c 100644 --- a/compiler/rustc_lint/src/internal.rs +++ b/compiler/rustc_lint/src/internal.rs @@ -143,8 +143,7 @@ fn check_into_iter_stability<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx return; }; let fn_sig = cx.tcx.fn_sig(callee_def_id).instantiate_identity().skip_binder(); - let (_, inputs) = fn_sig.inputs_and_output.as_slice().split_last().unwrap(); - for (arg_index, &input) in inputs.iter().enumerate() { + for (arg_index, &input) in fn_sig.inputs().iter().enumerate() { let &ty::Param(ParamTy { index: param_index, .. }) = input.kind() else { continue; }; From ae437dd730fa483da02faf8d4913952cfa70b422 Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Wed, 28 May 2025 14:35:41 -0400 Subject: [PATCH 5/6] Address two review comments - https://github.com/rust-lang/rust/pull/139345#discussion_r2109006991 - https://github.com/rust-lang/rust/pull/139345#discussion_r2109058588 --- compiler/rustc_lint/src/internal.rs | 73 ++++++------------- .../internal-lints/query_stability.rs | 9 +++ .../internal-lints/query_stability.stderr | 10 ++- 3 files changed, 40 insertions(+), 52 deletions(-) diff --git a/compiler/rustc_lint/src/internal.rs b/compiler/rustc_lint/src/internal.rs index f56cbcffdcb7c..c1a4fc52848be 100644 --- a/compiler/rustc_lint/src/internal.rs +++ b/compiler/rustc_lint/src/internal.rs @@ -58,25 +58,6 @@ impl LateLintPass<'_> for DefaultHashTypes { } } -/// Helper function for lints that check for expressions with calls and use typeck results to -/// get the `DefId` and `GenericArgsRef` of the function. -fn typeck_results_of_method_fn<'tcx>( - cx: &LateContext<'tcx>, - expr: &hir::Expr<'_>, -) -> Option<(Span, DefId, ty::GenericArgsRef<'tcx>)> { - match expr.kind { - hir::ExprKind::MethodCall(segment, ..) - if let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) => - { - Some((segment.ident.span, def_id, cx.typeck_results().node_args(expr.hir_id))) - } - _ => match cx.typeck_results().node_type(expr.hir_id).kind() { - &ty::FnDef(def_id, args) => Some((expr.span, def_id, args)), - _ => None, - }, - } -} - declare_tool_lint! { /// The `potential_query_instability` lint detects use of methods which can lead to /// potential query instability, such as iterating over a `HashMap`. @@ -105,9 +86,10 @@ declare_lint_pass!(QueryStability => [POTENTIAL_QUERY_INSTABILITY, UNTRACKED_QUE impl<'tcx> LateLintPass<'tcx> for QueryStability { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { - if let Some((span, def_id, args)) = typeck_results_of_method_fn(cx, expr) + if let Some((def_id, span, generic_args, _recv, _args)) = + get_callee_span_generic_args_and_args(cx, expr) && let Ok(Some(instance)) = - ty::Instance::try_resolve(cx.tcx, cx.typing_env(), def_id, args) + ty::Instance::try_resolve(cx.tcx, cx.typing_env(), def_id, generic_args) { let def_id = instance.def_id(); if cx.tcx.has_attr(def_id, sym::rustc_lint_query_instability) { @@ -137,8 +119,8 @@ fn check_into_iter_stability<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx return; }; // Is `expr` a function or method call? - let Some((callee_def_id, generic_args, recv, args)) = - get_callee_generic_args_and_args(cx, expr) + let Some((callee_def_id, _span, generic_args, recv, args)) = + get_callee_span_generic_args_and_args(cx, expr) else { return; }; @@ -195,23 +177,23 @@ fn check_into_iter_stability<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx } /// Checks whether an expression is a function or method call and, if so, returns its `DefId`, -/// `GenericArgs`, and arguments. -fn get_callee_generic_args_and_args<'tcx>( +/// `Span`, `GenericArgs`, and arguments. This is a slight augmentation of a similarly named Clippy +/// function, `get_callee_generic_args_and_args`. +fn get_callee_span_generic_args_and_args<'tcx>( cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, -) -> Option<(DefId, GenericArgsRef<'tcx>, Option<&'tcx Expr<'tcx>>, &'tcx [Expr<'tcx>])> { +) -> Option<(DefId, Span, GenericArgsRef<'tcx>, Option<&'tcx Expr<'tcx>>, &'tcx [Expr<'tcx>])> { if let ExprKind::Call(callee, args) = expr.kind && let callee_ty = cx.typeck_results().expr_ty(callee) - && let ty::FnDef(callee_def_id, _) = callee_ty.kind() + && let ty::FnDef(callee_def_id, generic_args) = callee_ty.kind() { - let generic_args = cx.typeck_results().node_args(callee.hir_id); - return Some((*callee_def_id, generic_args, None, args)); + return Some((*callee_def_id, callee.span, generic_args, None, args)); } - if let ExprKind::MethodCall(_, recv, args, _) = expr.kind + if let ExprKind::MethodCall(segment, recv, args, _) = expr.kind && let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) { let generic_args = cx.typeck_results().node_args(expr.hir_id); - return Some((method_def_id, generic_args, Some(recv), args)); + return Some((method_def_id, segment.ident.span, generic_args, Some(recv), args)); } None } @@ -552,33 +534,22 @@ declare_tool_lint! { declare_lint_pass!(Diagnostics => [UNTRANSLATABLE_DIAGNOSTIC, DIAGNOSTIC_OUTSIDE_OF_IMPL]); impl LateLintPass<'_> for Diagnostics { - fn check_expr(&mut self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) { + fn check_expr<'tcx>(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) { let collect_args_tys_and_spans = |args: &[hir::Expr<'_>], reserve_one_extra: bool| { let mut result = Vec::with_capacity(args.len() + usize::from(reserve_one_extra)); result.extend(args.iter().map(|arg| (cx.typeck_results().expr_ty(arg), arg.span))); result }; // Only check function calls and method calls. - let (span, def_id, fn_gen_args, arg_tys_and_spans) = match expr.kind { - hir::ExprKind::Call(callee, args) => { - match cx.typeck_results().node_type(callee.hir_id).kind() { - &ty::FnDef(def_id, fn_gen_args) => { - (callee.span, def_id, fn_gen_args, collect_args_tys_and_spans(args, false)) - } - _ => return, // occurs for fns passed as args - } - } - hir::ExprKind::MethodCall(_segment, _recv, args, _span) => { - let Some((span, def_id, fn_gen_args)) = typeck_results_of_method_fn(cx, expr) - else { - return; - }; - let mut args = collect_args_tys_and_spans(args, true); - args.insert(0, (cx.tcx.types.self_param, _recv.span)); // dummy inserted for `self` - (span, def_id, fn_gen_args, args) - } - _ => return, + let Some((def_id, span, fn_gen_args, recv, args)) = + get_callee_span_generic_args_and_args(cx, expr) + else { + return; }; + let mut arg_tys_and_spans = collect_args_tys_and_spans(args, recv.is_some()); + if let Some(recv) = recv { + arg_tys_and_spans.insert(0, (cx.tcx.types.self_param, recv.span)); // dummy inserted for `self` + } Self::diagnostic_outside_of_impl(cx, span, expr.hir_id, def_id, fn_gen_args); Self::untranslatable_diagnostic(cx, def_id, &arg_tys_and_spans); diff --git a/tests/ui-fulldeps/internal-lints/query_stability.rs b/tests/ui-fulldeps/internal-lints/query_stability.rs index f136dde7349b9..9dc6525006484 100644 --- a/tests/ui-fulldeps/internal-lints/query_stability.rs +++ b/tests/ui-fulldeps/internal-lints/query_stability.rs @@ -38,3 +38,12 @@ fn main() { FxHashMap::::default().extend(x); //~^ ERROR using `into_iter` can result in unstable query results } + +fn hide_into_iter(x: impl IntoIterator) -> impl Iterator { + x.into_iter() +} + +fn take(map: std::collections::HashMap) { + _ = hide_into_iter(map); + //~^ ERROR using `into_iter` can result in unstable query results +} diff --git a/tests/ui-fulldeps/internal-lints/query_stability.stderr b/tests/ui-fulldeps/internal-lints/query_stability.stderr index deedd7b1b91f1..dfd37417e8bb8 100644 --- a/tests/ui-fulldeps/internal-lints/query_stability.stderr +++ b/tests/ui-fulldeps/internal-lints/query_stability.stderr @@ -67,5 +67,13 @@ LL | FxHashMap::::default().extend(x); | = note: if you believe this case to be fine, allow this lint and add a comment explaining your rationale -error: aborting due to 8 previous errors +error: using `into_iter` can result in unstable query results + --> $DIR/query_stability.rs:47:24 + | +LL | _ = hide_into_iter(map); + | ^^^ + | + = note: if you believe this case to be fine, allow this lint and add a comment explaining your rationale + +error: aborting due to 9 previous errors From 085312b93e0384b27285e2b08612bba30428a184 Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Wed, 28 May 2025 19:33:18 -0400 Subject: [PATCH 6/6] Use `Instance::try_resolve` --- compiler/rustc_lint/src/internal.rs | 60 ++++++++++++----------------- 1 file changed, 25 insertions(+), 35 deletions(-) diff --git a/compiler/rustc_lint/src/internal.rs b/compiler/rustc_lint/src/internal.rs index c1a4fc52848be..323c267966930 100644 --- a/compiler/rustc_lint/src/internal.rs +++ b/compiler/rustc_lint/src/internal.rs @@ -115,6 +115,9 @@ fn check_into_iter_stability<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx let Some(into_iterator_def_id) = cx.tcx.get_diagnostic_item(sym::IntoIterator) else { return; }; + let Some(into_iter_fn_def_id) = cx.tcx.lang_items().into_iter_fn() else { + return; + }; if expr.span.from_expansion() { return; }; @@ -135,43 +138,30 @@ fn check_into_iter_stability<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx if trait_ref.def_id != into_iterator_def_id { continue; } - let self_ty = generic_args[param_index as usize].expect_ty(); - let Some(self_ty_adt_def) = self_ty.peel_refs().ty_adt_def() else { + let self_ty_generic_arg = generic_args[param_index as usize]; + let Ok(Some(instance)) = ty::Instance::try_resolve( + cx.tcx, + cx.typing_env(), + into_iter_fn_def_id, + cx.tcx.mk_args(&[self_ty_generic_arg]), + ) else { + continue; + }; + // Does the input type's `IntoIterator` implementation have the + // `rustc_lint_query_instability` attribute on its `into_iter` method? + if !cx.tcx.has_attr(instance.def_id(), sym::rustc_lint_query_instability) { return; + } + let span = if let Some(recv) = recv { + if arg_index == 0 { recv.span } else { args[arg_index - 1].span } + } else { + args[arg_index].span }; - cx.tcx.for_each_relevant_impl(into_iterator_def_id, self_ty, |impl_id| { - let impl_ty = cx.tcx.type_of(impl_id).skip_binder(); - let Some(impl_ty_adt_def) = impl_ty.peel_refs().ty_adt_def() else { - return; - }; - // To reduce false positives, verify that `self_ty` and `impl_ty` refer to the same ADT. - if self_ty_adt_def != impl_ty_adt_def { - return; - } - let Some(into_iter_item) = cx - .tcx - .associated_items(impl_id) - .filter_by_name_unhygienic(sym::into_iter) - .next() - else { - return; - }; - // Does the input type's `IntoIterator` implementation have the - // `rustc_lint_query_instability` attribute on its `into_iter` method? - if !cx.tcx.has_attr(into_iter_item.def_id, sym::rustc_lint_query_instability) { - return; - } - let span = if let Some(recv) = recv { - if arg_index == 0 { recv.span } else { args[arg_index - 1].span } - } else { - args[arg_index].span - }; - cx.emit_span_lint( - POTENTIAL_QUERY_INSTABILITY, - span, - QueryInstability { query: cx.tcx.item_name(into_iter_item.def_id) }, - ); - }); + cx.emit_span_lint( + POTENTIAL_QUERY_INSTABILITY, + span, + QueryInstability { query: cx.tcx.item_name(instance.def_id()) }, + ); } } }