Rollup merge of #139345 - smoelius:into-iter-stability, r=lcnr

Extend `QueryStability` to handle `IntoIterator` implementations

This PR extends the `rustc::potential_query_instability` lint to check values passed as `IntoIterator` implementations.

Full disclosure: I want the lint to warn about this line (please see #138871 for why): aa8f0fd716/src/librustdoc/json/mod.rs (L261)

However, the lint warns about several other lines as well.

Final note: the functions `get_callee_generic_args_and_args` and `get_input_traits_and_projections` were copied directly from [Clippy's source code](4fd8c04da0/src/tools/clippy/clippy_lints/src/methods/unnecessary_to_owned.rs (L445-L496)).

compiler/rustc_codegen_ssa/src/target_features.rs

@@ -180,6 +180,7 @@ fn parse_rust_feature_flag<'a>(
             while let Some(new_feature) = new_features.pop() {
                 if features.insert(new_feature) {
                     if let Some(implied_features) = inverse_implied_features.get(&new_feature) {
+                        #[allow(rustc::potential_query_instability)]
                         new_features.extend(implied_features)
                     }
                 }

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;
@@ -1853,7 +1853,7 @@ impl HumanEmitter {
                             && 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?

compiler/rustc_expand/src/mbe/macro_rules.rs

@@ -596,6 +596,7 @@ pub(super) fn try_match_macro_attr<'matcher, T: Tracker<'matcher>>(
         match result {
             Success(body_named_matches) => {
                 psess.gated_spans.merge(gated_spans_snapshot);
+                #[allow(rustc::potential_query_instability)]
                 named_matches.extend(body_named_matches);
                 return Ok((i, rule, named_matches));
             }

compiler/rustc_interface/src/interface.rs

@@ -285,7 +285,9 @@ pub(crate) fn parse_check_cfg(dcx: DiagCtxtHandle<'_>, specs: Vec<String>) -> 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,

compiler/rustc_lint/src/internal.rs

@@ -1,10 +1,10 @@
 //! 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, PredicatePolarity, TraitPredicate, Ty};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::hygiene::{ExpnKind, MacroKind};
 use rustc_span::{Span, sym};
@@ -56,25 +56,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`.
@@ -101,10 +82,12 @@ 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((callee_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(), callee_def_id, generic_args)
         {
             let def_id = instance.def_id();
             if cx.tcx.has_attr(def_id, sym::rustc_lint_query_instability) {
@@ -113,7 +96,15 @@ impl LateLintPass<'_> for QueryStability {
                     span,
                     QueryInstability { query: cx.tcx.item_name(def_id) },
                 );
+            } else if has_unstable_into_iter_predicate(cx, callee_def_id, generic_args) {
+                let call_span = span.with_hi(expr.span.hi());
+                cx.emit_span_lint(
+                    POTENTIAL_QUERY_INSTABILITY,
+                    call_span,
+                    QueryInstability { query: sym::into_iter },
+                );
             }
+
             if cx.tcx.has_attr(def_id, sym::rustc_lint_untracked_query_information) {
                 cx.emit_span_lint(
                     UNTRACKED_QUERY_INFORMATION,
@@ -125,6 +116,64 @@ impl LateLintPass<'_> for QueryStability {
     }
 }
 
+fn has_unstable_into_iter_predicate<'tcx>(
+    cx: &LateContext<'tcx>,
+    callee_def_id: DefId,
+    generic_args: GenericArgsRef<'tcx>,
+) -> bool {
+    let Some(into_iterator_def_id) = cx.tcx.get_diagnostic_item(sym::IntoIterator) else {
+        return false;
+    };
+    let Some(into_iter_fn_def_id) = cx.tcx.lang_items().into_iter_fn() else {
+        return false;
+    };
+    let predicates = cx.tcx.predicates_of(callee_def_id).instantiate(cx.tcx, generic_args);
+    for (predicate, _) in predicates {
+        let ClauseKind::Trait(TraitPredicate { trait_ref, polarity: PredicatePolarity::Positive }) =
+            predicate.kind().skip_binder()
+        else {
+            continue;
+        };
+        // Does the function or method require any of its arguments to implement `IntoIterator`?
+        if trait_ref.def_id != into_iterator_def_id {
+            continue;
+        }
+        let Ok(Some(instance)) =
+            ty::Instance::try_resolve(cx.tcx, cx.typing_env(), into_iter_fn_def_id, trait_ref.args)
+        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 true;
+        }
+    }
+    false
+}
+
+/// Checks whether an expression is a function or method call and, if so, returns its `DefId`,
+/// `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, 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, generic_args) = callee_ty.kind()
+    {
+        return Some((*callee_def_id, callee.span, generic_args, None, args));
+    }
+    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, segment.ident.span, generic_args, Some(recv), args));
+    }
+    None
+}
+
 declare_tool_lint! {
     /// The `usage_of_ty_tykind` lint detects usages of `ty::TyKind::<kind>`,
     /// where `ty::<kind>` would suffice.
@@ -461,33 +510,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);
@@ -496,7 +534,7 @@ impl LateLintPass<'_> for Diagnostics {
 
 impl Diagnostics {
     // Is the type `{D,Subd}iagMessage`?
-    fn is_diag_message<'cx>(cx: &LateContext<'cx>, ty: MiddleTy<'cx>) -> bool {
+    fn is_diag_message<'cx>(cx: &LateContext<'cx>, ty: Ty<'cx>) -> bool {
         if let Some(adt_def) = ty.ty_adt_def()
             && let Some(name) = cx.tcx.get_diagnostic_name(adt_def.did())
             && matches!(name, sym::DiagMessage | sym::SubdiagMessage)
@@ -510,7 +548,7 @@ impl Diagnostics {
     fn untranslatable_diagnostic<'cx>(
         cx: &LateContext<'cx>,
         def_id: DefId,
-        arg_tys_and_spans: &[(MiddleTy<'cx>, Span)],
+        arg_tys_and_spans: &[(Ty<'cx>, Span)],
     ) {
         let fn_sig = cx.tcx.fn_sig(def_id).instantiate_identity().skip_binder();
         let predicates = cx.tcx.predicates_of(def_id).instantiate_identity(cx.tcx).predicates;

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<DefId, (Vec<Symbol>, ItemType)>,
+    pub(crate) external_paths: FxIndexMap<DefId, (Vec<Symbol>, ItemType)>,
 
     /// Maps local `DefId`s of exported types to fully qualified paths.
     /// Unlike 'paths', this mapping ignores any renames that occur

tests/ui-fulldeps/internal-lints/query_stability.rs

@@ -1,4 +1,5 @@
 //@ compile-flags: -Z unstable-options
+//@ ignore-stage1
 
 #![feature(rustc_private)]
 #![deny(rustc::potential_query_instability)]
@@ -34,4 +35,16 @@ fn main() {
         //~^ ERROR using `values_mut` can result in unstable query results
         *val = *val + 10;
     }
+
+    FxHashMap::<u32, i32>::default().extend(x);
+    //~^ ERROR using `into_iter` can result in unstable query results
+}
+
+fn hide_into_iter<T>(x: impl IntoIterator<Item = T>) -> impl Iterator<Item = T> {
+    x.into_iter()
+}
+
+fn take(map: std::collections::HashMap<i32, i32>) {
+    _ = hide_into_iter(map);
+    //~^ ERROR using `into_iter` can result in unstable query results
 }

tests/ui-fulldeps/internal-lints/query_stability.stderr

@@ -1,18 +1,18 @@
 error: using `drain` can result in unstable query results
-  --> $DIR/query_stability.rs:13:16
+  --> $DIR/query_stability.rs:14:16
    |
 LL |     for _ in x.drain() {}
    |                ^^^^^
    |
    = 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.rs:4:9
+  --> $DIR/query_stability.rs:5:9
    |
 LL | #![deny(rustc::potential_query_instability)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: using `iter` can result in unstable query results
-  --> $DIR/query_stability.rs:16:16
+  --> $DIR/query_stability.rs:17:16
    |
 LL |     for _ in x.iter() {}
    |                ^^^^
@@ -20,7 +20,7 @@ LL |     for _ in x.iter() {}
    = note: if you believe this case to be fine, allow this lint and add a comment explaining your rationale
 
 error: using `iter_mut` can result in unstable query results
-  --> $DIR/query_stability.rs:19:36
+  --> $DIR/query_stability.rs:20:36
    |
 LL |     for _ in Some(&mut x).unwrap().iter_mut() {}
    |                                    ^^^^^^^^
@@ -28,7 +28,7 @@ LL |     for _ in Some(&mut x).unwrap().iter_mut() {}
    = note: if you believe this case to be fine, allow this lint and add a comment explaining your rationale
 
 error: using `into_iter` can result in unstable query results
-  --> $DIR/query_stability.rs:22:14
+  --> $DIR/query_stability.rs:23:14
    |
 LL |     for _ in x {}
    |              ^
@@ -36,7 +36,7 @@ LL |     for _ in x {}
    = note: if you believe this case to be fine, allow this lint and add a comment explaining your rationale
 
 error: using `keys` can result in unstable query results
-  --> $DIR/query_stability.rs:26:15
+  --> $DIR/query_stability.rs:27:15
    |
 LL |     let _ = x.keys();
    |               ^^^^
@@ -44,7 +44,7 @@ LL |     let _ = x.keys();
    = note: if you believe this case to be fine, allow this lint and add a comment explaining your rationale
 
 error: using `values` can result in unstable query results
-  --> $DIR/query_stability.rs:29:15
+  --> $DIR/query_stability.rs:30:15
    |
 LL |     let _ = x.values();
    |               ^^^^^^
@@ -52,12 +52,28 @@ LL |     let _ = x.values();
    = note: if you believe this case to be fine, allow this lint and add a comment explaining your rationale
 
 error: using `values_mut` can result in unstable query results
-  --> $DIR/query_stability.rs:33:18
+  --> $DIR/query_stability.rs:34:18
    |
 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:39:38
+   |
+LL |     FxHashMap::<u32, i32>::default().extend(x);
+   |                                      ^^^^^^^^^
+   |
+   = note: if you believe this case to be fine, allow this lint and add a comment explaining your rationale
+
+error: using `into_iter` can result in unstable query results
+  --> $DIR/query_stability.rs:48:9
+   |
+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