diff options
| author | bors <bors@rust-lang.org> | 2022-10-01 10:44:25 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2022-10-01 10:44:25 +0000 |
| commit | 744e397d8855f7da87d70aa8d0bd9e0f5f0b51a1 (patch) | |
| tree | 1721987352b5f0a8548fc46984821d974b661934 /compiler/rustc_trait_selection/src/traits/specialize/mod.rs | |
| parent | 277bb6653b55475b5fbce6309e9852fa2100dabe (diff) | |
| parent | b5b3ffe3fc9cfb524a6432ec60a0fc95c514d2e1 (diff) | |
| download | rust-744e397d8855f7da87d70aa8d0bd9e0f5f0b51a1.tar.gz rust-744e397d8855f7da87d70aa8d0bd9e0f5f0b51a1.zip | |
Auto merge of #101986 - WaffleLapkin:move_lint_note_to_the_bottom, r=estebank
Move lint level source explanation to the bottom So, uhhhhh r? `@estebank` ## User-facing change "note: `#[warn(...)]` on by default" and such are moved to the bottom of the diagnostic: ```diff - = note: `#[warn(unsupported_calling_conventions)]` on by default = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #87678 <https://github.com/rust-lang/rust/issues/87678> + = note: `#[warn(unsupported_calling_conventions)]` on by default ``` Why warning is enabled is the least important thing, so it shouldn't be the first note the user reads, IMO. ## Developer-facing change `struct_span_lint` and similar methods have a different signature. Before: `..., impl for<'a> FnOnce(LintDiagnosticBuilder<'a, ()>)` After: `..., impl Into<DiagnosticMessage>, impl for<'a, 'b> FnOnce(&'b mut DiagnosticBuilder<'a, ()>) -> &'b mut DiagnosticBuilder<'a, ()>` The reason for this is that `struct_span_lint` needs to edit the diagnostic _after_ `decorate` closure is called. This also makes lint code a little bit nicer in my opinion. Another option is to use `impl for<'a> FnOnce(LintDiagnosticBuilder<'a, ()>) -> DiagnosticBuilder<'a, ()>` altough I don't _really_ see reasons to do `let lint = lint.build(message)` everywhere. ## Subtle problem By moving the message outside of the closure (that may not be called if the lint is disabled) `format!(...)` is executed earlier, possibly formatting `Ty` which may call a query that trims paths that crashes the compiler if there were no warnings... I don't think it's that big of a deal, considering that we move from `format!(...)` to `fluent` (which is lazy by-default) anyway, however this required adding a workaround which is unfortunate. ## P.S. I'm sorry, I do not how to make this PR smaller/easier to review. Changes to the lint API affect SO MUCH 😢
Diffstat (limited to 'compiler/rustc_trait_selection/src/traits/specialize/mod.rs')
| -rw-r--r-- | compiler/rustc_trait_selection/src/traits/specialize/mod.rs | 58 |
1 files changed, 25 insertions, 33 deletions
diff --git a/compiler/rustc_trait_selection/src/traits/specialize/mod.rs b/compiler/rustc_trait_selection/src/traits/specialize/mod.rs index 56a88749c46..eac3f0f30e8 100644 --- a/compiler/rustc_trait_selection/src/traits/specialize/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/specialize/mod.rs @@ -17,7 +17,7 @@ use crate::infer::{InferCtxt, InferOk, TyCtxtInferExt}; use crate::traits::select::IntercrateAmbiguityCause; use crate::traits::{self, coherence, FutureCompatOverlapErrorKind, ObligationCause}; use rustc_data_structures::fx::{FxHashSet, FxIndexSet}; -use rustc_errors::{struct_span_err, EmissionGuarantee, LintDiagnosticBuilder}; +use rustc_errors::{struct_span_err, DiagnosticBuilder, EmissionGuarantee}; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_middle::ty::{self, ImplSubject, TyCtxt}; use rustc_middle::ty::{InternalSubsts, SubstsRef}; @@ -350,26 +350,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<G: EmissionGuarantee>( + fn decorate<'a, 'b, G: EmissionGuarantee>( tcx: TyCtxt<'_>, overlap: OverlapError, - used_to_be_allowed: Option<FutureCompatOverlapErrorKind>, impl_span: Span, - err: LintDiagnosticBuilder<'_, G>, - ) -> G { - let msg = format!( - "conflicting implementations of trait `{}`{}{}", - overlap.trait_desc, - overlap - .self_desc - .clone() - .map_or_else(String::new, |ty| { format!(" for type `{}`", ty) }), - match used_to_be_allowed { - Some(FutureCompatOverlapErrorKind::Issue33140) => ": (E0119)", - _ => "", - } - ); - let mut err = err.build(&msg); + err: &'b mut DiagnosticBuilder<'a, G>, + ) -> &'b mut DiagnosticBuilder<'a, G> { match tcx.span_of_impl(overlap.with_impl) { Ok(span) => { err.span_label(span, "first implementation here"); @@ -384,7 +370,9 @@ fn report_conflicting_impls( } Err(cname) => { let msg = match to_pretty_impl_header(tcx, overlap.with_impl) { - Some(s) => format!("conflicting implementation in crate `{}`:\n- {}", cname, s), + Some(s) => { + format!("conflicting implementation in crate `{}`:\n- {}", cname, s) + } None => format!("conflicting implementation in crate `{}`", cname), }; err.note(&msg); @@ -392,28 +380,33 @@ fn report_conflicting_impls( } for cause in &overlap.intercrate_ambiguity_causes { - cause.add_intercrate_ambiguity_hint(&mut err); + cause.add_intercrate_ambiguity_hint(err); } if overlap.involves_placeholder { - coherence::add_placeholder_note(&mut err); + coherence::add_placeholder_note(err); } - err.emit() + 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)", + _ => "", + } + ); + match used_to_be_allowed { None => { let reported = if overlap.with_impl.is_local() || tcx.orphan_check_impl(impl_def_id).is_ok() { - let err = struct_span_err!(tcx.sess, impl_span, E0119, ""); - Some(decorate( - tcx, - overlap, - used_to_be_allowed, - impl_span, - LintDiagnosticBuilder::new(err), - )) + let mut err = struct_span_err!(tcx.sess, impl_span, E0119, "{msg}",); + 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")) }; @@ -428,9 +421,8 @@ fn report_conflicting_impls( lint, tcx.hir().local_def_id_to_hir_id(impl_def_id), impl_span, - |ldb| { - decorate(tcx, overlap, used_to_be_allowed, impl_span, ldb); - }, + msg, + |err| decorate(tcx, overlap, impl_span, err), ); } }; |
