Skip to content

Commit

Permalink
Rollup merge of rust-lang#104229 - compiler-errors:overlap-full-path,…
Browse files Browse the repository at this point in the history
… r=davidtwco

Don't print full paths in overlap errors

We don't print the full path in other diagnostics -- I don't think it particularly helps with the error message. I also delayed the printing until actually needing to render the error message.

r? diagnostics
  • Loading branch information
matthiaskrgr authored Nov 14, 2022
2 parents a672cb8 + f902b49 commit e242b9d
Show file tree
Hide file tree
Showing 32 changed files with 157 additions and 156 deletions.
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 @@ -19,9 +19,9 @@ use crate::traits::engine::TraitEngineExt as _;
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 @@ -32,10 +32,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 @@ -275,9 +275,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 @@ -313,27 +313,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 @@ -343,12 +343,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 @@ -357,7 +357,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 @@ -379,26 +379,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 @@ -415,7 +417,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

0 comments on commit e242b9d

Please sign in to comment.