rust-lang/rust · #163538

Add `rustc::missing_generic_type_visitable` lint

ada4a · merged Oct 8, 202611 files · 321 + / 57 −
compiler/rustc_lint/src/diagnostics.rs11 + / 0 −
@@ -3261,3 +3261,14 @@ pub(crate) enum RawBorrowViaReferenceSuggestion<'a> {     #[help("consider using `&raw {$mutbl}` for a safer and more explicit raw pointer")]     Spanless { mutbl: &'a str }, }++#[derive(Diagnostic)]+#[diag("missing `#[derive(GenericTypeVisitable)]` on a type which implements `TypeVisitable`")]+#[note_once(+    "`GenericTypeVisitable` is rust-analyzer's version of `TypeVisitable`; it should be implemented whenever the latter is"+)]+#[help("mark the type with `#[derive(GenericTypeVisitable)]`")]+#[help(+    "or, if `GenericTypeVisitable` is derived, check whether all fields implement `GenericTypeVisitable`"+)]+pub(crate) struct MissingGenericTypeVisitableDeriveDiag;
compiler/rustc_lint/src/internal.rs68 + / 5 −
@@ -8,17 +8,20 @@ use rustc_hir as hir; use rustc_hir::def::Res; use rustc_hir::def_id::DefId; use rustc_hir::{Expr, ExprKind, HirId};+use rustc_infer::infer::TyCtxtInferExt as _; use rustc_lint_defs::{declare_lint_pass, declare_tool_lint}; use rustc_middle::ty::{self, ClausePolarity, GenericArgsRef}; use rustc_span::hygiene::{ExpnKind, MacroKind};-use rustc_span::{Span, sym};+use rustc_span::{DUMMY_SP, Span, sym};+use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _;+use rustc_trait_selection::traits::{Obligation, ObligationCause};  use crate::diagnostics::{     AttributeKindInFindAttr, BadOptAccessDiag, DefaultHashTypesDiag,-    ImplicitSysrootCrateImportDiag, LintPassByHand, NonGlobImportTypeIrInherent, QueryInstability,-    QueryUntracked, RustcMustMatchExhaustivelyNotExhaustive, SpanUseEqCtxtDiag,-    SymbolInternStringLiteralDiag, TyQualified, TykindDiag, TykindKind, TypeIrDirectUse,-    TypeIrInherentUsage, TypeIrTraitUsage,+    ImplicitSysrootCrateImportDiag, LintPassByHand, MissingGenericTypeVisitableDeriveDiag,+    NonGlobImportTypeIrInherent, QueryInstability, QueryUntracked,+    RustcMustMatchExhaustivelyNotExhaustive, SpanUseEqCtxtDiag, SymbolInternStringLiteralDiag,+    TyQualified, TykindDiag, TykindKind, TypeIrDirectUse, TypeIrInherentUsage, TypeIrTraitUsage, }; use crate::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext}; @@ -873,3 +876,63 @@ impl<'tcx> LateLintPass<'tcx> for RustcMustMatchExhaustively {         }     } }++declare_tool_lint! {+    /// The `missing_generic_type_visitable_derive` lint detects types implementing `TypeVisitable`+    /// but not deriving `GenericTypeVisitable`.+    ///+    /// `GenericTypeVisitable` is the analog of `TypeVisitable` used by rust-analyzer, and needs to+    /// be implemented whenever `TypeVisitable` is. See [`rustc_type_ir::generic_visit`] for more+    /// information.+    pub rustc::MISSING_GENERIC_TYPE_VISITABLE_DERIVE,+    // This lint is meant to be manually enabled in selected crates (currently only `rustc_type_ir`)+    Allow,+    "Forbid implementing `TypeVisitable` without deriving `GenericTypeVisitable`",+    report_in_external_macro: true+}++declare_lint_pass!(MissingGenericTypeVisitableDerive => [MISSING_GENERIC_TYPE_VISITABLE_DERIVE]);++impl<'tcx> LateLintPass<'tcx> for MissingGenericTypeVisitableDerive {+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) {+        // lint ADTs except unions, as neither trait can be implemented on the latter+        if matches!(item.kind, hir::ItemKind::Struct(..) | hir::ItemKind::Enum(..))+            && let Some(regular_id) = cx.tcx.get_diagnostic_item(sym::TypeVisitable)+            && let Some(generic_id) = cx.tcx.get_diagnostic_item(sym::GenericTypeVisitable)+            && let (infcx, param_env) = cx.tcx.infer_ctxt().build_with_typing_env(cx.typing_env())+            && let self_ty = {+                let self_id = item.owner_id.to_def_id();+                let args = infcx.fresh_args_for_item(DUMMY_SP, self_id);+                cx.tcx.type_of(self_id).instantiate(cx.tcx, args).skip_norm_wip()+            }+            // check whether `Self: TypeVisitable<I>` holds for some `I`..+            && {+                let predicate =+                    ty::TraitRef::new(cx.tcx, regular_id, [self_ty, infcx.next_ty_var(DUMMY_SP)]);+                let obligation =+                    Obligation::new(cx.tcx, ObligationCause::dummy(), param_env, predicate);++                // NOTE: I initially went with `predicate_must_hold_modulo_regions` (which is what's+                // used by Clippy's [`implements_trait`](https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/ty/fn.implements_trait.html),+                // but that returned `false` more often than I'd hoped, and so I went with the+                // next-most strict function.+                infcx.predicate_may_hold(&obligation)+            }+            // ..while `Self: GenericTypeVisitable<V>` doesn't+            && {+                let predicate =+                    ty::TraitRef::new(cx.tcx, generic_id, [self_ty, infcx.next_ty_var(DUMMY_SP)]);+                let obligation =+                    Obligation::new(cx.tcx, ObligationCause::dummy(), param_env, predicate);++                !infcx.predicate_may_hold(&obligation)+            }+        {+            cx.emit_span_lint(+                MISSING_GENERIC_TYPE_VISITABLE_DERIVE,+                item.span,+                MissingGenericTypeVisitableDeriveDiag,+            );+        }+    }+}
compiler/rustc_lint/src/lib.rs6 + / 0 −
@@ -298,6 +298,7 @@ late_lint_methods!(             BadOptAccess: BadOptAccess,             DefaultHashTypes: DefaultHashTypes,             DisallowedPassByRef: DisallowedPassByRef,+            MissingGenericTypeVisitableDerive: MissingGenericTypeVisitableDerive,             QueryStability: QueryStability,             RustcMustMatchExhaustively: RustcMustMatchExhaustively,             SpanUseEqCtxt: SpanUseEqCtxt,@@ -769,6 +770,11 @@ fn register_internals(store: &mut LintStore) {             //             // Late pass: RustcMustMatchExhaustively             LintId::of(RUSTC_MUST_MATCH_EXHAUSTIVELY),+            // Late pass: MissingGenericTypeVisitableDerive+            // Note: this one is not included so that it doesn't become effectively warn-by-default+            // (since `rustc::internal` is warn-by-default). Instead, it's enabled manually in+            // each relevant crate.+            // LintId::of(MISSING_GENERIC_TYPE_VISITABLE_DERIVE),         ],     ); }
compiler/rustc_span/src/symbol.rs2 + / 0 −
@@ -237,6 +237,7 @@ symbols! {         From,         FromIterator,         FromResidual,+        GenericTypeVisitable,         GlobalAlloc,         Hash,         HashMap,@@ -358,6 +359,7 @@ symbols! {         TyCtxt,         TyKind,         Type,+        TypeVisitable,         Union,         Unresolved,         Unsafe,
compiler/rustc_type_ir/src/generic_visit.rs7 + / 47 −
@@ -26,6 +26,7 @@ use crate::Interner; /// /// Therefore, it is advised to instead derive this using the derive /// macro located in `rustc_macros`.+#[cfg_attr(feature = "nightly", rustc_diagnostic_item = "GenericTypeVisitable")] pub unsafe trait GenericTypeVisitable<V> {     fn generic_visit_with(&self, visitor: &mut V); }@@ -116,6 +117,12 @@ unsafe impl<V, T: GenericTypeVisitable<V>> GenericTypeVisitable<V> for [T] {     } } +unsafe impl<V, T: GenericTypeVisitable<V>, const N: usize> GenericTypeVisitable<V> for [T; N] {+    fn generic_visit_with(&self, visitor: &mut V) {+        self.iter().for_each(|it| it.generic_visit_with(visitor));+    }+}+ unsafe impl<V, T: GenericTypeVisitable<V>, Ix: Idx> GenericTypeVisitable<V> for IndexVec<Ix, T> {     fn generic_visit_with(&self, visitor: &mut V) {         self.iter().for_each(|it| it.generic_visit_with(visitor));@@ -172,57 +179,10 @@ unsafe impl<V, T: GenericTypeVisitable<V>, S: GenericTypeVisitable<V>> GenericTy     } } -macro_rules! trivial_impls {-    ( $($ty:ty),* $(,)? ) => {-        $(-            unsafe impl<V>-                GenericTypeVisitable<V> for $ty-            {-                fn generic_visit_with(&self, _visitor: &mut V) {}-            }-        )*-    };-}- unsafe impl<T: ?Sized, V> GenericTypeVisitable<V> for std::marker::PhantomData<T> {     fn generic_visit_with(&self, _visitor: &mut V) {} } -trivial_impls!(-    (),-    rustc_ast_ir::Mutability,-    bool,-    i8,-    i16,-    i32,-    i64,-    i128,-    isize,-    u8,-    u16,-    u32,-    u64,-    u128,-    usize,-    crate::ClausePolarity,-    crate::BoundConstness,-    crate::DebruijnIndex,-    crate::solve::Certainty,-    crate::UniverseIndex,-    crate::BoundVar,-    crate::InferTy,-    crate::IntTy,-    crate::UintTy,-    crate::FloatTy,-    crate::InferConst,-    crate::RegionVid,-    rustc_hash::FxBuildHasher,-    crate::TypeFlags,-    crate::solve::GoalSource,-    crate::solve::VisibleForLeakCheck,-    rustc_abi::ExternAbi,-);- // SAFETY: `FnSigKind` is a packed representation, therefore visiting its fields doesn't make sense unsafe impl<I: Interner, V> GenericTypeVisitable<V> for crate::FnSigKind<I> {     fn generic_visit_with(&self, _visitor: &mut V) {}
compiler/rustc_type_ir/src/lib.rs7 + / 2 −
@@ -27,6 +27,12 @@ #![allow(rustc::usage_of_type_ir_traits)] #![cfg_attr(feature = "nightly", allow(internal_features))] #![cfg_attr(feature = "nightly", feature(associated_type_defaults, rustc_attrs, negative_impls))]+// NOTE: the outer `cfg_attr` is only there so that r-a doesn't see the nightly-only `bootstrap`+// in the inner `cfg_attr`. When the latter is removed, the former can be removed as well.+#![cfg_attr(+    feature = "nightly",+    cfg_attr(not(bootstrap), deny(rustc::missing_generic_type_visitable_derive))+)] // tidy-alphabetical-end  extern crate self as rustc_type_ir;@@ -105,7 +111,6 @@ pub use predicate::*; pub use predicate_kind::*; pub use region_kind::*; pub use rustc_ast_ir::{FloatTy, IntTy, Movability, Mutability, Pinnedness, UintTy};-use rustc_type_ir_macros::GenericTypeVisitable; #[cfg(feature = "nightly")] pub use serialize::*; pub use sty::*;@@ -251,7 +256,7 @@ pub fn debug_bound_var<T: std::fmt::Write>(     } } -#[derive(Copy, Clone, PartialEq, Eq, Hash, GenericTypeVisitable)]+#[derive(Copy, Clone, PartialEq, Eq, Hash)] #[cfg_attr(feature = "nightly", derive(Decodable, Encodable, StableHash))] #[cfg_attr(feature = "nightly", rustc_pass_by_value)] pub enum Variance {
compiler/rustc_type_ir/src/macros.rs22 + / 1 −
@@ -31,6 +31,12 @@ macro_rules! TrivialTypeTraversalImpls {                     <F::Result as $crate::VisitorResult>::output()                 }             }++            // NOTE: this deliberately avoids adding an `I: Interner` generic, because that would+            // allow creating a trivial impl for arena-allocating types, which would be incorrect.+            unsafe impl<V> $crate::GenericTypeVisitable<V> for $ty {+                fn generic_visit_with(&self, _visitor: &mut V) {}+            }         )+     }; }@@ -44,21 +50,36 @@ macro_rules! TrivialTypeTraversalImpls { TrivialTypeTraversalImpls! {     (),     bool,-    usize,+    i8,+    i16,+    i32,+    i64,+    i128,+    isize,     u8,     u16,     u32,     u64,+    usize,     // tidy-alphabetical-start     crate::BoundConstness,+    crate::BoundVar,     crate::ClausePolarity,     crate::DebruijnIndex,+    crate::FloatTy,+    crate::InferConst,+    crate::InferTy,+    crate::IntTy,+    crate::RegionVid,+    crate::TypeFlags,+    crate::UintTy,     crate::UniverseIndex,     crate::Variance,     crate::solve::BuiltinImplSource,     crate::solve::Certainty,     crate::solve::GoalSource,     crate::solve::VisibleForLeakCheck,+    rustc_abi::ExternAbi,     rustc_ast_ir::Mutability,     // tidy-alphabetical-end }
compiler/rustc_type_ir/src/unnormalized.rs2 + / 2 −
@@ -3,7 +3,7 @@ use std::marker::PhantomData; use derive_where::derive_where; #[cfg(feature = "nightly")] use rustc_macros::StableHash_NoContext;-use rustc_type_ir_macros::TypeVisitable_Generic;+use rustc_type_ir_macros::{GenericTypeVisitable, TypeVisitable_Generic};  use crate::fold::{FallibleTypeFolder, TypeFoldable, TypeFolder}; use crate::inherent::*;@@ -30,7 +30,7 @@ use crate::{ /// [here]: https://rust-lang.zulipchat.com/#narrow/channel/364551-t-types.2Ftrait-system-refactor/topic/Eager.20normalization.2C.20ahoy.21/with/582996293 #[derive_where(Clone, Copy, PartialOrd, PartialEq, Eq, Hash, Debug; T)] #[cfg_attr(feature = "nightly", derive(StableHash_NoContext))]-#[derive(TypeVisitable_Generic)]+#[derive(TypeVisitable_Generic, GenericTypeVisitable)] pub struct Unnormalized<I: Interner, T> {     value: T,     #[type_visitable(ignore)]
compiler/rustc_type_ir/src/visit.rs1 + / 0 −
@@ -59,6 +59,7 @@ use crate::{self as ty, Const, Interner, PredicateProxy, Region, TypeFlags}; /// /// To implement this conveniently, use the derive macro located in /// `rustc_macros`.+#[cfg_attr(feature = "nightly", rustc_diagnostic_item = "TypeVisitable")] pub trait TypeVisitable<I: Interner>: fmt::Debug {     /// The entry point for visiting. To visit a value `t` with a visitor `v`     /// call: `t.visit_with(v)`.
tests/ui-fulldeps/internal-lints/missing-generic-type-visitable-derive.rsadded130 + / 0 −
@@ -0,0 +1,130 @@+//@ compile-flags: -Z unstable-options+// FIXME(bootstrap): Remove the ignore once we promote the next beta.+//@ ignore-stage1++#![crate_type = "lib"]+#![feature(rustc_private)]+#![deny(rustc::missing_generic_type_visitable_derive)]++extern crate rustc_type_ir;+extern crate rustc_type_ir_macros;++use rustc_type_ir::{GenericTypeVisitable, TypeVisitable};+use rustc_type_ir::{Interner, TypeVisitor, VisitorResult};+use rustc_type_ir_macros::{GenericTypeVisitable, TypeVisitable_Generic};+use std::{fmt::Debug, hash::Hash};++// Should lint: `TypeVisitable` is implemented, but `GenericTypeVisitable` isn't++#[derive(Debug, TypeVisitable_Generic)]+struct DerivesRegular;+//~^ ERROR missing `#[derive(GenericTypeVisitable)]` on a type which implements `TypeVisitable`++#[derive(Debug)]+struct ImplsRegular;+//~^ ERROR missing `#[derive(GenericTypeVisitable)]` on a type which implements `TypeVisitable`++impl<I: Interner> TypeVisitable<I> for ImplsRegular {+    fn visit_with<F: TypeVisitor<I>>(&self, _f: &mut F) -> F::Result {+        F::Result::output()+    }+}++// Same but with an enum++#[derive(Debug, TypeVisitable_Generic)]+enum EnumDerivesRegular {}+//~^ ERROR missing `#[derive(GenericTypeVisitable)]` on a type which implements `TypeVisitable`++// Same but the type is generic, over the interner++#[derive(TypeVisitable_Generic)]+struct GenericOverIDerivesRegular<I: Interner>(I::Clause);+//~^ ERROR missing `#[derive(GenericTypeVisitable)]` on a type which implements `TypeVisitable`++// manually hand-rolled `#[derive_where(Debug; I: Interner)]`+impl<I: Interner> std::fmt::Debug for GenericOverIDerivesRegular<I> {+    fn fmt(&self, _f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {+        todo!()+    }+}++// Same but the type is generic, over an unrelated type++#[derive(TypeVisitable_Generic)]+struct GenericOverTDerivesRegular<T>([T; 67]);+//~^ ERROR missing `#[derive(GenericTypeVisitable)]` on a type which implements `TypeVisitable`++// manually hand-rolled `#[derive(Debug)]`, to avoid the spurious bound on `T`+impl<T> Debug for GenericOverTDerivesRegular<T> {+    fn fmt(&self, _f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {+        todo!()+    }+}++// Should not lint: both traits are implemented++#[derive(Debug, TypeVisitable_Generic, GenericTypeVisitable)]+struct DerivesBoth;++#[derive(Debug, GenericTypeVisitable)]+struct ImplsAndDerives;++impl<I: Interner> TypeVisitable<I> for ImplsAndDerives {+    fn visit_with<F: TypeVisitor<I>>(&self, _f: &mut F) -> F::Result {+        F::Result::output()+    }+}++#[derive(Debug, TypeVisitable_Generic)]+struct DerivesAndImpls;++unsafe impl<V> GenericTypeVisitable<V> for DerivesAndImpls {+    fn generic_visit_with(&self, _visitor: &mut V) {}+}++#[derive(Debug)]+struct ImplsBoth;++impl<I: Interner> TypeVisitable<I> for ImplsBoth {+    fn visit_with<F: TypeVisitor<I>>(&self, _f: &mut F) -> F::Result {+        F::Result::output()+    }+}++unsafe impl<V> GenericTypeVisitable<V> for ImplsBoth {+    fn generic_visit_with(&self, _visitor: &mut V) {}+}++// Should not lint: `TypeVisitable` isn't implemented, so `GenericTypeVisitable` doesn't need to be+// either++struct ImplsNone;++// Should not lint: `GenericTypeVisitable` is implemented but `TypeVisitable` isn't -- weird, but+// okay for the purposes of this lint++#[derive(GenericTypeVisitable)]+struct DerivesGeneric;++struct ImplsGeneric;++unsafe impl<V> GenericTypeVisitable<V> for ImplsGeneric {+    fn generic_visit_with(&self, _visitor: &mut V) {}+}++// FIXME(ada4a): this will currently lint, and suggest adding a derive, even though ther's already+// one present. The actual problem is that the item has a _field_ that doesn't implement the trait,+// which makes the impl generated by the derive on this item unusable due to unsatisfied bounds.+//+// There are two ways of solving this:+// - in the lint, emit a different suggestion -- namely, to add a derive to the problematic field's+//   type -- if we see that the linted type already has a derive.+// - change the derive to add _assertions_ that the bound holds for each field type, and not just+//   trait bounds on the impl.+#[derive(Debug)]+#[derive(TypeVisitable_Generic, GenericTypeVisitable)]+struct FieldDoesntImplGeneric {+    //~^ ERROR missing `#[derive(GenericTypeVisitable)]` on a type which implements `TypeVisitable`+    field: ImplsRegular,+}
tests/ui-fulldeps/internal-lints/missing-generic-type-visitable-derive.stderradded65 + / 0 −
@@ -0,0 +1,65 @@+error: missing `#[derive(GenericTypeVisitable)]` on a type which implements `TypeVisitable`+  --> $DIR/missing-generic-type-visitable-derive.rs:20:1+   |+LL | struct DerivesRegular;+   | ^^^^^^^^^^^^^^^^^^^^^^+   |+   = note: `GenericTypeVisitable` is rust-analyzer's version of `TypeVisitable`; it should be implemented whenever the latter is+   = help: mark the type with `#[derive(GenericTypeVisitable)]`+   = help: or, if `GenericTypeVisitable` is derived, check whether all fields implement `GenericTypeVisitable`+note: the lint level is defined here+  --> $DIR/missing-generic-type-visitable-derive.rs:7:9+   |+LL | #![deny(rustc::missing_generic_type_visitable_derive)]+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^++error: missing `#[derive(GenericTypeVisitable)]` on a type which implements `TypeVisitable`+  --> $DIR/missing-generic-type-visitable-derive.rs:24:1+   |+LL | struct ImplsRegular;+   | ^^^^^^^^^^^^^^^^^^^^+   |+   = help: mark the type with `#[derive(GenericTypeVisitable)]`+   = help: or, if `GenericTypeVisitable` is derived, check whether all fields implement `GenericTypeVisitable`++error: missing `#[derive(GenericTypeVisitable)]` on a type which implements `TypeVisitable`+  --> $DIR/missing-generic-type-visitable-derive.rs:36:1+   |+LL | enum EnumDerivesRegular {}+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^+   |+   = help: mark the type with `#[derive(GenericTypeVisitable)]`+   = help: or, if `GenericTypeVisitable` is derived, check whether all fields implement `GenericTypeVisitable`++error: missing `#[derive(GenericTypeVisitable)]` on a type which implements `TypeVisitable`+  --> $DIR/missing-generic-type-visitable-derive.rs:42:1+   |+LL | struct GenericOverIDerivesRegular<I: Interner>(I::Clause);+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^+   |+   = help: mark the type with `#[derive(GenericTypeVisitable)]`+   = help: or, if `GenericTypeVisitable` is derived, check whether all fields implement `GenericTypeVisitable`++error: missing `#[derive(GenericTypeVisitable)]` on a type which implements `TypeVisitable`+  --> $DIR/missing-generic-type-visitable-derive.rs:55:1+   |+LL | struct GenericOverTDerivesRegular<T>([T; 67]);+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^+   |+   = help: mark the type with `#[derive(GenericTypeVisitable)]`+   = help: or, if `GenericTypeVisitable` is derived, check whether all fields implement `GenericTypeVisitable`++error: missing `#[derive(GenericTypeVisitable)]` on a type which implements `TypeVisitable`+  --> $DIR/missing-generic-type-visitable-derive.rs:127:1+   |+LL | / struct FieldDoesntImplGeneric {+LL | |+LL | |     field: ImplsRegular,+LL | | }+   | |_^+   |+   = help: mark the type with `#[derive(GenericTypeVisitable)]`+   = help: or, if `GenericTypeVisitable` is derived, check whether all fields implement `GenericTypeVisitable`++error: aborting due to 6 previous errors+