Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

generalize "incoherent impls" impl for user defined types #96520

Merged
merged 4 commits into from
May 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,11 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
rustc_allow_incoherent_impl, AttributeType::Normal, template!(Word), ErrorFollowing,
"#[rustc_allow_incoherent_impl] has to be added to all impl items of an incoherent inherent impl."
),
rustc_attr!(
rustc_has_incoherent_inherent_impls, AttributeType::Normal, template!(Word), ErrorFollowing,
"#[rustc_has_incoherent_inherent_impls] allows the addition of incoherent inherent impls for \
the given type by annotating all impl items with #[rustc_allow_incoherent_impl]."
),
BuiltinAttribute {
name: sym::rustc_diagnostic_item,
type_: Normal,
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_hir/src/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,8 +327,6 @@ language_item_table! {
Range, sym::Range, range_struct, Target::Struct, GenericRequirement::None;
RangeToInclusive, sym::RangeToInclusive, range_to_inclusive_struct, Target::Struct, GenericRequirement::None;
RangeTo, sym::RangeTo, range_to_struct, Target::Struct, GenericRequirement::None;

CStr, sym::CStr, c_str, Target::Struct, GenericRequirement::None;
}

pub enum GenericRequirement {
Expand Down
28 changes: 27 additions & 1 deletion compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ impl CheckAttrVisitor<'_> {
sym::rustc_allow_incoherent_impl => {
self.check_allow_incoherent_impl(&attr, span, target)
}
sym::rustc_has_incoherent_inherent_impls => {
self.check_has_incoherent_inherent_impls(&attr, span, target)
}
sym::rustc_const_unstable
| sym::rustc_const_stable
| sym::unstable
Expand Down Expand Up @@ -1095,7 +1098,6 @@ impl CheckAttrVisitor<'_> {
}
}

/// Warns against some misuses of `#[pass_by_value]`
fn check_allow_incoherent_impl(&self, attr: &Attribute, span: Span, target: Target) -> bool {
match target {
Target::Method(MethodKind::Inherent) => true,
Expand All @@ -1113,6 +1115,30 @@ impl CheckAttrVisitor<'_> {
}
}

fn check_has_incoherent_inherent_impls(
&self,
attr: &Attribute,
span: Span,
target: Target,
) -> bool {
match target {
Target::Trait | Target::Struct | Target::Enum | Target::Union | Target::ForeignTy => {
true
}
_ => {
self.tcx
.sess
.struct_span_err(
attr.span,
"`rustc_has_incoherent_inherent_impls` attribute should be applied to types or traits.",
)
.span_label(span, "only adts, extern types and traits are supported")
.emit();
false
}
}
}

/// Warns against some misuses of `#[must_use]`
fn check_must_use(&self, hir_id: HirId, attr: &Attribute, span: Span, target: Target) -> bool {
let node = self.tcx.hir().get(hir_id);
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1187,6 +1187,7 @@ symbols! {
rustc_error,
rustc_evaluate_where_clauses,
rustc_expected_cgu_reuse,
rustc_has_incoherent_inherent_impls,
rustc_if_this_changed,
rustc_inherit_overflow_checks,
rustc_insignificant_dtor,
Expand Down
9 changes: 8 additions & 1 deletion compiler/rustc_typeck/src/check/method/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use rustc_span::def_id::LocalDefId;
use rustc_span::lev_distance::{
find_best_match_for_name_with_substrings, lev_distance_with_substrings,
};
use rustc_span::symbol::sym;
use rustc_span::{symbol::Ident, Span, Symbol, DUMMY_SP};
use rustc_trait_selection::autoderef::{self, Autoderef};
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
Expand Down Expand Up @@ -642,16 +643,22 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {

self.assemble_inherent_candidates_from_object(generalized_self_ty);
self.assemble_inherent_impl_candidates_for_type(p.def_id());
if self.tcx.has_attr(p.def_id(), sym::rustc_has_incoherent_inherent_impls) {
self.assemble_inherent_candidates_for_incoherent_ty(raw_self_ty);
}
}
ty::Adt(def, _) => {
let def_id = def.did();
self.assemble_inherent_impl_candidates_for_type(def_id);
if Some(def_id) == self.tcx.lang_items().c_str() {
if self.tcx.has_attr(def_id, sym::rustc_has_incoherent_inherent_impls) {
self.assemble_inherent_candidates_for_incoherent_ty(raw_self_ty);
}
}
ty::Foreign(did) => {
self.assemble_inherent_impl_candidates_for_type(did);
if self.tcx.has_attr(did, sym::rustc_has_incoherent_inherent_impls) {
self.assemble_inherent_candidates_for_incoherent_ty(raw_self_ty);
}
}
ty::Param(p) => {
self.assemble_inherent_candidates_from_param(p);
Expand Down
71 changes: 58 additions & 13 deletions compiler/rustc_typeck/src/coherence/inherent_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,13 @@ impl<'tcx> ItemLikeVisitor<'_> for InherentCollect<'tcx> {
let self_ty = self.tcx.type_of(item.def_id);
match *self_ty.kind() {
ty::Adt(def, _) => {
let def_id = def.did();
if !def_id.is_local() && Some(def_id) == self.tcx.lang_items().c_str() {
self.check_primitive_impl(item.def_id, self_ty, items, ty.span)
} else {
self.check_def_id(item, def_id);
}
self.check_def_id(item, self_ty, def.did());
}
ty::Foreign(did) => {
self.check_def_id(item, did);
self.check_def_id(item, self_ty, did);
}
ty::Dynamic(data, ..) if data.principal_def_id().is_some() => {
self.check_def_id(item, data.principal_def_id().unwrap());
self.check_def_id(item, self_ty, data.principal_def_id().unwrap());
}
ty::Dynamic(..) => {
struct_span_err!(
Expand Down Expand Up @@ -124,14 +119,67 @@ impl<'tcx> ItemLikeVisitor<'_> for InherentCollect<'tcx> {
fn visit_foreign_item(&mut self, _foreign_item: &hir::ForeignItem<'_>) {}
}

const INTO_CORE: &str = "consider moving this inherent impl into `core` if possible";
const INTO_DEFINING_CRATE: &str =
"consider moving this inherent impl into the crate defining the type if possible";
const ADD_ATTR_TO_TY: &str = "alternatively add `#[rustc_has_incoherent_inherent_impls]` to the type \
and `#[rustc_allow_incoherent_impl]` to the relevant impl items";
const ADD_ATTR: &str =
"alternatively add `#[rustc_allow_incoherent_impl]` to the relevant impl items";

impl<'tcx> InherentCollect<'tcx> {
fn check_def_id(&mut self, item: &hir::Item<'_>, def_id: DefId) {
fn check_def_id(&mut self, item: &hir::Item<'_>, self_ty: Ty<'tcx>, def_id: DefId) {
let impl_def_id = item.def_id;
if let Some(def_id) = def_id.as_local() {
// Add the implementation to the mapping from implementation to base
// type def ID, if there is a base type for this implementation and
// the implementation does not have any associated traits.
let vec = self.impls_map.inherent_impls.entry(def_id).or_default();
vec.push(item.def_id.to_def_id());
vec.push(impl_def_id.to_def_id());
return;
}

if self.tcx.features().rustc_attrs {
let hir::ItemKind::Impl(&hir::Impl { items, .. }) = item.kind else {
bug!("expected `impl` item: {:?}", item);
};

if !self.tcx.has_attr(def_id, sym::rustc_has_incoherent_inherent_impls) {
struct_span_err!(
self.tcx.sess,
item.span,
E0390,
"cannot define inherent `impl` for a type outside of the crate where the type is defined",
)
.help(INTO_DEFINING_CRATE)
.span_help(item.span, ADD_ATTR_TO_TY)
.emit();
return;
}

for impl_item in items {
if !self
.tcx
.has_attr(impl_item.id.def_id.to_def_id(), sym::rustc_allow_incoherent_impl)
{
struct_span_err!(
self.tcx.sess,
item.span,
E0390,
"cannot define inherent `impl` for a type outside of the crate where the type is defined",
)
.help(INTO_DEFINING_CRATE)
.span_help(impl_item.span, ADD_ATTR)
.emit();
return;
}
}

if let Some(simp) = simplify_type(self.tcx, self_ty, TreatParams::AsPlaceholders) {
self.impls_map.incoherent_impls.entry(simp).or_default().push(impl_def_id);
} else {
bug!("unexpected self type: {:?}", self_ty);
}
} else {
struct_span_err!(
self.tcx.sess,
Expand All @@ -153,9 +201,6 @@ impl<'tcx> InherentCollect<'tcx> {
items: &[hir::ImplItemRef],
span: Span,
) {
const INTO_CORE: &str = "consider moving this inherent impl into `core` if possible";
const ADD_ATTR: &str =
"alternatively add `#[rustc_allow_incoherent_impl]` to the relevant impl items";
if !self.tcx.hir().rustc_coherence_is_core() {
if self.tcx.features().rustc_attrs {
for item in items {
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/ffi/c_str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ use crate::str;
#[derive(Hash)]
#[cfg_attr(not(test), rustc_diagnostic_item = "CStr")]
#[unstable(feature = "core_c_str", issue = "94079")]
#[cfg_attr(not(bootstrap), lang = "CStr")]
#[cfg_attr(not(bootstrap), rustc_has_incoherent_inherent_impls)]
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
// FIXME:
// `fn from` in `impl From<&CStr> for Box<CStr>` current implementation relies
// on `CStr` being layout-compatible with `[u8]`.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#![feature(rustc_attrs)]

#[rustc_has_incoherent_inherent_impls]
pub struct StructWithAttr;
pub struct StructNoAttr;

#[rustc_has_incoherent_inherent_impls]
pub enum EnumWithAttr {}
pub enum EnumNoAttr {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// aux-build:extern-crate.rs
#![feature(rustc_attrs)]
extern crate extern_crate;

impl extern_crate::StructWithAttr {
//~^ ERROR cannot define inherent `impl` for a type outside of the crate
fn foo() {}
}
impl extern_crate::StructWithAttr {
#[rustc_allow_incoherent_impl]
fn bar() {}
}
impl extern_crate::StructNoAttr {
//~^ ERROR cannot define inherent `impl` for a type outside of the crate
fn foo() {}
}
impl extern_crate::StructNoAttr {
//~^ ERROR cannot define inherent `impl` for a type outside of the crate
#[rustc_allow_incoherent_impl]
fn bar() {}
}
impl extern_crate::EnumWithAttr {
//~^ ERROR cannot define inherent `impl` for a type outside of the crate
fn foo() {}
}
impl extern_crate::EnumWithAttr {
#[rustc_allow_incoherent_impl]
fn bar() {}
}
impl extern_crate::EnumNoAttr {
//~^ ERROR cannot define inherent `impl` for a type outside of the crate
fn foo() {}
}
impl extern_crate::EnumNoAttr {
//~^ ERROR cannot define inherent `impl` for a type outside of the crate
#[rustc_allow_incoherent_impl]
fn bar() {}
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
error[E0390]: cannot define inherent `impl` for a type outside of the crate where the type is defined
--> $DIR/needs-has-incoherent-impls.rs:5:1
|
LL | / impl extern_crate::StructWithAttr {
LL | |
LL | | fn foo() {}
LL | | }
| |_^
|
= help: consider moving this inherent impl into the crate defining the type if possible
help: alternatively add `#[rustc_allow_incoherent_impl]` to the relevant impl items
--> $DIR/needs-has-incoherent-impls.rs:7:5
|
LL | fn foo() {}
| ^^^^^^^^^^^

error[E0390]: cannot define inherent `impl` for a type outside of the crate where the type is defined
--> $DIR/needs-has-incoherent-impls.rs:13:1
|
LL | / impl extern_crate::StructNoAttr {
LL | |
LL | | fn foo() {}
LL | | }
| |_^
|
= help: consider moving this inherent impl into the crate defining the type if possible
help: alternatively add `#[rustc_has_incoherent_inherent_impls]` to the type and `#[rustc_allow_incoherent_impl]` to the relevant impl items
--> $DIR/needs-has-incoherent-impls.rs:13:1
|
LL | / impl extern_crate::StructNoAttr {
LL | |
LL | | fn foo() {}
LL | | }
| |_^

error[E0390]: cannot define inherent `impl` for a type outside of the crate where the type is defined
--> $DIR/needs-has-incoherent-impls.rs:17:1
|
LL | / impl extern_crate::StructNoAttr {
LL | |
LL | | #[rustc_allow_incoherent_impl]
LL | | fn bar() {}
LL | | }
| |_^
|
= help: consider moving this inherent impl into the crate defining the type if possible
help: alternatively add `#[rustc_has_incoherent_inherent_impls]` to the type and `#[rustc_allow_incoherent_impl]` to the relevant impl items
--> $DIR/needs-has-incoherent-impls.rs:17:1
|
LL | / impl extern_crate::StructNoAttr {
LL | |
LL | | #[rustc_allow_incoherent_impl]
LL | | fn bar() {}
LL | | }
| |_^

error[E0390]: cannot define inherent `impl` for a type outside of the crate where the type is defined
--> $DIR/needs-has-incoherent-impls.rs:22:1
|
LL | / impl extern_crate::EnumWithAttr {
LL | |
LL | | fn foo() {}
LL | | }
| |_^
|
= help: consider moving this inherent impl into the crate defining the type if possible
help: alternatively add `#[rustc_allow_incoherent_impl]` to the relevant impl items
--> $DIR/needs-has-incoherent-impls.rs:24:5
|
LL | fn foo() {}
| ^^^^^^^^^^^

error[E0390]: cannot define inherent `impl` for a type outside of the crate where the type is defined
--> $DIR/needs-has-incoherent-impls.rs:30:1
|
LL | / impl extern_crate::EnumNoAttr {
LL | |
LL | | fn foo() {}
LL | | }
| |_^
|
= help: consider moving this inherent impl into the crate defining the type if possible
help: alternatively add `#[rustc_has_incoherent_inherent_impls]` to the type and `#[rustc_allow_incoherent_impl]` to the relevant impl items
--> $DIR/needs-has-incoherent-impls.rs:30:1
|
LL | / impl extern_crate::EnumNoAttr {
LL | |
LL | | fn foo() {}
LL | | }
| |_^

error[E0390]: cannot define inherent `impl` for a type outside of the crate where the type is defined
--> $DIR/needs-has-incoherent-impls.rs:34:1
|
LL | / impl extern_crate::EnumNoAttr {
LL | |
LL | | #[rustc_allow_incoherent_impl]
LL | | fn bar() {}
LL | | }
| |_^
|
= help: consider moving this inherent impl into the crate defining the type if possible
help: alternatively add `#[rustc_has_incoherent_inherent_impls]` to the type and `#[rustc_allow_incoherent_impl]` to the relevant impl items
--> $DIR/needs-has-incoherent-impls.rs:34:1
|
LL | / impl extern_crate::EnumNoAttr {
LL | |
LL | | #[rustc_allow_incoherent_impl]
LL | | fn bar() {}
LL | | }
| |_^

error: aborting due to 6 previous errors

For more information about this error, try `rustc --explain E0390`.
Loading