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

Don't print full paths in overlap errors #104229

Merged
merged 1 commit into from
Nov 15, 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
4 changes: 4 additions & 0 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1254,6 +1254,10 @@ impl HandlerInner {
}

if diagnostic.has_future_breakage() {
// Future breakages aren't emitted if they're Level::Allowed,
// but they still need to be constructed and stashed below,
// so they'll trigger the good-path bug check.
self.suppressed_expected_diag = true;
self.future_breakage_diagnostics.push(diagnostic.clone());
}

Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_trait_selection/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,10 @@ pub struct NoValueInOnUnimplemented {
pub span: Span,
}

pub struct NegativePositiveConflict<'a> {
pub struct NegativePositiveConflict<'tcx> {
pub impl_span: Span,
pub trait_desc: &'a str,
pub self_desc: &'a Option<String>,
pub trait_desc: ty::TraitRef<'tcx>,
pub self_ty: Option<Ty<'tcx>>,
pub negative_impl_span: Result<Span, Symbol>,
pub positive_impl_span: Result<Span, Symbol>,
}
Expand All @@ -73,10 +73,10 @@ impl IntoDiagnostic<'_> for NegativePositiveConflict<'_> {
handler: &Handler,
) -> rustc_errors::DiagnosticBuilder<'_, ErrorGuaranteed> {
let mut diag = handler.struct_err(fluent::trait_selection_negative_positive_conflict);
diag.set_arg("trait_desc", self.trait_desc);
diag.set_arg("trait_desc", self.trait_desc.print_only_trait_path().to_string());
diag.set_arg(
"self_desc",
self.self_desc.clone().map_or_else(|| String::from("none"), |ty| ty),
self.self_ty.map_or_else(|| "none".to_string(), |ty| ty.to_string()),
);
diag.set_span(self.impl_span);
diag.code(rustc_errors::error_code!(E0751));
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_trait_selection/src/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,13 @@ pub fn add_placeholder_note(err: &mut Diagnostic) {
/// with a suitably-freshened `ImplHeader` with those types
/// substituted. Otherwise, returns `None`.
#[instrument(skip(tcx, skip_leak_check), level = "debug")]
pub fn overlapping_impls(
tcx: TyCtxt<'_>,
pub fn overlapping_impls<'tcx>(
tcx: TyCtxt<'tcx>,
impl1_def_id: DefId,
impl2_def_id: DefId,
skip_leak_check: SkipLeakCheck,
overlap_mode: OverlapMode,
) -> Option<OverlapResult<'_>> {
) -> Option<OverlapResult<'tcx>> {
// Before doing expensive operations like entering an inference context, do
// a quick check via fast_reject to tell if the impl headers could possibly
// unify.
Expand Down
75 changes: 40 additions & 35 deletions compiler/rustc_trait_selection/src/traits/specialize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ use crate::infer::{InferCtxt, InferOk, TyCtxtInferExt};
use crate::traits::select::IntercrateAmbiguityCause;
use crate::traits::{self, coherence, FutureCompatOverlapErrorKind, ObligationCause};
use rustc_data_structures::fx::FxIndexSet;
use rustc_errors::{struct_span_err, DiagnosticBuilder, EmissionGuarantee};
use rustc_errors::{error_code, DelayDm, Diagnostic};
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_middle::ty::{self, ImplSubject, TyCtxt};
use rustc_middle::ty::{self, ImplSubject, Ty, TyCtxt};
use rustc_middle::ty::{InternalSubsts, SubstsRef};
use rustc_session::lint::builtin::COHERENCE_LEAK_CHECK;
use rustc_session::lint::builtin::ORDER_DEPENDENT_TRAIT_OBJECTS;
Expand All @@ -30,10 +30,10 @@ use super::SelectionContext;

/// Information pertinent to an overlapping impl error.
#[derive(Debug)]
pub struct OverlapError {
pub struct OverlapError<'tcx> {
pub with_impl: DefId,
pub trait_desc: String,
pub self_desc: Option<String>,
pub trait_ref: ty::TraitRef<'tcx>,
pub self_ty: Option<Ty<'tcx>>,
pub intercrate_ambiguity_causes: FxIndexSet<IntercrateAmbiguityCause>,
pub involves_placeholder: bool,
}
Expand Down Expand Up @@ -277,9 +277,9 @@ pub(super) fn specialization_graph_provider(
// it negatively impacts perf.
#[cold]
#[inline(never)]
fn report_overlap_conflict(
tcx: TyCtxt<'_>,
overlap: OverlapError,
fn report_overlap_conflict<'tcx>(
tcx: TyCtxt<'tcx>,
overlap: OverlapError<'tcx>,
impl_def_id: LocalDefId,
used_to_be_allowed: Option<FutureCompatOverlapErrorKind>,
sg: &mut specialization_graph::Graph,
Expand Down Expand Up @@ -315,27 +315,27 @@ fn report_overlap_conflict(
}
}

fn report_negative_positive_conflict(
tcx: TyCtxt<'_>,
overlap: &OverlapError,
fn report_negative_positive_conflict<'tcx>(
tcx: TyCtxt<'tcx>,
overlap: &OverlapError<'tcx>,
local_impl_def_id: LocalDefId,
negative_impl_def_id: DefId,
positive_impl_def_id: DefId,
sg: &mut specialization_graph::Graph,
) {
let mut err = tcx.sess.create_err(NegativePositiveConflict {
impl_span: tcx.def_span(local_impl_def_id),
trait_desc: &overlap.trait_desc,
self_desc: &overlap.self_desc,
trait_desc: overlap.trait_ref,
self_ty: overlap.self_ty,
negative_impl_span: tcx.span_of_impl(negative_impl_def_id),
positive_impl_span: tcx.span_of_impl(positive_impl_def_id),
});
sg.has_errored = Some(err.emit());
}

fn report_conflicting_impls(
tcx: TyCtxt<'_>,
overlap: OverlapError,
fn report_conflicting_impls<'tcx>(
tcx: TyCtxt<'tcx>,
overlap: OverlapError<'tcx>,
impl_def_id: LocalDefId,
used_to_be_allowed: Option<FutureCompatOverlapErrorKind>,
sg: &mut specialization_graph::Graph,
Expand All @@ -345,12 +345,12 @@ fn report_conflicting_impls(
// Work to be done after we've built the DiagnosticBuilder. We have to define it
// now because the struct_lint methods don't return back the DiagnosticBuilder
// that's passed in.
fn decorate<'a, 'b, G: EmissionGuarantee>(
tcx: TyCtxt<'_>,
overlap: OverlapError,
fn decorate<'tcx>(
tcx: TyCtxt<'tcx>,
overlap: &OverlapError<'tcx>,
impl_span: Span,
err: &'b mut DiagnosticBuilder<'a, G>,
) -> &'b mut DiagnosticBuilder<'a, G> {
err: &mut Diagnostic,
) {
match tcx.span_of_impl(overlap.with_impl) {
Ok(span) => {
err.span_label(span, "first implementation here");
Expand All @@ -359,7 +359,7 @@ fn report_conflicting_impls(
impl_span,
format!(
"conflicting implementation{}",
overlap.self_desc.map_or_else(String::new, |ty| format!(" for `{}`", ty))
overlap.self_ty.map_or_else(String::new, |ty| format!(" for `{}`", ty))
),
);
}
Expand All @@ -381,26 +381,28 @@ fn report_conflicting_impls(
if overlap.involves_placeholder {
coherence::add_placeholder_note(err);
}
err
}

let msg = format!(
"conflicting implementations of trait `{}`{}{}",
overlap.trait_desc,
overlap.self_desc.as_deref().map_or_else(String::new, |ty| format!(" for type `{ty}`")),
match used_to_be_allowed {
Some(FutureCompatOverlapErrorKind::Issue33140) => ": (E0119)",
_ => "",
}
);
let msg = DelayDm(|| {
format!(
"conflicting implementations of trait `{}`{}{}",
overlap.trait_ref.print_only_trait_path(),
overlap.self_ty.map_or_else(String::new, |ty| format!(" for type `{ty}`")),
match used_to_be_allowed {
Some(FutureCompatOverlapErrorKind::Issue33140) => ": (E0119)",
_ => "",
}
)
});

match used_to_be_allowed {
None => {
let reported = if overlap.with_impl.is_local()
|| tcx.orphan_check_impl(impl_def_id).is_ok()
{
let mut err = struct_span_err!(tcx.sess, impl_span, E0119, "{msg}",);
decorate(tcx, overlap, impl_span, &mut err);
let mut err = tcx.sess.struct_span_err(impl_span, msg);
err.code(error_code!(E0119));
decorate(tcx, &overlap, impl_span, &mut err);
Some(err.emit())
} else {
Some(tcx.sess.delay_span_bug(impl_span, "impl should have failed the orphan check"))
Expand All @@ -417,7 +419,10 @@ fn report_conflicting_impls(
tcx.hir().local_def_id_to_hir_id(impl_def_id),
impl_span,
msg,
|err| decorate(tcx, overlap, impl_span, err),
|err| {
decorate(tcx, &overlap, impl_span, err);
err
},
);
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use super::OverlapError;
use crate::traits;
use rustc_hir::def_id::DefId;
use rustc_middle::ty::fast_reject::{self, SimplifiedType, TreatParams};
use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::{self, TyCtxt, TypeVisitable};

pub use rustc_middle::traits::specialization_graph::*;
Expand All @@ -15,15 +14,15 @@ pub enum FutureCompatOverlapErrorKind {
}

#[derive(Debug)]
pub struct FutureCompatOverlapError {
pub error: OverlapError,
pub struct FutureCompatOverlapError<'tcx> {
pub error: OverlapError<'tcx>,
pub kind: FutureCompatOverlapErrorKind,
}

/// The result of attempting to insert an impl into a group of children.
enum Inserted {
enum Inserted<'tcx> {
/// The impl was inserted as a new child in this group of children.
BecameNewSibling(Option<FutureCompatOverlapError>),
BecameNewSibling(Option<FutureCompatOverlapError<'tcx>>),

/// The impl should replace existing impls [X1, ..], because the impl specializes X1, X2, etc.
ReplaceChildren(Vec<DefId>),
Expand All @@ -42,12 +41,12 @@ trait ChildrenExt<'tcx> {
impl_def_id: DefId,
simplified_self: Option<SimplifiedType>,
overlap_mode: OverlapMode,
) -> Result<Inserted, OverlapError>;
) -> Result<Inserted<'tcx>, OverlapError<'tcx>>;
}

impl ChildrenExt<'_> for Children {
impl<'tcx> ChildrenExt<'tcx> for Children {
/// Insert an impl into this set of children without comparing to any existing impls.
fn insert_blindly(&mut self, tcx: TyCtxt<'_>, impl_def_id: DefId) {
fn insert_blindly(&mut self, tcx: TyCtxt<'tcx>, impl_def_id: DefId) {
let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap();
if let Some(st) = fast_reject::simplify_type(tcx, trait_ref.self_ty(), TreatParams::AsInfer)
{
Expand All @@ -62,7 +61,7 @@ impl ChildrenExt<'_> for Children {
/// Removes an impl from this set of children. Used when replacing
/// an impl with a parent. The impl must be present in the list of
/// children already.
fn remove_existing(&mut self, tcx: TyCtxt<'_>, impl_def_id: DefId) {
fn remove_existing(&mut self, tcx: TyCtxt<'tcx>, impl_def_id: DefId) {
let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap();
let vec: &mut Vec<DefId>;
if let Some(st) = fast_reject::simplify_type(tcx, trait_ref.self_ty(), TreatParams::AsInfer)
Expand All @@ -82,11 +81,11 @@ impl ChildrenExt<'_> for Children {
/// specialization relationships.
fn insert(
&mut self,
tcx: TyCtxt<'_>,
tcx: TyCtxt<'tcx>,
impl_def_id: DefId,
simplified_self: Option<SimplifiedType>,
overlap_mode: OverlapMode,
) -> Result<Inserted, OverlapError> {
) -> Result<Inserted<'tcx>, OverlapError<'tcx>> {
let mut last_lint = None;
let mut replace_children = Vec::new();

Expand All @@ -103,30 +102,23 @@ impl ChildrenExt<'_> for Children {
impl_def_id, simplified_self, possible_sibling,
);

let create_overlap_error = |overlap: traits::coherence::OverlapResult<'_>| {
let create_overlap_error = |overlap: traits::coherence::OverlapResult<'tcx>| {
let trait_ref = overlap.impl_header.trait_ref.unwrap();
let self_ty = trait_ref.self_ty();

// FIXME: should postpone string formatting until we decide to actually emit.
with_no_trimmed_paths!({
OverlapError {
with_impl: possible_sibling,
trait_desc: trait_ref.print_only_trait_path().to_string(),
// Only report the `Self` type if it has at least
// some outer concrete shell; otherwise, it's
// not adding much information.
self_desc: if self_ty.has_concrete_skeleton() {
Some(self_ty.to_string())
} else {
None
},
intercrate_ambiguity_causes: overlap.intercrate_ambiguity_causes,
involves_placeholder: overlap.involves_placeholder,
}
})
OverlapError {
with_impl: possible_sibling,
trait_ref,
// Only report the `Self` type if it has at least
// some outer concrete shell; otherwise, it's
// not adding much information.
self_ty: if self_ty.has_concrete_skeleton() { Some(self_ty) } else { None },
intercrate_ambiguity_causes: overlap.intercrate_ambiguity_causes,
involves_placeholder: overlap.involves_placeholder,
}
};

let report_overlap_error = |overlap: traits::coherence::OverlapResult<'_>,
let report_overlap_error = |overlap: traits::coherence::OverlapResult<'tcx>,
last_lint: &mut _| {
// Found overlap, but no specialization; error out or report future-compat warning.

Expand Down Expand Up @@ -255,31 +247,31 @@ where
}
}

pub trait GraphExt {
pub trait GraphExt<'tcx> {
/// Insert a local impl into the specialization graph. If an existing impl
/// conflicts with it (has overlap, but neither specializes the other),
/// information about the area of overlap is returned in the `Err`.
fn insert(
&mut self,
tcx: TyCtxt<'_>,
tcx: TyCtxt<'tcx>,
impl_def_id: DefId,
overlap_mode: OverlapMode,
) -> Result<Option<FutureCompatOverlapError>, OverlapError>;
) -> Result<Option<FutureCompatOverlapError<'tcx>>, OverlapError<'tcx>>;

/// Insert cached metadata mapping from a child impl back to its parent.
fn record_impl_from_cstore(&mut self, tcx: TyCtxt<'_>, parent: DefId, child: DefId);
fn record_impl_from_cstore(&mut self, tcx: TyCtxt<'tcx>, parent: DefId, child: DefId);
}

impl GraphExt for Graph {
impl<'tcx> GraphExt<'tcx> for Graph {
/// Insert a local impl into the specialization graph. If an existing impl
/// conflicts with it (has overlap, but neither specializes the other),
/// information about the area of overlap is returned in the `Err`.
fn insert(
&mut self,
tcx: TyCtxt<'_>,
tcx: TyCtxt<'tcx>,
impl_def_id: DefId,
overlap_mode: OverlapMode,
) -> Result<Option<FutureCompatOverlapError>, OverlapError> {
) -> Result<Option<FutureCompatOverlapError<'tcx>>, OverlapError<'tcx>> {
assert!(impl_def_id.is_local());

let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap();
Expand Down Expand Up @@ -376,7 +368,7 @@ impl GraphExt for Graph {
}

/// Insert cached metadata mapping from a child impl back to its parent.
fn record_impl_from_cstore(&mut self, tcx: TyCtxt<'_>, parent: DefId, child: DefId) {
fn record_impl_from_cstore(&mut self, tcx: TyCtxt<'tcx>, parent: DefId, child: DefId) {
if self.parent.insert(child, parent).is_some() {
bug!(
"When recording an impl from the crate store, information about its parent \
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error[E0119]: conflicting implementations of trait `go_trait::GoMut` for type `MyThingy`
error[E0119]: conflicting implementations of trait `GoMut` for type `MyThingy`
--> $DIR/coherence-blanket-conflicts-with-specific-cross-crate.rs:15:1
|
LL | impl GoMut for MyThingy {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error[E0751]: found both positive and negative implementation of trait `std::marker::Send` for type `TestType<_>`:
error[E0751]: found both positive and negative implementation of trait `Send` for type `TestType<_>`:
--> $DIR/coherence-conflicting-negative-trait-impl.rs:11:1
|
LL | unsafe impl<T: MyTrait + 'static> Send for TestType<T> {}
Expand All @@ -7,7 +7,7 @@ LL |
LL | impl<T: MyTrait> !Send for TestType<T> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ negative implementation here

error[E0119]: conflicting implementations of trait `std::marker::Send` for type `TestType<_>`
error[E0119]: conflicting implementations of trait `Send` for type `TestType<_>`
--> $DIR/coherence-conflicting-negative-trait-impl.rs:13:1
|
LL | unsafe impl<T: MyTrait + 'static> Send for TestType<T> {}
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/coherence/coherence-impls-copy.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ LL | impl Copy for i32 {}
|
= note: define and implement a trait or new type instead

error[E0119]: conflicting implementations of trait `std::marker::Copy` for type `&NotSync`
error[E0119]: conflicting implementations of trait `Copy` for type `&NotSync`
--> $DIR/coherence-impls-copy.rs:28:1
|
LL | impl Copy for &'static NotSync {}
Expand Down
Loading