rust-lang/rust · #163538
Add `rustc::missing_generic_type_visitable` lint
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+