Skip to content

Commit 13a63e5

Browse files
committed
lint incoherent inherent impls
1 parent 3a72713 commit 13a63e5

File tree

9 files changed

+240
-18
lines changed

9 files changed

+240
-18
lines changed

crates/hir-def/src/data.rs

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ pub struct FunctionData {
3535
pub visibility: RawVisibility,
3636
pub abi: Option<Interned<str>>,
3737
pub legacy_const_generics_indices: Box<[u32]>,
38+
pub rustc_allow_incoherent_impl: bool,
3839
flags: FnFlags,
3940
}
4041

@@ -84,13 +85,14 @@ impl FunctionData {
8485
}
8586
}
8687

87-
let legacy_const_generics_indices = item_tree
88-
.attrs(db, krate, ModItem::from(loc.id.value).into())
88+
let attrs = item_tree.attrs(db, krate, ModItem::from(loc.id.value).into());
89+
let legacy_const_generics_indices = attrs
8990
.by_key("rustc_legacy_const_generics")
9091
.tt_values()
9192
.next()
9293
.map(parse_rustc_legacy_const_generics)
9394
.unwrap_or_default();
95+
let rustc_allow_incoherent_impl = attrs.by_key("rustc_allow_incoherent_impl").exists();
9496

9597
Arc::new(FunctionData {
9698
name: func.name.clone(),
@@ -108,6 +110,7 @@ impl FunctionData {
108110
abi: func.abi.clone(),
109111
legacy_const_generics_indices,
110112
flags,
113+
rustc_allow_incoherent_impl,
111114
})
112115
}
113116

@@ -171,6 +174,7 @@ pub struct TypeAliasData {
171174
pub visibility: RawVisibility,
172175
pub is_extern: bool,
173176
pub rustc_has_incoherent_inherent_impls: bool,
177+
pub rustc_allow_incoherent_impl: bool,
174178
/// Bounds restricting the type alias itself (eg. `type Ty: Bound;` in a trait or impl).
175179
pub bounds: Vec<Interned<TypeBound>>,
176180
}
@@ -189,17 +193,22 @@ impl TypeAliasData {
189193
item_tree[typ.visibility].clone()
190194
};
191195

192-
let rustc_has_incoherent_inherent_impls = item_tree
193-
.attrs(db, loc.container.module(db).krate(), ModItem::from(loc.id.value).into())
194-
.by_key("rustc_has_incoherent_inherent_impls")
195-
.exists();
196+
let attrs = item_tree.attrs(
197+
db,
198+
loc.container.module(db).krate(),
199+
ModItem::from(loc.id.value).into(),
200+
);
201+
let rustc_has_incoherent_inherent_impls =
202+
attrs.by_key("rustc_has_incoherent_inherent_impls").exists();
203+
let rustc_allow_incoherent_impl = attrs.by_key("rustc_allow_incoherent_impl").exists();
196204

197205
Arc::new(TypeAliasData {
198206
name: typ.name.clone(),
199207
type_ref: typ.type_ref.clone(),
200208
visibility,
201209
is_extern: matches!(loc.container, ItemContainerId::ExternBlockId(_)),
202210
rustc_has_incoherent_inherent_impls,
211+
rustc_allow_incoherent_impl,
203212
bounds: typ.bounds.to_vec(),
204213
})
205214
}
@@ -441,6 +450,7 @@ pub struct ConstData {
441450
pub name: Option<Name>,
442451
pub type_ref: Interned<TypeRef>,
443452
pub visibility: RawVisibility,
453+
pub rustc_allow_incoherent_impl: bool,
444454
}
445455

446456
impl ConstData {
@@ -454,10 +464,16 @@ impl ConstData {
454464
item_tree[konst.visibility].clone()
455465
};
456466

467+
let rustc_allow_incoherent_impl = item_tree
468+
.attrs(db, loc.container.module(db).krate(), ModItem::from(loc.id.value).into())
469+
.by_key("rustc_allow_incoherent_impl")
470+
.exists();
471+
457472
Arc::new(ConstData {
458473
name: konst.name.clone(),
459474
type_ref: konst.type_ref.clone(),
460475
visibility,
476+
rustc_allow_incoherent_impl,
461477
})
462478
}
463479
}

crates/hir-def/src/nameres.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,8 @@ pub struct DefMap {
120120
registered_tools: Vec<SmolStr>,
121121
/// Unstable features of Rust enabled with `#![feature(A, B)]`.
122122
unstable_features: FxHashSet<SmolStr>,
123+
/// #[rustc_coherence_is_core]
124+
rustc_coherence_is_core: bool,
123125

124126
edition: Edition,
125127
recursion_limit: Option<u32>,
@@ -292,6 +294,7 @@ impl DefMap {
292294
registered_tools: Vec::new(),
293295
unstable_features: FxHashSet::default(),
294296
diagnostics: Vec::new(),
297+
rustc_coherence_is_core: false,
295298
}
296299
}
297300

@@ -325,6 +328,10 @@ impl DefMap {
325328
self.unstable_features.contains(feature)
326329
}
327330

331+
pub fn is_rustc_coherence_is_core(&self) -> bool {
332+
self.rustc_coherence_is_core
333+
}
334+
328335
pub fn root(&self) -> LocalModuleId {
329336
self.root
330337
}
@@ -337,7 +344,7 @@ impl DefMap {
337344
self.proc_macro_loading_error.as_deref()
338345
}
339346

340-
pub(crate) fn krate(&self) -> CrateId {
347+
pub fn krate(&self) -> CrateId {
341348
self.krate
342349
}
343350

@@ -502,6 +509,7 @@ impl DefMap {
502509
krate: _,
503510
prelude: _,
504511
root: _,
512+
rustc_coherence_is_core: _,
505513
} = self;
506514

507515
extern_prelude.shrink_to_fit();

crates/hir-def/src/nameres/collector.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,11 @@ impl DefCollector<'_> {
296296
continue;
297297
}
298298

299+
if attr_name.as_text().as_deref() == Some("rustc_coherence_is_core") {
300+
self.def_map.rustc_coherence_is_core = true;
301+
continue;
302+
}
303+
299304
if *attr_name == hir_expand::name![feature] {
300305
let features =
301306
attr.parse_path_comma_token_tree().into_iter().flatten().filter_map(

crates/hir-ty/src/diagnostics.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,9 @@ pub use crate::diagnostics::{
1111
},
1212
unsafe_check::{missing_unsafe, unsafe_expressions, UnsafeExpr},
1313
};
14+
15+
#[derive(Debug, PartialEq, Eq)]
16+
pub struct IncoherentImpl {
17+
pub file_id: hir_expand::HirFileId,
18+
pub impl_: syntax::AstPtr<syntax::ast::Impl>,
19+
}

crates/hir-ty/src/method_resolution.rs

Lines changed: 102 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use stdx::never;
1818
use crate::{
1919
autoderef::{self, AutoderefKind},
2020
db::HirDatabase,
21-
from_foreign_def_id,
21+
from_chalk_trait_id, from_foreign_def_id,
2222
infer::{unify::InferenceTable, Adjust, Adjustment, AutoBorrow, OverloadedDeref, PointerCast},
2323
primitive::{FloatTy, IntTy, UintTy},
2424
static_lifetime, to_chalk_trait_id,
@@ -265,11 +265,12 @@ impl TraitImpls {
265265
#[derive(Debug, Eq, PartialEq)]
266266
pub struct InherentImpls {
267267
map: FxHashMap<TyFingerprint, Vec<ImplId>>,
268+
invalid_impls: Vec<ImplId>,
268269
}
269270

270271
impl InherentImpls {
271272
pub(crate) fn inherent_impls_in_crate_query(db: &dyn HirDatabase, krate: CrateId) -> Arc<Self> {
272-
let mut impls = Self { map: FxHashMap::default() };
273+
let mut impls = Self { map: FxHashMap::default(), invalid_impls: Vec::default() };
273274

274275
let crate_def_map = db.crate_def_map(krate);
275276
impls.collect_def_map(db, &crate_def_map);
@@ -282,7 +283,7 @@ impl InherentImpls {
282283
db: &dyn HirDatabase,
283284
block: BlockId,
284285
) -> Option<Arc<Self>> {
285-
let mut impls = Self { map: FxHashMap::default() };
286+
let mut impls = Self { map: FxHashMap::default(), invalid_impls: Vec::default() };
286287
if let Some(block_def_map) = db.block_def_map(block) {
287288
impls.collect_def_map(db, &block_def_map);
288289
impls.shrink_to_fit();
@@ -305,11 +306,17 @@ impl InherentImpls {
305306
}
306307

307308
let self_ty = db.impl_self_ty(impl_id);
308-
let fp = TyFingerprint::for_inherent_impl(self_ty.skip_binders());
309-
if let Some(fp) = fp {
310-
self.map.entry(fp).or_default().push(impl_id);
309+
let self_ty = self_ty.skip_binders();
310+
311+
match is_inherent_impl_coherent(db, def_map, &data, self_ty) {
312+
true => {
313+
// `fp` should only be `None` in error cases (either erroneous code or incomplete name resolution)
314+
if let Some(fp) = TyFingerprint::for_inherent_impl(self_ty) {
315+
self.map.entry(fp).or_default().push(impl_id);
316+
}
317+
}
318+
false => self.invalid_impls.push(impl_id),
311319
}
312-
// `fp` should only be `None` in error cases (either erroneous code or incomplete name resolution)
313320
}
314321

315322
// To better support custom derives, collect impls in all unnamed const items.
@@ -333,6 +340,10 @@ impl InherentImpls {
333340
pub fn all_impls(&self) -> impl Iterator<Item = ImplId> + '_ {
334341
self.map.values().flat_map(|v| v.iter().copied())
335342
}
343+
344+
pub fn invalid_impls(&self) -> &[ImplId] {
345+
&self.invalid_impls
346+
}
336347
}
337348

338349
pub(crate) fn incoherent_inherent_impl_crates(
@@ -759,6 +770,90 @@ fn find_matching_impl(
759770
}
760771
}
761772

773+
fn is_inherent_impl_coherent(
774+
db: &dyn HirDatabase,
775+
def_map: &DefMap,
776+
impl_data: &ImplData,
777+
self_ty: &Ty,
778+
) -> bool {
779+
let self_ty = self_ty.kind(Interner);
780+
let impl_allowed = match self_ty {
781+
TyKind::Tuple(_, _)
782+
| TyKind::FnDef(_, _)
783+
| TyKind::Array(_, _)
784+
| TyKind::Never
785+
| TyKind::Raw(_, _)
786+
| TyKind::Ref(_, _, _)
787+
| TyKind::Slice(_)
788+
| TyKind::Str
789+
| TyKind::Scalar(_) => def_map.is_rustc_coherence_is_core(),
790+
791+
&TyKind::Adt(AdtId(adt), _) => adt.module(db.upcast()).krate() == def_map.krate(),
792+
// FIXME: Factor out the principal trait fetching into a function
793+
TyKind::Dyn(it) => it
794+
.bounds
795+
.skip_binders()
796+
.interned()
797+
.get(0)
798+
.and_then(|b| match b.skip_binders() {
799+
crate::WhereClause::Implemented(trait_ref) => Some(trait_ref),
800+
_ => None,
801+
})
802+
.map_or(false, |trait_ref| {
803+
from_chalk_trait_id(trait_ref.trait_id).module(db.upcast()).krate()
804+
== def_map.krate()
805+
}),
806+
807+
_ => true,
808+
};
809+
impl_allowed || {
810+
let rustc_has_incoherent_inherent_impls = match self_ty {
811+
TyKind::Tuple(_, _)
812+
| TyKind::FnDef(_, _)
813+
| TyKind::Array(_, _)
814+
| TyKind::Never
815+
| TyKind::Raw(_, _)
816+
| TyKind::Ref(_, _, _)
817+
| TyKind::Slice(_)
818+
| TyKind::Str
819+
| TyKind::Scalar(_) => true,
820+
821+
&TyKind::Adt(AdtId(adt), _) => match adt {
822+
hir_def::AdtId::StructId(it) => {
823+
db.struct_data(it).rustc_has_incoherent_inherent_impls
824+
}
825+
hir_def::AdtId::UnionId(it) => {
826+
db.union_data(it).rustc_has_incoherent_inherent_impls
827+
}
828+
hir_def::AdtId::EnumId(it) => db.enum_data(it).rustc_has_incoherent_inherent_impls,
829+
},
830+
// FIXME: Factor out the principal trait fetching into a function
831+
TyKind::Dyn(it) => it
832+
.bounds
833+
.skip_binders()
834+
.interned()
835+
.get(0)
836+
.and_then(|b| match b.skip_binders() {
837+
crate::WhereClause::Implemented(trait_ref) => Some(trait_ref),
838+
_ => None,
839+
})
840+
.map_or(false, |trait_ref| {
841+
db.trait_data(from_chalk_trait_id(trait_ref.trait_id))
842+
.rustc_has_incoherent_inherent_impls
843+
}),
844+
845+
_ => false,
846+
};
847+
rustc_has_incoherent_inherent_impls
848+
&& !impl_data.items.is_empty()
849+
&& impl_data.items.iter().copied().all(|assoc| match assoc {
850+
AssocItemId::FunctionId(it) => db.function_data(it).rustc_allow_incoherent_impl,
851+
AssocItemId::ConstId(it) => db.const_data(it).rustc_allow_incoherent_impl,
852+
AssocItemId::TypeAliasId(it) => db.type_alias_data(it).rustc_allow_incoherent_impl,
853+
})
854+
}
855+
}
856+
762857
pub fn iterate_path_candidates(
763858
ty: &Canonical<Ty>,
764859
db: &dyn HirDatabase,

crates/hir/src/diagnostics.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ diagnostics![
3434
InactiveCode,
3535
IncorrectCase,
3636
InvalidDeriveTarget,
37+
IncoherentImpl,
3738
MacroError,
3839
MalformedDerive,
3940
MismatchedArgCount,
@@ -184,4 +185,4 @@ pub struct TypeMismatch {
184185
pub actual: Type,
185186
}
186187

187-
pub use hir_ty::diagnostics::IncorrectCase;
188+
pub use hir_ty::diagnostics::{IncoherentImpl, IncorrectCase};

crates/hir/src/lib.rs

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,9 @@ use crate::db::{DefDatabase, HirDatabase};
8383
pub use crate::{
8484
attrs::{HasAttrs, Namespace},
8585
diagnostics::{
86-
AnyDiagnostic, BreakOutsideOfLoop, InactiveCode, IncorrectCase, InvalidDeriveTarget,
87-
MacroError, MalformedDerive, MismatchedArgCount, MissingFields, MissingMatchArms,
88-
MissingUnsafe, NoSuchField, PrivateAssocItem, PrivateField,
86+
AnyDiagnostic, BreakOutsideOfLoop, InactiveCode, IncoherentImpl, IncorrectCase,
87+
InvalidDeriveTarget, MacroError, MalformedDerive, MismatchedArgCount, MissingFields,
88+
MissingMatchArms, MissingUnsafe, NoSuchField, PrivateAssocItem, PrivateField,
8989
ReplaceFilterMapNextWithFindMap, TypeMismatch, UnimplementedBuiltinMacro,
9090
UnresolvedExternCrate, UnresolvedImport, UnresolvedMacroCall, UnresolvedModule,
9191
UnresolvedProcMacro,
@@ -579,11 +579,23 @@ impl Module {
579579
}
580580
}
581581

582+
let inherent_impls = db.inherent_impls_in_crate(self.id.krate());
583+
582584
for impl_def in self.impl_defs(db) {
583585
for diag in db.impl_data_with_diagnostics(impl_def.id).1.iter() {
584586
emit_def_diagnostic(db, acc, diag);
585587
}
586588

589+
if inherent_impls.invalid_impls().contains(&impl_def.id) {
590+
let loc = impl_def.id.lookup(db.upcast());
591+
let tree = loc.id.item_tree(db.upcast());
592+
let node = &tree[loc.id.value];
593+
let file_id = loc.id.file_id();
594+
let ast_id_map = db.ast_id_map(file_id);
595+
596+
acc.push(IncoherentImpl { impl_: ast_id_map.get(node.ast_id()), file_id }.into())
597+
}
598+
587599
for item in impl_def.items(db) {
588600
let def: DefWithBody = match item {
589601
AssocItem::Function(it) => it.into(),

0 commit comments

Comments
 (0)