Skip to content

Commit

Permalink
Rollup merge of #130764 - compiler-errors:inherent, r=estebank
Browse files Browse the repository at this point in the history
Separate collection of crate-local inherent impls from error tracking

#119895 changed the return type of the `crate_inherent_impls` query from `CrateInherentImpls` to `Result<CrateInherentImpls, ErrorGuaranteed>` to avoid needing to use the non-parallel-friendly `track_errors()` to track if an error was reporting from within the query... This was mostly fine until #121113, which stopped halting compilation when we hit an `Err(ErrorGuaranteed)` in the `crate_inherent_impls` query.

Thus we proceed onwards to typeck, and since a return type of `Result<CrateInherentImpls, ErrorGuaranteed>` means that the query can *either* return one of "the list inherent impls" or "error has been reported", later on when we want to assemble method or associated item candidates for inherent impls, we were just treating any `Err(ErrorGuaranteed)` return value as if Rust had no inherent impls defined anywhere at all! This leads to basically every inherent method call failing with an error, lol, which was reported in #127798.

This PR changes the `crate_inherent_impls` query to return `(CrateInherentImpls, Result<(), ErrorGuaranteed>)`, i.e. returning the inherent impls collected *and* whether an error was reported in the query itself. It firewalls the latter part of that query into a new `crate_inherent_impls_validity_check` just for the `ensure()` call.

This fixes #127798.
  • Loading branch information
tgross35 authored Sep 24, 2024
2 parents 1f52c07 + 28f6980 commit 3b45f8f
Show file tree
Hide file tree
Showing 31 changed files with 136 additions and 144 deletions.
34 changes: 18 additions & 16 deletions compiler/rustc_hir_analysis/src/coherence/inherent_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,36 +22,38 @@ use crate::errors;
pub(crate) fn crate_inherent_impls(
tcx: TyCtxt<'_>,
(): (),
) -> Result<&'_ CrateInherentImpls, ErrorGuaranteed> {
) -> (&'_ CrateInherentImpls, Result<(), ErrorGuaranteed>) {
let mut collect = InherentCollect { tcx, impls_map: Default::default() };

let mut res = Ok(());
for id in tcx.hir().items() {
res = res.and(collect.check_item(id));
}
res?;
Ok(tcx.arena.alloc(collect.impls_map))

(tcx.arena.alloc(collect.impls_map), res)
}

pub(crate) fn crate_incoherent_impls(
pub(crate) fn crate_inherent_impls_validity_check(
tcx: TyCtxt<'_>,
simp: SimplifiedType,
) -> Result<&[DefId], ErrorGuaranteed> {
let crate_map = tcx.crate_inherent_impls(())?;
Ok(tcx.arena.alloc_from_iter(
(): (),
) -> Result<(), ErrorGuaranteed> {
tcx.crate_inherent_impls(()).1
}

pub(crate) fn crate_incoherent_impls(tcx: TyCtxt<'_>, simp: SimplifiedType) -> &[DefId] {
let (crate_map, _) = tcx.crate_inherent_impls(());
tcx.arena.alloc_from_iter(
crate_map.incoherent_impls.get(&simp).unwrap_or(&Vec::new()).iter().map(|d| d.to_def_id()),
))
)
}

/// On-demand query: yields a vector of the inherent impls for a specific type.
pub(crate) fn inherent_impls(
tcx: TyCtxt<'_>,
ty_def_id: LocalDefId,
) -> Result<&[DefId], ErrorGuaranteed> {
let crate_map = tcx.crate_inherent_impls(())?;
Ok(match crate_map.inherent_impls.get(&ty_def_id) {
pub(crate) fn inherent_impls(tcx: TyCtxt<'_>, ty_def_id: LocalDefId) -> &[DefId] {
let (crate_map, _) = tcx.crate_inherent_impls(());
match crate_map.inherent_impls.get(&ty_def_id) {
Some(v) => &v[..],
None => &[],
})
}
}

struct InherentCollect<'tcx> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,7 @@ impl<'tcx> InherentOverlapChecker<'tcx> {
return Ok(());
}

let impls = self.tcx.inherent_impls(id.owner_id)?;

let impls = self.tcx.inherent_impls(id.owner_id);
let overlap_mode = OverlapMode::get(self.tcx, id.owner_id.to_def_id());

let impls_items = impls
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_hir_analysis/src/coherence/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,10 @@ fn enforce_empty_impls_for_marker_traits(

pub(crate) fn provide(providers: &mut Providers) {
use self::builtin::coerce_unsized_info;
use self::inherent_impls::{crate_incoherent_impls, crate_inherent_impls, inherent_impls};
use self::inherent_impls::{
crate_incoherent_impls, crate_inherent_impls, crate_inherent_impls_validity_check,
inherent_impls,
};
use self::inherent_impls_overlap::crate_inherent_impls_overlap_check;
use self::orphan::orphan_check_impl;

Expand All @@ -133,6 +136,7 @@ pub(crate) fn provide(providers: &mut Providers) {
crate_inherent_impls,
crate_incoherent_impls,
inherent_impls,
crate_inherent_impls_validity_check,
crate_inherent_impls_overlap_check,
coerce_unsized_info,
orphan_check_impl,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1028,7 +1028,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
..
}) = node
&& let Some(ty_def_id) = qself_ty.ty_def_id()
&& let Ok([inherent_impl]) = tcx.inherent_impls(ty_def_id)
&& let [inherent_impl] = tcx.inherent_impls(ty_def_id)
&& let name = format!("{ident2}_{ident3}")
&& let Some(ty::AssocItem { kind: ty::AssocKind::Fn, .. }) = tcx
.associated_items(inherent_impl)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1272,7 +1272,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
}

let candidates: Vec<_> = tcx
.inherent_impls(adt_did)?
.inherent_impls(adt_did)
.iter()
.filter_map(|&impl_| {
let (item, scope) =
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ pub fn check_crate(tcx: TyCtxt<'_>) {
let _ = tcx.ensure().coherent_trait(trait_def_id);
}
// these queries are executed for side-effects (error reporting):
let _ = tcx.ensure().crate_inherent_impls(());
let _ = tcx.ensure().crate_inherent_impls_validity_check(());
let _ = tcx.ensure().crate_inherent_impls_overlap_check(());
});

Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2126,7 +2126,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.tcx
.inherent_impls(def_id)
.into_iter()
.flatten()
.flat_map(|i| self.tcx.associated_items(i).in_definition_order())
// Only assoc fn with no receivers.
.filter(|item| {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_typeck/src/method/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -728,13 +728,13 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
let Some(simp) = simplify_type(self.tcx, self_ty, TreatParams::InstantiateWithInfer) else {
bug!("unexpected incoherent type: {:?}", self_ty)
};
for &impl_def_id in self.tcx.incoherent_impls(simp).into_iter().flatten() {
for &impl_def_id in self.tcx.incoherent_impls(simp).into_iter() {
self.assemble_inherent_impl_probe(impl_def_id);
}
}

fn assemble_inherent_impl_candidates_for_type(&mut self, def_id: DefId) {
let impl_def_ids = self.tcx.at(self.span).inherent_impls(def_id).into_iter().flatten();
let impl_def_ids = self.tcx.at(self.span).inherent_impls(def_id).into_iter();
for &impl_def_id in impl_def_ids {
self.assemble_inherent_impl_probe(impl_def_id);
}
Expand Down
11 changes: 4 additions & 7 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.tcx
.inherent_impls(adt_def.did())
.into_iter()
.flatten()
.any(|def_id| self.associated_value(*def_id, item_name).is_some())
} else {
false
Expand Down Expand Up @@ -1438,7 +1437,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.tcx
.inherent_impls(adt.did())
.into_iter()
.flatten()
.copied()
.filter(|def_id| {
if let Some(assoc) = self.associated_value(*def_id, item_name) {
Expand Down Expand Up @@ -1900,7 +1898,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
call_args: Option<Vec<Ty<'tcx>>>,
) -> Option<Symbol> {
if let ty::Adt(adt, adt_args) = rcvr_ty.kind() {
for inherent_impl_did in self.tcx.inherent_impls(adt.did()).into_iter().flatten() {
for inherent_impl_did in self.tcx.inherent_impls(adt.did()).into_iter() {
for inherent_method in
self.tcx.associated_items(inherent_impl_did).in_definition_order()
{
Expand Down Expand Up @@ -2114,9 +2112,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let ty::Adt(adt_def, _) = rcvr_ty.kind() else {
return;
};
// FIXME(oli-obk): try out bubbling this error up one level and cancelling the other error in that case.
let Ok(impls) = self.tcx.inherent_impls(adt_def.did()) else { return };
let mut items = impls
let mut items = self
.tcx
.inherent_impls(adt_def.did())
.iter()
.flat_map(|i| self.tcx.associated_items(i).in_definition_order())
// Only assoc fn with no receivers and only if
Expand Down Expand Up @@ -2495,7 +2493,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.and_then(|simp| {
tcx.incoherent_impls(simp)
.into_iter()
.flatten()
.find_map(|&id| self.associated_value(id, item_name))
})
.is_some()
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ provide! { tcx, def_id, other, cdata,
tcx.arena.alloc_from_iter(cdata.get_associated_item_or_field_def_ids(def_id.index))
}
associated_item => { cdata.get_associated_item(def_id.index, tcx.sess) }
inherent_impls => { Ok(cdata.get_inherent_implementations_for_type(tcx, def_id.index)) }
inherent_impls => { cdata.get_inherent_implementations_for_type(tcx, def_id.index) }
item_attrs => { tcx.arena.alloc_from_iter(cdata.get_item_attrs(def_id.index, tcx.sess)) }
is_mir_available => { cdata.is_item_mir_available(def_id.index) }
is_ctfe_mir_available => { cdata.is_ctfe_mir_available(def_id.index) }
Expand Down Expand Up @@ -393,7 +393,7 @@ provide! { tcx, def_id, other, cdata,
traits => { tcx.arena.alloc_from_iter(cdata.get_traits()) }
trait_impls_in_crate => { tcx.arena.alloc_from_iter(cdata.get_trait_impls()) }
implementations_of_trait => { cdata.get_implementations_of_trait(tcx, other) }
crate_incoherent_impls => { Ok(cdata.get_incoherent_impls(tcx, other)) }
crate_incoherent_impls => { cdata.get_incoherent_impls(tcx, other) }

dep_kind => { cdata.dep_kind }
module_children => {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1540,7 +1540,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
}
}

for (def_id, impls) in &tcx.crate_inherent_impls(()).unwrap().inherent_impls {
for (def_id, impls) in &tcx.crate_inherent_impls(()).0.inherent_impls {
record_defaulted_array!(self.tables.inherent_impls[def_id.to_def_id()] <- impls.iter().map(|def_id| {
assert!(def_id.is_local());
def_id.index
Expand Down Expand Up @@ -2089,7 +2089,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {

let all_impls: Vec<_> = tcx
.crate_inherent_impls(())
.unwrap()
.0
.incoherent_impls
.iter()
.map(|(&simp, impls)| IncoherentImpls {
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_middle/src/query/erase.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use std::intrinsics::transmute_unchecked;
use std::mem::MaybeUninit;

use rustc_span::ErrorGuaranteed;

use crate::query::CyclePlaceholder;
use crate::ty::adjustment::CoerceUnsizedInfo;
use crate::ty::{self, Ty};
Expand Down Expand Up @@ -216,6 +218,10 @@ impl<T0, T1> EraseType for (&'_ T0, &'_ [T1]) {
type Result = [u8; size_of::<(&'static (), &'static [()])>()];
}

impl<T0> EraseType for (&'_ T0, Result<(), ErrorGuaranteed>) {
type Result = [u8; size_of::<(&'static (), Result<(), ErrorGuaranteed>)>()];
}

macro_rules! trivial {
($($ty:ty),+ $(,)?) => {
$(
Expand Down
14 changes: 10 additions & 4 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -881,13 +881,13 @@ rustc_queries! {
/// Maps a `DefId` of a type to a list of its inherent impls.
/// Contains implementations of methods that are inherent to a type.
/// Methods in these implementations don't need to be exported.
query inherent_impls(key: DefId) -> Result<&'tcx [DefId], ErrorGuaranteed> {
query inherent_impls(key: DefId) -> &'tcx [DefId] {
desc { |tcx| "collecting inherent impls for `{}`", tcx.def_path_str(key) }
cache_on_disk_if { key.is_local() }
separate_provide_extern
}

query incoherent_impls(key: SimplifiedType) -> Result<&'tcx [DefId], ErrorGuaranteed> {
query incoherent_impls(key: SimplifiedType) -> &'tcx [DefId] {
desc { |tcx| "collecting all inherent impls for `{:?}`", key }
}

Expand Down Expand Up @@ -1017,8 +1017,14 @@ rustc_queries! {

/// Gets a complete map from all types to their inherent impls.
/// Not meant to be used directly outside of coherence.
query crate_inherent_impls(k: ()) -> Result<&'tcx CrateInherentImpls, ErrorGuaranteed> {
query crate_inherent_impls(k: ()) -> (&'tcx CrateInherentImpls, Result<(), ErrorGuaranteed>) {
desc { "finding all inherent impls defined in crate" }
}

/// Checks all types in the crate for overlap in their inherent impls. Reports errors.
/// Not meant to be used directly outside of coherence.
query crate_inherent_impls_validity_check(_: ()) -> Result<(), ErrorGuaranteed> {
desc { "check for inherent impls that should not be defined in crate" }
ensure_forwards_result_if_red
}

Expand Down Expand Up @@ -1715,7 +1721,7 @@ rustc_queries! {
///
/// Do not call this directly, but instead use the `incoherent_impls` query.
/// This query is only used to get the data necessary for that query.
query crate_incoherent_impls(key: (CrateNum, SimplifiedType)) -> Result<&'tcx [DefId], ErrorGuaranteed> {
query crate_incoherent_impls(key: (CrateNum, SimplifiedType)) -> &'tcx [DefId] {
desc { |tcx| "collecting all impls for a type in a crate" }
separate_provide_extern
}
Expand Down
20 changes: 3 additions & 17 deletions compiler/rustc_middle/src/ty/trait_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,30 +253,16 @@ pub(super) fn trait_impls_of_provider(tcx: TyCtxt<'_>, trait_id: DefId) -> Trait
}

/// Query provider for `incoherent_impls`.
pub(super) fn incoherent_impls_provider(
tcx: TyCtxt<'_>,
simp: SimplifiedType,
) -> Result<&[DefId], ErrorGuaranteed> {
pub(super) fn incoherent_impls_provider(tcx: TyCtxt<'_>, simp: SimplifiedType) -> &[DefId] {
let mut impls = Vec::new();

let mut res = Ok(());
for cnum in iter::once(LOCAL_CRATE).chain(tcx.crates(()).iter().copied()) {
let incoherent_impls = match tcx.crate_incoherent_impls((cnum, simp)) {
Ok(impls) => impls,
Err(e) => {
res = Err(e);
continue;
}
};
for &impl_def_id in incoherent_impls {
for &impl_def_id in tcx.crate_incoherent_impls((cnum, simp)) {
impls.push(impl_def_id)
}
}

debug!(?impls);
res?;

Ok(tcx.arena.alloc_slice(&impls))
tcx.arena.alloc_slice(&impls)
}

pub(super) fn traits_provider(tcx: TyCtxt<'_>, _: LocalCrate) -> &[DefId] {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_monomorphize/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1183,7 +1183,7 @@ fn collect_alloc<'tcx>(tcx: TyCtxt<'tcx>, alloc_id: AllocId, output: &mut MonoIt
}

fn assoc_fn_of_type<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, fn_ident: Ident) -> Option<DefId> {
for impl_def_id in tcx.inherent_impls(def_id).ok()? {
for impl_def_id in tcx.inherent_impls(def_id) {
if let Some(new) = tcx.associated_items(impl_def_id).find_by_name_and_kind(
tcx,
fn_ident,
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1825,10 +1825,12 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
// Doing analysis on local `DefId`s would cause infinite recursion.
return;
}
let Ok(impls) = self.r.tcx.inherent_impls(def_id) else { return };
// Look at all the associated functions without receivers in the type's
// inherent impls to look for builders that return `Self`
let mut items = impls
let mut items = self
.r
.tcx
.inherent_impls(def_id)
.iter()
.flat_map(|i| self.r.tcx.associated_items(i).in_definition_order())
// Only assoc fn with no receivers.
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ pub(crate) fn build_impls(
let tcx = cx.tcx;

// for each implementation of an item represented by `did`, build the clean::Item for that impl
for &did in tcx.inherent_impls(did).into_iter().flatten() {
for &did in tcx.inherent_impls(did).into_iter() {
cx.with_param_env(did, |cx| {
build_impl(cx, did, attrs, ret);
});
Expand All @@ -388,7 +388,7 @@ pub(crate) fn build_impls(
if tcx.has_attr(did, sym::rustc_has_incoherent_inherent_impls) {
let type_ =
if tcx.is_trait(did) { SimplifiedType::Trait(did) } else { SimplifiedType::Adt(did) };
for &did in tcx.incoherent_impls(type_).into_iter().flatten() {
for &did in tcx.incoherent_impls(type_).into_iter() {
cx.with_param_env(did, |cx| {
build_impl(cx, did, attrs, ret);
});
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1863,15 +1863,15 @@ impl PrimitiveType {
.get(self)
.into_iter()
.flatten()
.flat_map(move |&simp| tcx.incoherent_impls(simp).into_iter().flatten())
.flat_map(move |&simp| tcx.incoherent_impls(simp).into_iter())
.copied()
}

pub(crate) fn all_impls(tcx: TyCtxt<'_>) -> impl Iterator<Item = DefId> + '_ {
Self::simplified_types()
.values()
.flatten()
.flat_map(move |&simp| tcx.incoherent_impls(simp).into_iter().flatten())
.flat_map(move |&simp| tcx.incoherent_impls(simp).into_iter())
.copied()
}

Expand Down
1 change: 0 additions & 1 deletion src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,6 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
let mut assoc_items: Vec<_> = tcx
.inherent_impls(did)
.into_iter()
.flatten()
.flat_map(|&imp| {
filter_assoc_items_by_name_and_namespace(
tcx,
Expand Down
1 change: 0 additions & 1 deletion src/tools/clippy/clippy_lints/src/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,6 @@ fn check_unsafe_derive_deserialize<'tcx>(
.tcx
.inherent_impls(def.did())
.into_iter()
.flatten()
.map(|imp_did| cx.tcx.hir().expect_item(imp_did.expect_local()))
.any(|imp| has_unsafe(cx, imp))
{
Expand Down
4 changes: 1 addition & 3 deletions src/tools/clippy/clippy_lints/src/inherent_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,7 @@ impl<'tcx> LateLintPass<'tcx> for MultipleInherentImpl {
// List of spans to lint. (lint_span, first_span)
let mut lint_spans = Vec::new();

let Ok(impls) = cx.tcx.crate_inherent_impls(()) else {
return;
};
let (impls, _) = cx.tcx.crate_inherent_impls(());

for (&id, impl_ids) in &impls.inherent_impls {
if impl_ids.len() < 2
Expand Down
Loading

0 comments on commit 3b45f8f

Please sign in to comment.