From ddf7f18190674b4c5446e21d5da89cb67817fc95 Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Thu, 3 Apr 2025 20:59:05 -0400 Subject: [PATCH 1/3] 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 1d4be24ea9f..f9d96ce01bb 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,9 +125,121 @@ 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! { /// The `usage_of_ty_tykind` lint detects usages of `ty::TyKind::`, /// where `ty::` would suffice. 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 00000000000..c674786f09c --- /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 00000000000..08caf796241 --- /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/3] 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 fe01e289334..b1e02fedf23 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 3f87b1a547b..048c00f9d94 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 20e19caf909..bbad1278486 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 19402004ed5..17f4b032ad2 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/3] 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 f9d96ce01bb..f51a6050e96 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 7b897fabd2d..f136dde7349 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 43b156dc20a..deedd7b1b91 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 c674786f09c..00000000000 --- 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 08caf796241..00000000000 --- 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 -