1
Fork 0

specialization_graph_of's errored field is used in the only call site, and used to immediately throw away the rest of the value. Let's use Result to statically signal that this is happening

This commit is contained in:
Oli Scherer 2024-01-11 10:05:12 +00:00
parent 3a6bf351a3
commit 5461836281
5 changed files with 49 additions and 46 deletions

View file

@ -113,6 +113,7 @@ macro_rules! arena_types {
[] stripped_cfg_items: rustc_ast::expand::StrippedCfgItem, [] stripped_cfg_items: rustc_ast::expand::StrippedCfgItem,
[] mod_child: rustc_middle::metadata::ModChild, [] mod_child: rustc_middle::metadata::ModChild,
[] features: rustc_feature::Features, [] features: rustc_feature::Features,
[decode] specialization_graph: rustc_middle::traits::specialization_graph::Graph,
]); ]);
) )
} }

View file

@ -1294,8 +1294,7 @@ rustc_queries! {
desc { |tcx| "finding trait impls of `{}`", tcx.def_path_str(trait_id) } desc { |tcx| "finding trait impls of `{}`", tcx.def_path_str(trait_id) }
} }
query specialization_graph_of(trait_id: DefId) -> &'tcx specialization_graph::Graph { query specialization_graph_of(trait_id: DefId) -> Result<&'tcx specialization_graph::Graph, ErrorGuaranteed> {
arena_cache
desc { |tcx| "building specialization graph of trait `{}`", tcx.def_path_str(trait_id) } desc { |tcx| "building specialization graph of trait `{}`", tcx.def_path_str(trait_id) }
cache_on_disk_if { true } cache_on_disk_if { true }
} }

View file

@ -786,6 +786,15 @@ impl<'a, 'tcx> Decodable<CacheDecoder<'a, 'tcx>> for &'tcx [rustc_ast::InlineAsm
} }
} }
impl<'a, 'tcx> Decodable<CacheDecoder<'a, 'tcx>>
for &'tcx crate::traits::specialization_graph::Graph
{
#[inline]
fn decode(d: &mut CacheDecoder<'a, 'tcx>) -> Self {
RefDecodable::decode(d)
}
}
macro_rules! impl_ref_decoder { macro_rules! impl_ref_decoder {
(<$tcx:tt> $($ty:ty,)*) => { (<$tcx:tt> $($ty:ty,)*) => {
$(impl<'a, $tcx> Decodable<CacheDecoder<'a, $tcx>> for &$tcx [$ty] { $(impl<'a, $tcx> Decodable<CacheDecoder<'a, $tcx>> for &$tcx [$ty] {

View file

@ -30,18 +30,16 @@ pub struct Graph {
/// The "root" impls are found by looking up the trait's def_id. /// The "root" impls are found by looking up the trait's def_id.
pub children: DefIdMap<Children>, pub children: DefIdMap<Children>,
/// Whether an error was emitted while constructing the graph.
pub has_errored: Option<ErrorGuaranteed>,
} }
impl Graph { impl Graph {
pub fn new() -> Graph { pub fn new() -> Graph {
Graph { parent: Default::default(), children: Default::default(), has_errored: None } Graph { parent: Default::default(), children: Default::default() }
} }
/// The parent of a given impl, which is the `DefId` of the trait when the /// The parent of a given impl, which is the `DefId` of the trait when the
/// impl is a "specialization root". /// impl is a "specialization root".
#[track_caller]
pub fn parent(&self, child: DefId) -> DefId { pub fn parent(&self, child: DefId) -> DefId {
*self.parent.get(&child).unwrap_or_else(|| panic!("Failed to get parent for {child:?}")) *self.parent.get(&child).unwrap_or_else(|| panic!("Failed to get parent for {child:?}"))
} }
@ -255,13 +253,9 @@ pub fn ancestors(
trait_def_id: DefId, trait_def_id: DefId,
start_from_impl: DefId, start_from_impl: DefId,
) -> Result<Ancestors<'_>, ErrorGuaranteed> { ) -> Result<Ancestors<'_>, ErrorGuaranteed> {
let specialization_graph = tcx.specialization_graph_of(trait_def_id); let specialization_graph = tcx.specialization_graph_of(trait_def_id)?;
if let Some(reported) = specialization_graph.has_errored { if let Err(reported) = tcx.type_of(start_from_impl).instantiate_identity().error_reported() {
Err(reported)
} else if let Err(reported) =
tcx.type_of(start_from_impl).instantiate_identity().error_reported()
{
Err(reported) Err(reported)
} else { } else {
Ok(Ancestors { Ok(Ancestors {

View file

@ -26,7 +26,7 @@ use rustc_middle::ty::{self, ImplSubject, Ty, TyCtxt, TypeVisitableExt};
use rustc_middle::ty::{GenericArgs, GenericArgsRef}; use rustc_middle::ty::{GenericArgs, GenericArgsRef};
use rustc_session::lint::builtin::COHERENCE_LEAK_CHECK; use rustc_session::lint::builtin::COHERENCE_LEAK_CHECK;
use rustc_session::lint::builtin::ORDER_DEPENDENT_TRAIT_OBJECTS; use rustc_session::lint::builtin::ORDER_DEPENDENT_TRAIT_OBJECTS;
use rustc_span::{Span, DUMMY_SP}; use rustc_span::{ErrorGuaranteed, Span, DUMMY_SP};
use super::util; use super::util;
use super::SelectionContext; use super::SelectionContext;
@ -258,7 +258,7 @@ fn fulfill_implication<'tcx>(
pub(super) fn specialization_graph_provider( pub(super) fn specialization_graph_provider(
tcx: TyCtxt<'_>, tcx: TyCtxt<'_>,
trait_id: DefId, trait_id: DefId,
) -> specialization_graph::Graph { ) -> Result<&'_ specialization_graph::Graph, ErrorGuaranteed> {
let mut sg = specialization_graph::Graph::new(); let mut sg = specialization_graph::Graph::new();
let overlap_mode = specialization_graph::OverlapMode::get(tcx, trait_id); let overlap_mode = specialization_graph::OverlapMode::get(tcx, trait_id);
@ -271,6 +271,8 @@ pub(super) fn specialization_graph_provider(
trait_impls trait_impls
.sort_unstable_by_key(|def_id| (-(def_id.krate.as_u32() as i64), def_id.index.index())); .sort_unstable_by_key(|def_id| (-(def_id.krate.as_u32() as i64), def_id.index.index()));
let mut errored = Ok(());
for impl_def_id in trait_impls { for impl_def_id in trait_impls {
if let Some(impl_def_id) = impl_def_id.as_local() { if let Some(impl_def_id) = impl_def_id.as_local() {
// This is where impl overlap checking happens: // This is where impl overlap checking happens:
@ -283,15 +285,21 @@ pub(super) fn specialization_graph_provider(
}; };
if let Some(overlap) = overlap { if let Some(overlap) = overlap {
report_overlap_conflict(tcx, overlap, impl_def_id, used_to_be_allowed, &mut sg); errored = errored.and(report_overlap_conflict(
tcx,
overlap,
impl_def_id,
used_to_be_allowed,
));
} }
} else { } else {
let parent = tcx.impl_parent(impl_def_id).unwrap_or(trait_id); let parent = tcx.impl_parent(impl_def_id).unwrap_or(trait_id);
sg.record_impl_from_cstore(tcx, parent, impl_def_id) sg.record_impl_from_cstore(tcx, parent, impl_def_id)
} }
} }
errored?;
sg Ok(tcx.arena.alloc(sg))
} }
// This function is only used when // This function is only used when
@ -304,36 +312,31 @@ fn report_overlap_conflict<'tcx>(
overlap: OverlapError<'tcx>, overlap: OverlapError<'tcx>,
impl_def_id: LocalDefId, impl_def_id: LocalDefId,
used_to_be_allowed: Option<FutureCompatOverlapErrorKind>, used_to_be_allowed: Option<FutureCompatOverlapErrorKind>,
sg: &mut specialization_graph::Graph, ) -> Result<(), ErrorGuaranteed> {
) {
let impl_polarity = tcx.impl_polarity(impl_def_id.to_def_id()); let impl_polarity = tcx.impl_polarity(impl_def_id.to_def_id());
let other_polarity = tcx.impl_polarity(overlap.with_impl); let other_polarity = tcx.impl_polarity(overlap.with_impl);
match (impl_polarity, other_polarity) { match (impl_polarity, other_polarity) {
(ty::ImplPolarity::Negative, ty::ImplPolarity::Positive) => { (ty::ImplPolarity::Negative, ty::ImplPolarity::Positive) => {
report_negative_positive_conflict( Err(report_negative_positive_conflict(
tcx, tcx,
&overlap, &overlap,
impl_def_id, impl_def_id,
impl_def_id.to_def_id(), impl_def_id.to_def_id(),
overlap.with_impl, overlap.with_impl,
sg, ))
);
} }
(ty::ImplPolarity::Positive, ty::ImplPolarity::Negative) => { (ty::ImplPolarity::Positive, ty::ImplPolarity::Negative) => {
report_negative_positive_conflict( Err(report_negative_positive_conflict(
tcx, tcx,
&overlap, &overlap,
impl_def_id, impl_def_id,
overlap.with_impl, overlap.with_impl,
impl_def_id.to_def_id(), impl_def_id.to_def_id(),
sg, ))
);
} }
_ => { _ => report_conflicting_impls(tcx, overlap, impl_def_id, used_to_be_allowed),
report_conflicting_impls(tcx, overlap, impl_def_id, used_to_be_allowed, sg);
}
} }
} }
@ -343,16 +346,16 @@ fn report_negative_positive_conflict<'tcx>(
local_impl_def_id: LocalDefId, local_impl_def_id: LocalDefId,
negative_impl_def_id: DefId, negative_impl_def_id: DefId,
positive_impl_def_id: DefId, positive_impl_def_id: DefId,
sg: &mut specialization_graph::Graph, ) -> ErrorGuaranteed {
) { tcx.dcx()
let err = tcx.dcx().create_err(NegativePositiveConflict { .create_err(NegativePositiveConflict {
impl_span: tcx.def_span(local_impl_def_id), impl_span: tcx.def_span(local_impl_def_id),
trait_desc: overlap.trait_ref, trait_desc: overlap.trait_ref,
self_ty: overlap.self_ty, self_ty: overlap.self_ty,
negative_impl_span: tcx.span_of_impl(negative_impl_def_id), negative_impl_span: tcx.span_of_impl(negative_impl_def_id),
positive_impl_span: tcx.span_of_impl(positive_impl_def_id), positive_impl_span: tcx.span_of_impl(positive_impl_def_id),
}); })
sg.has_errored = Some(err.emit()); .emit()
} }
fn report_conflicting_impls<'tcx>( fn report_conflicting_impls<'tcx>(
@ -360,8 +363,7 @@ fn report_conflicting_impls<'tcx>(
overlap: OverlapError<'tcx>, overlap: OverlapError<'tcx>,
impl_def_id: LocalDefId, impl_def_id: LocalDefId,
used_to_be_allowed: Option<FutureCompatOverlapErrorKind>, used_to_be_allowed: Option<FutureCompatOverlapErrorKind>,
sg: &mut specialization_graph::Graph, ) -> Result<(), ErrorGuaranteed> {
) {
let impl_span = tcx.def_span(impl_def_id); let impl_span = tcx.def_span(impl_def_id);
// Work to be done after we've built the DiagnosticBuilder. We have to define it // Work to be done after we've built the DiagnosticBuilder. We have to define it
@ -429,14 +431,11 @@ fn report_conflicting_impls<'tcx>(
let mut err = tcx.dcx().struct_span_err(impl_span, msg); let mut err = tcx.dcx().struct_span_err(impl_span, msg);
err.code(error_code!(E0119)); err.code(error_code!(E0119));
decorate(tcx, &overlap, impl_span, &mut err); decorate(tcx, &overlap, impl_span, &mut err);
Some(err.emit()) err.emit()
} else { } else {
Some( tcx.dcx().span_delayed_bug(impl_span, "impl should have failed the orphan check")
tcx.dcx()
.span_delayed_bug(impl_span, "impl should have failed the orphan check"),
)
}; };
sg.has_errored = reported; Err(reported)
} }
Some(kind) => { Some(kind) => {
let lint = match kind { let lint = match kind {
@ -452,8 +451,9 @@ fn report_conflicting_impls<'tcx>(
decorate(tcx, &overlap, impl_span, err); decorate(tcx, &overlap, impl_span, err);
}, },
); );
Ok(())
} }
}; }
} }
/// Recovers the "impl X for Y" signature from `impl_def_id` and returns it as a /// Recovers the "impl X for Y" signature from `impl_def_id` and returns it as a