Skip to content

Commit

Permalink
Auto merge of #124356 - fmease:fewer-magic-numbers-in-names, r=lcnr
Browse files Browse the repository at this point in the history
Cleanup: Replace item names referencing GitHub issues or error codes with something more meaningful

**lcnr** in #117164 (review):

> […] while I know that there's precendent to name things `Issue69420`, I really dislike this as it requires looking up the issue to figure out the purpose of such a variant. Actually referring to the underlying issue, e.g. `AliasMayNormToUncovered` or whatever and then linking to the issue in a doc comment feels a lot more desirable to me. We should ideally rename all the functions and enums which currently use issue numbers.

I've grepped through `compiler/` like crazy and think that I've found all instances of this pattern.
However, I haven't renamed `compute_2229_migrations_*`. Should I?

The first commit introduces an abhorrent and super long name for an item because naming is hard but also scary looking / unwelcoming names are good for things related to temporary-ish backcompat hacks. I'll let you discover it by yourself.

Contains a bit of drive-by cleanup and a diag migration bc that was the simplest option.

r? lcnr or compiler
  • Loading branch information
bors committed May 1, 2024
2 parents f705de5 + 2a1d748 commit f5355b9
Show file tree
Hide file tree
Showing 27 changed files with 111 additions and 105 deletions.
11 changes: 5 additions & 6 deletions compiler/rustc_ast_passes/src/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -632,20 +632,19 @@ impl<'a> AstValidator<'a> {
}
}

fn emit_e0568(&self, span: Span, ident: Span) {
self.dcx().emit_err(errors::AutoTraitBounds { span, ident });
}

fn deny_super_traits(&self, bounds: &GenericBounds, ident_span: Span) {
if let [.., last] = &bounds[..] {
let span = ident_span.shrink_to_hi().to(last.span());
self.emit_e0568(span, ident_span);
self.dcx().emit_err(errors::AutoTraitBounds { span, ident: ident_span });
}
}

fn deny_where_clause(&self, where_clause: &WhereClause, ident_span: Span) {
if !where_clause.predicates.is_empty() {
self.emit_e0568(where_clause.span, ident_span);
// FIXME: The current diagnostic is misleading since it only talks about
// super trait and lifetime bounds while we should just say “bounds”.
self.dcx()
.emit_err(errors::AutoTraitBounds { span: where_clause.span, ident: ident_span });
}
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_data_structures/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,9 @@ pub fn make_display(f: impl Fn(&mut fmt::Formatter<'_>) -> fmt::Result) -> impl
Printer { f }
}

// See comments in compiler/rustc_middle/src/tests.rs
// See comment in compiler/rustc_middle/src/tests.rs and issue #27438.
#[doc(hidden)]
pub fn __noop_fix_for_27438() {}
pub fn __noop_fix_for_windows_dllimport_issue() {}

#[macro_export]
macro_rules! external_bitflags_debug {
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,12 @@ hir_analysis_inherent_ty_outside_relevant = cannot define inherent `impl` for a
.help = consider moving this inherent impl into the crate defining the type if possible
.span_help = alternatively add `#[rustc_allow_incoherent_impl]` to the relevant impl items
hir_analysis_invalid_receiver_ty = invalid `self` parameter type: `{$receiver_ty}`
.note = type of `self` must be `Self` or a type that dereferences to it
hir_analysis_invalid_receiver_ty_help =
consider changing to `self`, `&self`, `&mut self`, `self: Box<Self>`, `self: Rc<Self>`, `self: Arc<Self>`, or `self: Pin<P>` (where P is one of the previous types except `Self`)
hir_analysis_invalid_union_field =
field must implement `Copy` or be wrapped in `ManuallyDrop<...>` to be used in a union
.note = union fields must not have drop side-effects, which is currently enforced via either `Copy` or `ManuallyDrop<...>`
Expand Down
18 changes: 4 additions & 14 deletions compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::autoderef::Autoderef;
use crate::collect::CollectItemTypesVisitor;
use crate::constrained_generic_params::{identify_constrained_generic_params, Parameter};
use crate::errors;
use crate::fluent_generated as fluent;

use hir::intravisit::Visitor;
use rustc_ast as ast;
Expand Down Expand Up @@ -1636,10 +1637,6 @@ fn check_fn_or_method<'tcx>(
}
}

const HELP_FOR_SELF_TYPE: &str = "consider changing to `self`, `&self`, `&mut self`, `self: Box<Self>`, \
`self: Rc<Self>`, `self: Arc<Self>`, or `self: Pin<P>` (where P is one \
of the previous types except `Self`)";

#[instrument(level = "debug", skip(wfcx))]
fn check_method_receiver<'tcx>(
wfcx: &WfCheckingCtxt<'_, 'tcx>,
Expand Down Expand Up @@ -1675,7 +1672,7 @@ fn check_method_receiver<'tcx>(
if tcx.features().arbitrary_self_types {
if !receiver_is_valid(wfcx, span, receiver_ty, self_ty, true) {
// Report error; `arbitrary_self_types` was enabled.
return Err(e0307(tcx, span, receiver_ty));
return Err(tcx.dcx().emit_err(errors::InvalidReceiverTy { span, receiver_ty }));
}
} else {
if !receiver_is_valid(wfcx, span, receiver_ty, self_ty, false) {
Expand All @@ -1690,24 +1687,17 @@ fn check_method_receiver<'tcx>(
the `arbitrary_self_types` feature",
),
)
.with_help(HELP_FOR_SELF_TYPE)
.with_help(fluent::hir_analysis_invalid_receiver_ty_help)
.emit()
} else {
// Report error; would not have worked with `arbitrary_self_types`.
e0307(tcx, span, receiver_ty)
tcx.dcx().emit_err(errors::InvalidReceiverTy { span, receiver_ty })
});
}
}
Ok(())
}

fn e0307(tcx: TyCtxt<'_>, span: Span, receiver_ty: Ty<'_>) -> ErrorGuaranteed {
struct_span_code_err!(tcx.dcx(), span, E0307, "invalid `self` parameter type: {receiver_ty}")
.with_note("type of `self` must be `Self` or a type that dereferences to it")
.with_help(HELP_FOR_SELF_TYPE)
.emit()
}

/// Returns whether `receiver_ty` would be considered a valid receiver type for `self_ty`. If
/// `arbitrary_self_types` is enabled, `receiver_ty` must transitively deref to `self_ty`, possibly
/// through a `*const/mut T` raw pointer. If the feature is not enabled, the requirements are more
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1666,3 +1666,13 @@ pub struct NonConstRange {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(hir_analysis_invalid_receiver_ty, code = E0307)]
#[note]
#[help(hir_analysis_invalid_receiver_ty_help)]
pub struct InvalidReceiverTy<'tcx> {
#[primary_span]
pub span: Span,
pub receiver_ty: Ty<'tcx>,
}
14 changes: 10 additions & 4 deletions compiler/rustc_hir_typeck/src/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1228,16 +1228,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
);
}
} else {
// Pattern has wrong number of fields.
let e =
self.e0023(pat.span, res, qpath, subpats, &variant.fields.raw, expected, had_err);
let e = self.emit_err_pat_wrong_number_of_fields(
pat.span,
res,
qpath,
subpats,
&variant.fields.raw,
expected,
had_err,
);
on_error(e);
return Ty::new_error(tcx, e);
}
pat_ty
}

fn e0023(
fn emit_err_pat_wrong_number_of_fields(
&self,
pat_span: Span,
res: Res,
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -847,8 +847,10 @@ rustc_queries! {
separate_provide_extern
}

query issue33140_self_ty(key: DefId) -> Option<ty::EarlyBinder<ty::Ty<'tcx>>> {
desc { |tcx| "computing Self type wrt issue #33140 `{}`", tcx.def_path_str(key) }
query self_ty_of_trait_impl_enabling_order_dep_trait_object_hack(
key: DefId
) -> Option<ty::EarlyBinder<ty::Ty<'tcx>>> {
desc { |tcx| "computing self type wrt issue #33140 `{}`", tcx.def_path_str(key) }
}

/// Maps a `DefId` of a type to a list of its inherent impls.
Expand Down
14 changes: 6 additions & 8 deletions compiler/rustc_middle/src/tests.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
// FIXME(#27438): right now the unit tests of rustc_middle don't refer to any actual
// functions generated in rustc_data_structures (all
// references are through generic functions), but statics are
// referenced from time to time. Due to this bug we won't
// actually correctly link in the statics unless we also
// reference a function, so be sure to reference a dummy
// function.
// FIXME(#27438): Right now, the unit tests of `rustc_middle` don't refer to any actual functions
// generated in `rustc_data_structures` (all references are through generic functions),
// but statics are referenced from time to time. Due to this Windows `dllimport` bug
// we won't actually correctly link in the statics unless we also reference a function,
// so be sure to reference a dummy function.
#[test]
fn noop() {
rustc_data_structures::__noop_fix_for_27438();
rustc_data_structures::__noop_fix_for_windows_dllimport_issue();
}
43 changes: 21 additions & 22 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1503,14 +1503,14 @@ pub enum ImplOverlapKind {
/// Whether or not the impl is permitted due to the trait being a `#[marker]` trait
marker: bool,
},
/// These impls are allowed to overlap, but that raises
/// an issue #33140 future-compatibility warning.
/// These impls are allowed to overlap, but that raises an
/// issue #33140 future-compatibility warning (tracked in #56484).
///
/// Some background: in Rust 1.0, the trait-object types `Send + Sync` (today's
/// `dyn Send + Sync`) and `Sync + Send` (now `dyn Sync + Send`) were different.
///
/// The widely-used version 0.1.0 of the crate `traitobject` had accidentally relied
/// that difference, making what reduces to the following set of impls:
/// The widely-used version 0.1.0 of the crate `traitobject` had accidentally relied on
/// that difference, doing what reduces to the following set of impls:
///
/// ```compile_fail,(E0119)
/// trait Trait {}
Expand All @@ -1535,7 +1535,7 @@ pub enum ImplOverlapKind {
/// 4. Neither of the impls can have any where-clauses.
///
/// Once `traitobject` 0.1.0 is no longer an active concern, this hack can be removed.
Issue33140,
FutureCompatOrderDepTraitObjects,
}

/// Useful source information about where a desugared associated type for an
Expand Down Expand Up @@ -1730,27 +1730,26 @@ impl<'tcx> TyCtxt<'tcx> {
| (ImplPolarity::Negative, ImplPolarity::Negative) => {}
};

let is_marker_overlap = {
let is_marker_impl =
|trait_ref: TraitRef<'_>| -> bool { self.trait_def(trait_ref.def_id).is_marker };
is_marker_impl(trait_ref1) && is_marker_impl(trait_ref2)
};
let is_marker_impl = |trait_ref: TraitRef<'_>| self.trait_def(trait_ref.def_id).is_marker;
let is_marker_overlap = is_marker_impl(trait_ref1) && is_marker_impl(trait_ref2);

if is_marker_overlap {
Some(ImplOverlapKind::Permitted { marker: true })
} else {
if let Some(self_ty1) = self.issue33140_self_ty(def_id1) {
if let Some(self_ty2) = self.issue33140_self_ty(def_id2) {
if self_ty1 == self_ty2 {
return Some(ImplOverlapKind::Issue33140);
} else {
debug!("found {self_ty1:?} != {self_ty2:?}");
}
}
}
return Some(ImplOverlapKind::Permitted { marker: true });
}

None
if let Some(self_ty1) =
self.self_ty_of_trait_impl_enabling_order_dep_trait_object_hack(def_id1)
&& let Some(self_ty2) =
self.self_ty_of_trait_impl_enabling_order_dep_trait_object_hack(def_id2)
{
if self_ty1 == self_ty2 {
return Some(ImplOverlapKind::FutureCompatOrderDepTraitObjects);
} else {
debug!("found {self_ty1:?} != {self_ty2:?}");
}
}

None
}

/// Returns `ty::VariantDef` if `res` refers to a struct,
Expand Down
16 changes: 5 additions & 11 deletions compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ use rustc_arena::{DroplessArena, TypedArena};
use rustc_ast::Mutability;
use rustc_data_structures::fx::FxIndexSet;
use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_errors::{
codes::*, struct_span_code_err, Applicability, Diag, ErrorGuaranteed, MultiSpan,
};
use rustc_errors::{codes::*, struct_span_code_err, Applicability, ErrorGuaranteed, MultiSpan};
use rustc_hir::def::*;
use rustc_hir::def_id::LocalDefId;
use rustc_hir::{self as hir, BindingMode, ByRef, HirId};
Expand All @@ -24,7 +22,6 @@ use rustc_middle::ty::{self, AdtDef, Ty, TyCtxt};
use rustc_session::lint::builtin::{
BINDINGS_WITH_VARIANT_NAME, IRREFUTABLE_LET_PATTERNS, UNREACHABLE_PATTERNS,
};
use rustc_session::Session;
use rustc_span::hygiene::DesugaringKind;
use rustc_span::{sym, Span};

Expand Down Expand Up @@ -64,10 +61,6 @@ pub(crate) fn check_match(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Result<(), Err
visitor.error
}

fn create_e0004(sess: &Session, sp: Span, error_message: String) -> Diag<'_> {
struct_span_code_err!(sess.dcx(), sp, E0004, "{}", &error_message)
}

#[derive(Debug, Copy, Clone, PartialEq)]
enum RefutableFlag {
Irrefutable,
Expand Down Expand Up @@ -975,10 +968,11 @@ fn report_non_exhaustive_match<'p, 'tcx>(

// FIXME: migration of this diagnostic will require list support
let joined_patterns = joined_uncovered_patterns(cx, &witnesses);
let mut err = create_e0004(
cx.tcx.sess,
let mut err = struct_span_code_err!(
cx.tcx.dcx(),
sp,
format!("non-exhaustive patterns: {joined_patterns} not covered"),
E0004,
"non-exhaustive patterns: {joined_patterns} not covered"
);
err.span_label(
sp,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2001,7 +2001,7 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
// any associated items and there are no where-clauses.
//
// We can just arbitrarily drop one of the impls.
Some(ty::ImplOverlapKind::Issue33140) => {
Some(ty::ImplOverlapKind::FutureCompatOrderDepTraitObjects) => {
assert_eq!(other.evaluation, victim.evaluation);
DropVictim::Yes
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_trait_selection/src/traits/specialize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ fn report_conflicting_impls<'tcx>(
overlap.trait_ref.print_trait_sugared(),
overlap.self_ty.map_or_else(String::new, |ty| format!(" for type `{ty}`")),
match used_to_be_allowed {
Some(FutureCompatOverlapErrorKind::Issue33140) => ": (E0119)",
Some(FutureCompatOverlapErrorKind::OrderDepTraitObjects) => ": (E0119)",
_ => "",
}
)
Expand All @@ -480,7 +480,7 @@ fn report_conflicting_impls<'tcx>(
}
Some(kind) => {
let lint = match kind {
FutureCompatOverlapErrorKind::Issue33140 => ORDER_DEPENDENT_TRAIT_OBJECTS,
FutureCompatOverlapErrorKind::OrderDepTraitObjects => ORDER_DEPENDENT_TRAIT_OBJECTS,
FutureCompatOverlapErrorKind::LeakCheck => COHERENCE_LEAK_CHECK,
};
tcx.node_span_lint(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub use rustc_middle::traits::specialization_graph::*;

#[derive(Copy, Clone, Debug)]
pub enum FutureCompatOverlapErrorKind {
Issue33140,
OrderDepTraitObjects,
LeakCheck,
}

Expand Down Expand Up @@ -150,10 +150,10 @@ impl<'tcx> Children {
{
match overlap_kind {
ty::ImplOverlapKind::Permitted { marker: _ } => {}
ty::ImplOverlapKind::Issue33140 => {
ty::ImplOverlapKind::FutureCompatOrderDepTraitObjects => {
*last_lint_mut = Some(FutureCompatOverlapError {
error: create_overlap_error(overlap),
kind: FutureCompatOverlapErrorKind::Issue33140,
kind: FutureCompatOverlapErrorKind::OrderDepTraitObjects,
});
}
}
Expand Down
Loading

0 comments on commit f5355b9

Please sign in to comment.