Skip to content

Commit

Permalink
Auto merge of #94963 - lcnr:inherent-impls-std, r=oli-obk,m-ou-se
Browse files Browse the repository at this point in the history
allow arbitrary inherent impls for builtin types in core

Part of rust-lang/compiler-team#487. Slightly adjusted after some talks with `@m-ou-se` about the requirements of `t-libs-api`.

This adds a crate attribute `#![rustc_coherence_is_core]` which allows arbitrary impls for builtin types in core.

For other library crates impls for builtin types should be avoided if possible. We do have to allow the existing stable impls however. To prevent us from accidentally adding more of these in the future, there is a second attribute `#[rustc_allow_incoherent_impl]` which has to be added to **all impl items**. This only supports impls for builtin types but can easily be extended to additional types in a future PR.

This implementation does not check for overlaps in these impls. Perfectly checking that requires us to check the coherence of these incoherent impls in every crate, as two distinct dependencies may add overlapping methods. It should be easy enough to detect if it goes wrong and the attribute is only intended for use inside of std.

The first two commits are mostly unrelated cleanups.
  • Loading branch information
bors committed Mar 30, 2022
2 parents e50ff9b + 46340f2 commit 3e75146
Show file tree
Hide file tree
Showing 66 changed files with 703 additions and 851 deletions.
29 changes: 4 additions & 25 deletions compiler/rustc_error_codes/src/error_codes/E0118.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ enum, union, or trait object.
Erroneous code example:

```compile_fail,E0118
impl (u8, u8) { // error: no nominal type found for inherent implementation
impl fn(u8) { // error: no nominal type found for inherent implementation
fn get_state(&self) -> String {
// ...
}
Expand All @@ -20,8 +20,8 @@ trait LiveLongAndProsper {
fn get_state(&self) -> String;
}
// and now you can implement it on (u8, u8)
impl LiveLongAndProsper for (u8, u8) {
// and now you can implement it on fn(u8)
impl LiveLongAndProsper for fn(u8) {
fn get_state(&self) -> String {
"He's dead, Jim!".to_owned()
}
Expand All @@ -33,32 +33,11 @@ For example, `NewType` is a newtype over `Foo` in `struct NewType(Foo)`.
Example:

```
struct TypeWrapper((u8, u8));
struct TypeWrapper(fn(u8));
impl TypeWrapper {
fn get_state(&self) -> String {
"Fascinating!".to_owned()
}
}
```

Instead of defining an inherent implementation on a reference, you could also
move the reference inside the implementation:

```compile_fail,E0118
struct Foo;
impl &Foo { // error: no nominal type found for inherent implementation
fn bar(self, other: Self) {}
}
```

becomes

```
struct Foo;
impl Foo {
fn bar(&self, other: &Self) {}
}
```
24 changes: 22 additions & 2 deletions compiler/rustc_error_codes/src/error_codes/E0390.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ struct Foo {
}
impl *mut Foo {}
// error: only a single inherent implementation marked with
// `#[lang = "mut_ptr"]` is allowed for the `*mut T` primitive
// error: cannot define inherent `impl` for primitive types
```

This isn't allowed, but using a trait to implement a method or constant
Expand All @@ -29,3 +28,24 @@ impl Bar for *mut Foo {
fn bar() {} // ok!
}
```

Instead of defining an inherent implementation on a reference, you could also
move the reference inside the implementation:

```compile_fail,E0390
struct Foo;
impl &Foo { // error: no nominal type found for inherent implementation
fn bar(self, other: Self) {}
}
```

becomes

```
struct Foo;
impl Foo {
fn bar(&self, other: &Self) {}
}
```
8 changes: 8 additions & 0 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,14 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
template!(Word), ErrorFollowing,
"#[rustc_pass_by_value] is used to mark types that must be passed by value instead of reference."
),
rustc_attr!(
rustc_coherence_is_core, AttributeType::CrateLevel, template!(Word), ErrorFollowing,
"#![rustc_coherence_is_core] allows inherent methods on builtin types, only intended to be used in `core`."
),
rustc_attr!(
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."
),
BuiltinAttribute {
name: sym::rustc_diagnostic_item,
type_: Normal,
Expand Down
30 changes: 0 additions & 30 deletions compiler/rustc_hir/src/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,36 +166,6 @@ pub fn extract(attrs: &[ast::Attribute]) -> Option<(Symbol, Span)> {

language_item_table! {
// Variant name, Name, Method name, Target Generic requirements;
Bool, sym::bool, bool_impl, Target::Impl, GenericRequirement::None;
Char, sym::char, char_impl, Target::Impl, GenericRequirement::None;
Str, sym::str, str_impl, Target::Impl, GenericRequirement::None;
Array, sym::array, array_impl, Target::Impl, GenericRequirement::None;
Slice, sym::slice, slice_impl, Target::Impl, GenericRequirement::None;
SliceU8, sym::slice_u8, slice_u8_impl, Target::Impl, GenericRequirement::None;
StrAlloc, sym::str_alloc, str_alloc_impl, Target::Impl, GenericRequirement::None;
SliceAlloc, sym::slice_alloc, slice_alloc_impl, Target::Impl, GenericRequirement::None;
SliceU8Alloc, sym::slice_u8_alloc, slice_u8_alloc_impl, Target::Impl, GenericRequirement::None;
ConstPtr, sym::const_ptr, const_ptr_impl, Target::Impl, GenericRequirement::None;
MutPtr, sym::mut_ptr, mut_ptr_impl, Target::Impl, GenericRequirement::None;
ConstSlicePtr, sym::const_slice_ptr, const_slice_ptr_impl, Target::Impl, GenericRequirement::None;
MutSlicePtr, sym::mut_slice_ptr, mut_slice_ptr_impl, Target::Impl, GenericRequirement::None;
I8, sym::i8, i8_impl, Target::Impl, GenericRequirement::None;
I16, sym::i16, i16_impl, Target::Impl, GenericRequirement::None;
I32, sym::i32, i32_impl, Target::Impl, GenericRequirement::None;
I64, sym::i64, i64_impl, Target::Impl, GenericRequirement::None;
I128, sym::i128, i128_impl, Target::Impl, GenericRequirement::None;
Isize, sym::isize, isize_impl, Target::Impl, GenericRequirement::None;
U8, sym::u8, u8_impl, Target::Impl, GenericRequirement::None;
U16, sym::u16, u16_impl, Target::Impl, GenericRequirement::None;
U32, sym::u32, u32_impl, Target::Impl, GenericRequirement::None;
U64, sym::u64, u64_impl, Target::Impl, GenericRequirement::None;
U128, sym::u128, u128_impl, Target::Impl, GenericRequirement::None;
Usize, sym::usize, usize_impl, Target::Impl, GenericRequirement::None;
F32, sym::f32, f32_impl, Target::Impl, GenericRequirement::None;
F64, sym::f64, f64_impl, Target::Impl, GenericRequirement::None;
F32Runtime, sym::f32_runtime, f32_runtime_impl, Target::Impl, GenericRequirement::None;
F64Runtime, sym::f64_runtime, f64_runtime_impl, Target::Impl, GenericRequirement::None;

Sized, sym::sized, sized_trait, Target::Trait, GenericRequirement::Exact(0);
Unsize, sym::unsize, unsize_trait, Target::Trait, GenericRequirement::Minimum(1);
/// Trait injected by `#[derive(PartialEq)]`, (i.e. "Partial EQ").
Expand Down
44 changes: 36 additions & 8 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ crate struct CrateMetadata {
/// FIXME: Used only from queries and can use query cache,
/// so pre-decoding can probably be avoided.
trait_impls: FxHashMap<(u32, DefIndex), Lazy<[(DefIndex, Option<SimplifiedType>)]>>,
/// Inherent impls which do not follow the normal coherence rules.
///
/// These can be introduced using either `#![rustc_coherence_is_core]`
/// or `#[rustc_allow_incoherent_impl]`.
incoherent_impls: FxHashMap<SimplifiedType, Lazy<[DefIndex]>>,
/// Proc macro descriptions for this crate, if it's a proc macro crate.
raw_proc_macros: Option<&'static [ProcMacro]>,
/// Source maps for code from the crate.
Expand Down Expand Up @@ -1028,11 +1033,13 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
}

/// Iterates over the language items in the given crate.
fn get_lang_items(self) -> impl Iterator<Item = (DefId, usize)> + 'a {
self.root
.lang_items
.decode(self)
.map(move |(def_index, index)| (self.local_def_id(def_index), index))
fn get_lang_items(self, tcx: TyCtxt<'tcx>) -> &'tcx [(DefId, usize)] {
tcx.arena.alloc_from_iter(
self.root
.lang_items
.decode(self)
.map(move |(def_index, index)| (self.local_def_id(def_index), index)),
)
}

/// Iterates over the diagnostic items in the given crate.
Expand Down Expand Up @@ -1327,17 +1334,32 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {

/// Decodes all trait impls in the crate (for rustdoc).
fn get_trait_impls(self) -> impl Iterator<Item = (DefId, DefId, Option<SimplifiedType>)> + 'a {
self.cdata.trait_impls.iter().flat_map(move |((trait_cnum_raw, trait_index), impls)| {
self.cdata.trait_impls.iter().flat_map(move |(&(trait_cnum_raw, trait_index), impls)| {
let trait_def_id = DefId {
krate: self.cnum_map[CrateNum::from_u32(*trait_cnum_raw)],
index: *trait_index,
krate: self.cnum_map[CrateNum::from_u32(trait_cnum_raw)],
index: trait_index,
};
impls.decode(self).map(move |(impl_index, simplified_self_ty)| {
(trait_def_id, self.local_def_id(impl_index), simplified_self_ty)
})
})
}

fn get_all_incoherent_impls(self) -> impl Iterator<Item = DefId> + 'a {
self.cdata
.incoherent_impls
.values()
.flat_map(move |impls| impls.decode(self).map(move |idx| self.local_def_id(idx)))
}

fn get_incoherent_impls(self, tcx: TyCtxt<'tcx>, simp: SimplifiedType) -> &'tcx [DefId] {
if let Some(impls) = self.cdata.incoherent_impls.get(&simp) {
tcx.arena.alloc_from_iter(impls.decode(self).map(|idx| self.local_def_id(idx)))
} else {
&[]
}
}

fn get_implementations_of_trait(
self,
tcx: TyCtxt<'tcx>,
Expand Down Expand Up @@ -1754,6 +1776,11 @@ impl CrateMetadata {
.decode((&blob, sess))
.map(|trait_impls| (trait_impls.trait_id, trait_impls.impls))
.collect();
let incoherent_impls = root
.incoherent_impls
.decode((&blob, sess))
.map(|incoherent_impls| (incoherent_impls.self_ty, incoherent_impls.impls))
.collect();
let alloc_decoding_state =
AllocDecodingState::new(root.interpret_alloc_index.decode(&blob).collect());
let dependencies = Lock::new(cnum_map.iter().cloned().collect());
Expand All @@ -1766,6 +1793,7 @@ impl CrateMetadata {
blob,
root,
trait_impls,
incoherent_impls,
raw_proc_macros,
source_map_import_info: OnceCell::new(),
def_path_hash_map,
Expand Down
39 changes: 27 additions & 12 deletions compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,30 +81,42 @@ macro_rules! provide {
// small trait to work around different signature queries all being defined via
// the macro above.
trait IntoArgs {
fn into_args(self) -> (DefId, DefId);
type Other;
fn into_args(self) -> (DefId, Self::Other);
}

impl IntoArgs for DefId {
fn into_args(self) -> (DefId, DefId) {
(self, self)
type Other = ();
fn into_args(self) -> (DefId, ()) {
(self, ())
}
}

impl IntoArgs for CrateNum {
fn into_args(self) -> (DefId, DefId) {
(self.as_def_id(), self.as_def_id())
type Other = ();
fn into_args(self) -> (DefId, ()) {
(self.as_def_id(), ())
}
}

impl IntoArgs for (CrateNum, DefId) {
type Other = DefId;
fn into_args(self) -> (DefId, DefId) {
(self.0.as_def_id(), self.1)
}
}

impl<'tcx> IntoArgs for ty::InstanceDef<'tcx> {
fn into_args(self) -> (DefId, DefId) {
(self.def_id(), self.def_id())
type Other = ();
fn into_args(self) -> (DefId, ()) {
(self.def_id(), ())
}
}

impl IntoArgs for (CrateNum, SimplifiedType) {
type Other = SimplifiedType;
fn into_args(self) -> (DefId, SimplifiedType) {
(self.0.as_def_id(), self.1)
}
}

Expand Down Expand Up @@ -199,6 +211,7 @@ provide! { <'tcx> tcx, def_id, other, cdata,

traits_in_crate => { tcx.arena.alloc_from_iter(cdata.get_traits()) }
implementations_of_trait => { cdata.get_implementations_of_trait(tcx, other) }
crate_incoherent_impls => { cdata.get_incoherent_impls(tcx, other) }

dep_kind => {
let r = *cdata.dep_kind.lock();
Expand All @@ -210,7 +223,7 @@ provide! { <'tcx> tcx, def_id, other, cdata,
tcx.arena.alloc_slice(&result)
}
defined_lib_features => { cdata.get_lib_features(tcx) }
defined_lang_items => { tcx.arena.alloc_from_iter(cdata.get_lang_items()) }
defined_lang_items => { cdata.get_lang_items(tcx) }
diagnostic_items => { cdata.get_diagnostic_items() }
missing_lang_items => { cdata.get_missing_lang_items(tcx) }

Expand Down Expand Up @@ -371,7 +384,6 @@ pub(in crate::rmeta) fn provide(providers: &mut Providers) {
.alloc_slice(&CStore::from_tcx(tcx).crate_dependencies_in_postorder(LOCAL_CRATE))
},
crates: |tcx, ()| tcx.arena.alloc_from_iter(CStore::from_tcx(tcx).crates_untracked()),

..*providers
};
}
Expand Down Expand Up @@ -511,9 +523,12 @@ impl CStore {
self.get_crate_data(cnum).get_inherent_impls()
}

/// Decodes all lang items in the crate (for rustdoc).
pub fn lang_items_untracked(&self, cnum: CrateNum) -> impl Iterator<Item = DefId> + '_ {
self.get_crate_data(cnum).get_lang_items().map(|(def_id, _)| def_id)
/// Decodes all incoherent inherent impls in the crate (for rustdoc).
pub fn incoherent_impls_in_crate_untracked(
&self,
cnum: CrateNum,
) -> impl Iterator<Item = DefId> + '_ {
self.get_crate_data(cnum).get_all_incoherent_impls()
}
}

Expand Down
38 changes: 35 additions & 3 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ use crate::rmeta::def_path_hash_map::DefPathHashMapRef;
use crate::rmeta::table::{FixedSizeEncoding, TableBuilder};
use crate::rmeta::*;

use rustc_data_structures::fingerprint::Fingerprint;
use rustc_data_structures::fx::{FxHashMap, FxIndexSet};
use rustc_data_structures::stable_hasher::StableHasher;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_data_structures::sync::{join, par_iter, Lrc, ParallelIterator};
use rustc_hir as hir;
use rustc_hir::def::DefKind;
Expand Down Expand Up @@ -578,6 +579,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
}

fn encode_crate_root(&mut self) -> Lazy<CrateRoot<'tcx>> {
let tcx = self.tcx;
let mut i = self.position();

// Encode the crate deps
Expand Down Expand Up @@ -623,8 +625,9 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
let impls = self.encode_impls();
let impls_bytes = self.position() - i;

let tcx = self.tcx;

i = self.position();
let incoherent_impls = self.encode_incoherent_impls();
let incoherent_impls_bytes = self.position() - i;
// Encode MIR.
i = self.position();
self.encode_mir();
Expand Down Expand Up @@ -734,6 +737,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
source_map,
traits,
impls,
incoherent_impls,
exported_symbols,
interpret_alloc_index,
tables,
Expand Down Expand Up @@ -762,6 +766,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
eprintln!(" source_map bytes: {}", source_map_bytes);
eprintln!(" traits bytes: {}", traits_bytes);
eprintln!(" impls bytes: {}", impls_bytes);
eprintln!("incoherent_impls bytes: {}", incoherent_impls_bytes);
eprintln!(" exp. symbols bytes: {}", exported_symbols_bytes);
eprintln!(" def-path table bytes: {}", def_path_table_bytes);
eprintln!(" def-path hashes bytes: {}", def_path_hash_map_bytes);
Expand Down Expand Up @@ -1813,6 +1818,33 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
self.lazy(&all_impls)
}

fn encode_incoherent_impls(&mut self) -> Lazy<[IncoherentImpls]> {
debug!("EncodeContext::encode_traits_and_impls()");
empty_proc_macro!(self);
let tcx = self.tcx;
let mut ctx = tcx.create_stable_hashing_context();
let mut all_impls: Vec<_> = tcx.crate_inherent_impls(()).incoherent_impls.iter().collect();
all_impls.sort_by_cached_key(|&(&simp, _)| {
let mut hasher = StableHasher::new();
simp.hash_stable(&mut ctx, &mut hasher);
hasher.finish::<Fingerprint>();
});
let all_impls: Vec<_> = all_impls
.into_iter()
.map(|(&simp, impls)| {
let mut impls: Vec<_> =
impls.into_iter().map(|def_id| def_id.local_def_index).collect();
impls.sort_by_cached_key(|&local_def_index| {
tcx.hir().def_path_hash(LocalDefId { local_def_index })
});

IncoherentImpls { self_ty: simp, impls: self.lazy(impls) }
})
.collect();

self.lazy(&all_impls)
}

// Encodes all symbols exported from this crate into the metadata.
//
// This pass is seeded off the reachability list calculated in the
Expand Down

0 comments on commit 3e75146

Please sign in to comment.