lint incoherent inherent impls

This commit is contained in:
Lukas Wirth 2023-01-20 23:09:35 +01:00
parent c15335c8b0
commit f34b2469bd
12 changed files with 284 additions and 51 deletions

View File

@ -35,6 +35,7 @@ pub struct FunctionData {
pub visibility: RawVisibility,
pub abi: Option<Interned<str>>,
pub legacy_const_generics_indices: Box<[u32]>,
pub rustc_allow_incoherent_impl: bool,
flags: FnFlags,
}
@ -84,13 +85,14 @@ pub(crate) fn fn_data_query(db: &dyn DefDatabase, func: FunctionId) -> Arc<Funct
}
}
let legacy_const_generics_indices = item_tree
.attrs(db, krate, ModItem::from(loc.id.value).into())
let attrs = item_tree.attrs(db, krate, ModItem::from(loc.id.value).into());
let legacy_const_generics_indices = attrs
.by_key("rustc_legacy_const_generics")
.tt_values()
.next()
.map(parse_rustc_legacy_const_generics)
.unwrap_or_default();
let rustc_allow_incoherent_impl = attrs.by_key("rustc_allow_incoherent_impl").exists();
Arc::new(FunctionData {
name: func.name.clone(),
@ -108,6 +110,7 @@ pub(crate) fn fn_data_query(db: &dyn DefDatabase, func: FunctionId) -> Arc<Funct
abi: func.abi.clone(),
legacy_const_generics_indices,
flags,
rustc_allow_incoherent_impl,
})
}
@ -171,6 +174,7 @@ pub struct TypeAliasData {
pub visibility: RawVisibility,
pub is_extern: bool,
pub rustc_has_incoherent_inherent_impls: bool,
pub rustc_allow_incoherent_impl: bool,
/// Bounds restricting the type alias itself (eg. `type Ty: Bound;` in a trait or impl).
pub bounds: Vec<Interned<TypeBound>>,
}
@ -189,10 +193,14 @@ pub(crate) fn type_alias_data_query(
item_tree[typ.visibility].clone()
};
let rustc_has_incoherent_inherent_impls = item_tree
.attrs(db, loc.container.module(db).krate(), ModItem::from(loc.id.value).into())
.by_key("rustc_has_incoherent_inherent_impls")
.exists();
let attrs = item_tree.attrs(
db,
loc.container.module(db).krate(),
ModItem::from(loc.id.value).into(),
);
let rustc_has_incoherent_inherent_impls =
attrs.by_key("rustc_has_incoherent_inherent_impls").exists();
let rustc_allow_incoherent_impl = attrs.by_key("rustc_allow_incoherent_impl").exists();
Arc::new(TypeAliasData {
name: typ.name.clone(),
@ -200,6 +208,7 @@ pub(crate) fn type_alias_data_query(
visibility,
is_extern: matches!(loc.container, ItemContainerId::ExternBlockId(_)),
rustc_has_incoherent_inherent_impls,
rustc_allow_incoherent_impl,
bounds: typ.bounds.to_vec(),
})
}
@ -450,6 +459,7 @@ pub struct ConstData {
pub name: Option<Name>,
pub type_ref: Interned<TypeRef>,
pub visibility: RawVisibility,
pub rustc_allow_incoherent_impl: bool,
}
impl ConstData {
@ -463,10 +473,16 @@ pub(crate) fn const_data_query(db: &dyn DefDatabase, konst: ConstId) -> Arc<Cons
item_tree[konst.visibility].clone()
};
let rustc_allow_incoherent_impl = item_tree
.attrs(db, loc.container.module(db).krate(), ModItem::from(loc.id.value).into())
.by_key("rustc_allow_incoherent_impl")
.exists();
Arc::new(ConstData {
name: konst.name.clone(),
type_ref: konst.type_ref.clone(),
visibility,
rustc_allow_incoherent_impl,
})
}
}

View File

@ -120,6 +120,8 @@ pub struct DefMap {
registered_tools: Vec<SmolStr>,
/// Unstable features of Rust enabled with `#![feature(A, B)]`.
unstable_features: FxHashSet<SmolStr>,
/// #[rustc_coherence_is_core]
rustc_coherence_is_core: bool,
edition: Edition,
recursion_limit: Option<u32>,
@ -292,6 +294,7 @@ fn empty(krate: CrateId, edition: Edition, module_data: ModuleData) -> DefMap {
registered_tools: Vec::new(),
unstable_features: FxHashSet::default(),
diagnostics: Vec::new(),
rustc_coherence_is_core: false,
}
}
@ -325,6 +328,10 @@ pub fn is_unstable_feature_enabled(&self, feature: &str) -> bool {
self.unstable_features.contains(feature)
}
pub fn is_rustc_coherence_is_core(&self) -> bool {
self.rustc_coherence_is_core
}
pub fn root(&self) -> LocalModuleId {
self.root
}
@ -337,7 +344,7 @@ pub fn proc_macro_loading_error(&self) -> Option<&str> {
self.proc_macro_loading_error.as_deref()
}
pub(crate) fn krate(&self) -> CrateId {
pub fn krate(&self) -> CrateId {
self.krate
}
@ -502,6 +509,7 @@ fn shrink_to_fit(&mut self) {
krate: _,
prelude: _,
root: _,
rustc_coherence_is_core: _,
} = self;
extern_prelude.shrink_to_fit();

View File

@ -296,6 +296,11 @@ fn seed_with_top_level(&mut self) {
continue;
}
if attr_name.as_text().as_deref() == Some("rustc_coherence_is_core") {
self.def_map.rustc_coherence_is_core = true;
continue;
}
if *attr_name == hir_expand::name![feature] {
let features =
attr.parse_path_comma_token_tree().into_iter().flatten().filter_map(

View File

@ -11,3 +11,9 @@
},
unsafe_check::{missing_unsafe, unsafe_expressions, UnsafeExpr},
};
#[derive(Debug, PartialEq, Eq)]
pub struct IncoherentImpl {
pub file_id: hir_expand::HirFileId,
pub impl_: syntax::AstPtr<syntax::ast::Impl>,
}

View File

@ -19,7 +19,7 @@
use crate::{
autoderef::{self, AutoderefKind},
db::HirDatabase,
from_foreign_def_id,
from_chalk_trait_id, from_foreign_def_id,
infer::{unify::InferenceTable, Adjust, Adjustment, AutoBorrow, OverloadedDeref, PointerCast},
primitive::{FloatTy, IntTy, UintTy},
static_lifetime, to_chalk_trait_id,
@ -266,11 +266,12 @@ pub fn all_impls(&self) -> impl Iterator<Item = ImplId> + '_ {
#[derive(Debug, Eq, PartialEq)]
pub struct InherentImpls {
map: FxHashMap<TyFingerprint, Vec<ImplId>>,
invalid_impls: Vec<ImplId>,
}
impl InherentImpls {
pub(crate) fn inherent_impls_in_crate_query(db: &dyn HirDatabase, krate: CrateId) -> Arc<Self> {
let mut impls = Self { map: FxHashMap::default() };
let mut impls = Self { map: FxHashMap::default(), invalid_impls: Vec::default() };
let crate_def_map = db.crate_def_map(krate);
impls.collect_def_map(db, &crate_def_map);
@ -283,7 +284,7 @@ pub(crate) fn inherent_impls_in_block_query(
db: &dyn HirDatabase,
block: BlockId,
) -> Option<Arc<Self>> {
let mut impls = Self { map: FxHashMap::default() };
let mut impls = Self { map: FxHashMap::default(), invalid_impls: Vec::default() };
if let Some(block_def_map) = db.block_def_map(block) {
impls.collect_def_map(db, &block_def_map);
impls.shrink_to_fit();
@ -306,11 +307,17 @@ fn collect_def_map(&mut self, db: &dyn HirDatabase, def_map: &DefMap) {
}
let self_ty = db.impl_self_ty(impl_id);
let fp = TyFingerprint::for_inherent_impl(self_ty.skip_binders());
if let Some(fp) = fp {
self.map.entry(fp).or_default().push(impl_id);
let self_ty = self_ty.skip_binders();
match is_inherent_impl_coherent(db, def_map, &data, self_ty) {
true => {
// `fp` should only be `None` in error cases (either erroneous code or incomplete name resolution)
if let Some(fp) = TyFingerprint::for_inherent_impl(self_ty) {
self.map.entry(fp).or_default().push(impl_id);
}
}
false => self.invalid_impls.push(impl_id),
}
// `fp` should only be `None` in error cases (either erroneous code or incomplete name resolution)
}
// To better support custom derives, collect impls in all unnamed const items.
@ -334,6 +341,10 @@ pub fn for_self_ty(&self, self_ty: &Ty) -> &[ImplId] {
pub fn all_impls(&self) -> impl Iterator<Item = ImplId> + '_ {
self.map.values().flat_map(|v| v.iter().copied())
}
pub fn invalid_impls(&self) -> &[ImplId] {
&self.invalid_impls
}
}
pub(crate) fn incoherent_inherent_impl_crates(
@ -775,6 +786,90 @@ fn find_matching_impl(
}
}
fn is_inherent_impl_coherent(
db: &dyn HirDatabase,
def_map: &DefMap,
impl_data: &ImplData,
self_ty: &Ty,
) -> bool {
let self_ty = self_ty.kind(Interner);
let impl_allowed = match self_ty {
TyKind::Tuple(_, _)
| TyKind::FnDef(_, _)
| TyKind::Array(_, _)
| TyKind::Never
| TyKind::Raw(_, _)
| TyKind::Ref(_, _, _)
| TyKind::Slice(_)
| TyKind::Str
| TyKind::Scalar(_) => def_map.is_rustc_coherence_is_core(),
&TyKind::Adt(AdtId(adt), _) => adt.module(db.upcast()).krate() == def_map.krate(),
// FIXME: Factor out the principal trait fetching into a function
TyKind::Dyn(it) => it
.bounds
.skip_binders()
.interned()
.get(0)
.and_then(|b| match b.skip_binders() {
crate::WhereClause::Implemented(trait_ref) => Some(trait_ref),
_ => None,
})
.map_or(false, |trait_ref| {
from_chalk_trait_id(trait_ref.trait_id).module(db.upcast()).krate()
== def_map.krate()
}),
_ => true,
};
impl_allowed || {
let rustc_has_incoherent_inherent_impls = match self_ty {
TyKind::Tuple(_, _)
| TyKind::FnDef(_, _)
| TyKind::Array(_, _)
| TyKind::Never
| TyKind::Raw(_, _)
| TyKind::Ref(_, _, _)
| TyKind::Slice(_)
| TyKind::Str
| TyKind::Scalar(_) => true,
&TyKind::Adt(AdtId(adt), _) => match adt {
hir_def::AdtId::StructId(it) => {
db.struct_data(it).rustc_has_incoherent_inherent_impls
}
hir_def::AdtId::UnionId(it) => {
db.union_data(it).rustc_has_incoherent_inherent_impls
}
hir_def::AdtId::EnumId(it) => db.enum_data(it).rustc_has_incoherent_inherent_impls,
},
// FIXME: Factor out the principal trait fetching into a function
TyKind::Dyn(it) => it
.bounds
.skip_binders()
.interned()
.get(0)
.and_then(|b| match b.skip_binders() {
crate::WhereClause::Implemented(trait_ref) => Some(trait_ref),
_ => None,
})
.map_or(false, |trait_ref| {
db.trait_data(from_chalk_trait_id(trait_ref.trait_id))
.rustc_has_incoherent_inherent_impls
}),
_ => false,
};
rustc_has_incoherent_inherent_impls
&& !impl_data.items.is_empty()
&& impl_data.items.iter().copied().all(|assoc| match assoc {
AssocItemId::FunctionId(it) => db.function_data(it).rustc_allow_incoherent_impl,
AssocItemId::ConstId(it) => db.const_data(it).rustc_allow_incoherent_impl,
AssocItemId::TypeAliasId(it) => db.type_alias_data(it).rustc_allow_incoherent_impl,
})
}
}
pub fn iterate_path_candidates(
ty: &Canonical<Ty>,
db: &dyn HirDatabase,

View File

@ -9,6 +9,7 @@ fn infer_slice_method() {
check_types(
r#"
impl<T> [T] {
#[rustc_allow_incoherent_impl]
fn foo(&self) -> T {
loop {}
}
@ -35,6 +36,7 @@ fn test() {
//- /lib.rs crate:other_crate
mod foo {
impl f32 {
#[rustc_allow_incoherent_impl]
pub fn foo(self) -> f32 { 0. }
}
}
@ -47,6 +49,7 @@ fn infer_array_inherent_impl() {
check_types(
r#"
impl<T, const N: usize> [T; N] {
#[rustc_allow_incoherent_impl]
fn foo(&self) -> T {
loop {}
}
@ -1437,6 +1440,7 @@ fn resolve_const_generic_array_methods() {
r#"
#[lang = "array"]
impl<T, const N: usize> [T; N] {
#[rustc_allow_incoherent_impl]
pub fn map<F, U>(self, f: F) -> [U; N]
where
F: FnMut(T) -> U,
@ -1445,6 +1449,7 @@ fn resolve_const_generic_array_methods() {
#[lang = "slice"]
impl<T> [T] {
#[rustc_allow_incoherent_impl]
pub fn map<F, U>(self, f: F) -> &[U]
where
F: FnMut(T) -> U,
@ -1468,6 +1473,7 @@ fn resolve_const_generic_method() {
#[lang = "array"]
impl<T, const N: usize> [T; N] {
#[rustc_allow_incoherent_impl]
pub fn my_map<F, U, const X: usize>(self, f: F, c: Const<X>) -> [U; X]
where
F: FnMut(T) -> U,
@ -1476,6 +1482,7 @@ fn resolve_const_generic_method() {
#[lang = "slice"]
impl<T> [T] {
#[rustc_allow_incoherent_impl]
pub fn my_map<F, const X: usize, U>(self, f: F, c: Const<X>) -> &[U]
where
F: FnMut(T) -> U,
@ -1874,14 +1881,14 @@ fn incoherent_impls() {
pub struct Box<T>(T);
use core::error::Error;
#[rustc_allow_incoherent_impl]
impl dyn Error {
#[rustc_allow_incoherent_impl]
pub fn downcast<T: Error + 'static>(self: Box<Self>) -> Result<Box<T>, Box<dyn Error>> {
loop {}
}
}
#[rustc_allow_incoherent_impl]
impl dyn Error + Send {
#[rustc_allow_incoherent_impl]
/// Attempts to downcast the box to a concrete type.
pub fn downcast<T: Error + 'static>(self: Box<Self>) -> Result<Box<T>, Box<dyn Error + Send>> {
let err: Box<dyn Error> = self;

View File

@ -1116,21 +1116,22 @@ fn test(a: A) {
fn infer_inherent_method_str() {
check_infer(
r#"
#[lang = "str"]
impl str {
fn foo(&self) -> i32 {}
}
#![rustc_coherence_is_core]
#[lang = "str"]
impl str {
fn foo(&self) -> i32 {}
}
fn test() {
"foo".foo();
}
"#,
fn test() {
"foo".foo();
}
"#,
expect![[r#"
39..43 'self': &str
52..54 '{}': i32
68..88 '{ ...o(); }': ()
74..79 '"foo"': &str
74..85 '"foo".foo()': i32
67..71 'self': &str
80..82 '{}': i32
96..116 '{ ...o(); }': ()
102..107 '"foo"': &str
102..113 '"foo".foo()': i32
"#]],
);
}
@ -2640,6 +2641,7 @@ impl<T> [T] {}
#[lang = "slice_alloc"]
impl<T> [T] {
#[rustc_allow_incoherent_impl]
pub fn into_vec<A: Allocator>(self: Box<Self, A>) -> Vec<T, A> {
unimplemented!()
}
@ -2655,22 +2657,22 @@ trait B{}
impl B for Astruct {}
"#,
expect![[r#"
569..573 'self': Box<[T], A>
602..634 '{ ... }': Vec<T, A>
648..761 '{ ...t]); }': ()
658..661 'vec': Vec<i32, Global>
664..679 '<[_]>::into_vec': fn into_vec<i32, Global>(Box<[i32], Global>) -> Vec<i32, Global>
664..691 '<[_]>:...1i32])': Vec<i32, Global>
680..690 'box [1i32]': Box<[i32; 1], Global>
684..690 '[1i32]': [i32; 1]
685..689 '1i32': i32
701..702 'v': Vec<Box<dyn B, Global>, Global>
722..739 '<[_]> ...to_vec': fn into_vec<Box<dyn B, Global>, Global>(Box<[Box<dyn B, Global>], Global>) -> Vec<Box<dyn B, Global>, Global>
722..758 '<[_]> ...ruct])': Vec<Box<dyn B, Global>, Global>
740..757 'box [b...truct]': Box<[Box<dyn B, Global>; 1], Global>
744..757 '[box Astruct]': [Box<dyn B, Global>; 1]
745..756 'box Astruct': Box<Astruct, Global>
749..756 'Astruct': Astruct
604..608 'self': Box<[T], A>
637..669 '{ ... }': Vec<T, A>
683..796 '{ ...t]); }': ()
693..696 'vec': Vec<i32, Global>
699..714 '<[_]>::into_vec': fn into_vec<i32, Global>(Box<[i32], Global>) -> Vec<i32, Global>
699..726 '<[_]>:...1i32])': Vec<i32, Global>
715..725 'box [1i32]': Box<[i32; 1], Global>
719..725 '[1i32]': [i32; 1]
720..724 '1i32': i32
736..737 'v': Vec<Box<dyn B, Global>, Global>
757..774 '<[_]> ...to_vec': fn into_vec<Box<dyn B, Global>, Global>(Box<[Box<dyn B, Global>], Global>) -> Vec<Box<dyn B, Global>, Global>
757..793 '<[_]> ...ruct])': Vec<Box<dyn B, Global>, Global>
775..792 'box [b...truct]': Box<[Box<dyn B, Global>; 1], Global>
779..792 '[box Astruct]': [Box<dyn B, Global>; 1]
780..791 'box Astruct': Box<Astruct, Global>
784..791 'Astruct': Astruct
"#]],
)
}

View File

@ -3,6 +3,8 @@
//!
//! This probably isn't the best way to do this -- ideally, diagnostics should
//! be expressed in terms of hir types themselves.
pub use hir_ty::diagnostics::{IncoherentImpl, IncorrectCase};
use base_db::CrateId;
use cfg::{CfgExpr, CfgOptions};
use either::Either;
@ -35,6 +37,7 @@ fn from(d: $diag) -> AnyDiagnostic {
InactiveCode,
IncorrectCase,
InvalidDeriveTarget,
IncoherentImpl,
MacroError,
MalformedDerive,
MismatchedArgCount,
@ -220,5 +223,3 @@ pub struct NeedMut {
pub struct UnusedMut {
pub local: Local,
}
pub use hir_ty::diagnostics::IncorrectCase;

View File

@ -85,10 +85,10 @@
pub use crate::{
attrs::{HasAttrs, Namespace},
diagnostics::{
AnyDiagnostic, BreakOutsideOfLoop, ExpectedFunction, InactiveCode, IncorrectCase,
InvalidDeriveTarget, MacroError, MalformedDerive, MismatchedArgCount, MissingFields,
MissingMatchArms, MissingUnsafe, NeedMut, NoSuchField, PrivateAssocItem, PrivateField,
ReplaceFilterMapNextWithFindMap, TypeMismatch, UnimplementedBuiltinMacro,
AnyDiagnostic, BreakOutsideOfLoop, ExpectedFunction, InactiveCode, IncoherentImpl,
IncorrectCase, InvalidDeriveTarget, MacroError, MalformedDerive, MismatchedArgCount,
MissingFields, MissingMatchArms, MissingUnsafe, NeedMut, NoSuchField, PrivateAssocItem,
PrivateField, ReplaceFilterMapNextWithFindMap, TypeMismatch, UnimplementedBuiltinMacro,
UnresolvedExternCrate, UnresolvedField, UnresolvedImport, UnresolvedMacroCall,
UnresolvedMethodCall, UnresolvedModule, UnresolvedProcMacro, UnusedMut,
},
@ -604,11 +604,23 @@ pub fn diagnostics(self, db: &dyn HirDatabase, acc: &mut Vec<AnyDiagnostic>) {
}
}
let inherent_impls = db.inherent_impls_in_crate(self.id.krate());
for impl_def in self.impl_defs(db) {
for diag in db.impl_data_with_diagnostics(impl_def.id).1.iter() {
emit_def_diagnostic(db, acc, diag);
}
if inherent_impls.invalid_impls().contains(&impl_def.id) {
let loc = impl_def.id.lookup(db.upcast());
let tree = loc.id.item_tree(db.upcast());
let node = &tree[loc.id.value];
let file_id = loc.id.file_id();
let ast_id_map = db.ast_id_map(file_id);
acc.push(IncoherentImpl { impl_: ast_id_map.get(node.ast_id()), file_id }.into())
}
for item in impl_def.items(db) {
let def: DefWithBody = match item {
AssocItem::Function(it) => it.into(),

View File

@ -0,0 +1,77 @@
use hir::InFile;
use crate::{Diagnostic, DiagnosticsContext, Severity};
// Diagnostic: incoherent-impl
//
// This diagnostic is triggered if the targe type of an impl is from a foreign crate.
pub(crate) fn incoherent_impl(ctx: &DiagnosticsContext<'_>, d: &hir::IncoherentImpl) -> Diagnostic {
Diagnostic::new(
"incoherent-impl",
format!("cannot define inherent `impl` for foreign type"),
ctx.sema.diagnostics_display_range(InFile::new(d.file_id, d.impl_.clone().into())).range,
)
.severity(Severity::Error)
}
#[cfg(test)]
mod change_case {
use crate::tests::check_diagnostics;
#[test]
fn primitive() {
check_diagnostics(
r#"
impl bool {}
//^^^^^^^^^^^^ error: cannot define inherent `impl` for foreign type
"#,
);
}
#[test]
fn primitive_rustc_allow_incoherent_impl() {
check_diagnostics(
r#"
impl bool {
#[rustc_allow_incoherent_impl]
fn falsch(self) -> Self { false }
}
"#,
);
}
#[test]
fn rustc_allow_incoherent_impl() {
check_diagnostics(
r#"
//- /lib.rs crate:foo
#[rustc_has_incoherent_inherent_impls]
pub struct S;
//- /main.rs crate:main deps:foo
impl foo::S {
#[rustc_allow_incoherent_impl]
fn func(self) {}
}
"#,
);
check_diagnostics(
r#"
//- /lib.rs crate:foo
pub struct S;
//- /main.rs crate:main deps:foo
impl foo::S { #[rustc_allow_incoherent_impl] fn func(self) {} }
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: cannot define inherent `impl` for foreign type
"#,
);
check_diagnostics(
r#"
//- /lib.rs crate:foo
#[rustc_has_incoherent_inherent_impls]
pub struct S;
//- /main.rs crate:main deps:foo
impl foo::S { fn func(self) {} }
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: cannot define inherent `impl` for foreign type
"#,
);
}
}

View File

@ -29,6 +29,7 @@ mod handlers {
pub(crate) mod break_outside_of_loop;
pub(crate) mod expected_function;
pub(crate) mod inactive_code;
pub(crate) mod incoherent_impl;
pub(crate) mod incorrect_case;
pub(crate) mod invalid_derive_target;
pub(crate) mod macro_error;
@ -254,6 +255,7 @@ pub fn diagnostics(
AnyDiagnostic::BreakOutsideOfLoop(d) => handlers::break_outside_of_loop::break_outside_of_loop(&ctx, &d),
AnyDiagnostic::ExpectedFunction(d) => handlers::expected_function::expected_function(&ctx, &d),
AnyDiagnostic::IncorrectCase(d) => handlers::incorrect_case::incorrect_case(&ctx, &d),
AnyDiagnostic::IncoherentImpl(d) => handlers::incoherent_impl::incoherent_impl(&ctx, &d),
AnyDiagnostic::MacroError(d) => handlers::macro_error::macro_error(&ctx, &d),
AnyDiagnostic::MalformedDerive(d) => handlers::malformed_derive::malformed_derive(&ctx, &d),
AnyDiagnostic::MismatchedArgCount(d) => handlers::mismatched_arg_count::mismatched_arg_count(&ctx, &d),

View File

@ -44,6 +44,8 @@
//! try: infallible
//! unsize: sized
#![rustc_coherence_is_core]
pub mod marker {
// region:sized
#[lang = "sized"]