Skip to content

Commit eac4883

Browse files
authored
Unrolled build for rust-lang#126575
Rollup merge of rust-lang#126575 - fmease:update-lint-type_alias_bounds, r=compiler-errors Make it crystal clear what lint `type_alias_bounds` actually signifies This is part of my work on https://github.com/rust-lang/rust/labels/F-lazy_type_alias ([tracking issue](rust-lang#112792)). --- To recap, the lint `type_alias_bounds` detects bounds on generic parameters and where clauses on (eager) type aliases. These bounds should've never been allowed because they are currently neither enforced[^1] at usage sites of type aliases nor thoroughly checked for correctness at definition sites due to the way type aliases are represented in the compiler. Allowing them was an oversight. Explicitly label this as a known limitation of the type checker/system and establish the experimental feature `lazy_type_alias` as its eventual proper solution. Where this becomes a bit tricky (for me as a rustc dev) are the "secondary effects" of these bounds whose existence I sadly can't deny. As a matter of fact, type alias bounds do play some small roles during type checking. However, after a lot of thinking over the last two weeks I've come to the conclusion (not without second-guessing myself though) that these use cases should not trump the fact that these bounds are currently *inherently broken*. Therefore the lint `type_alias_bounds` should and will continue to flag bounds that may have subordinate uses. The two *known* secondary effects are: 1. They may enable the use of "shorthand" associated type paths `T::Assoc` (as opposed to fully qualified paths `<T as Trait>::Assoc`) where `T` is a type param bounded by some trait `Trait` which defines that assoc ty. 2. They may affect the default lifetime of trait object types passed as a type argument to the type alias. That concept is called (trait) object lifetime default. The second one is negligible, no question asked. The first one however is actually "kinda nice" (for writability) and comes up in practice from time to time. So why don't I just special-case trait bounds that "define" shorthand assoc type paths as originally planned in rust-lang#125709? 1. Starting to permit even a tiny subset of bounds would already be enough to send a signal to users that bounds in type aliases have been legitimized and that they can expect to see type alias bounds in the wild from now on (proliferation). This would be actively misleading and dangerous because those bounds don't behave at all like one would expect, they are *not real*[^2]! 1. Let's take `type A<T: Trait> = T::Proj;` for example. Everywhere else in the language `T: Trait` means `T: Trait + Sized`. For type aliases, that's not the case though: `T: Trait` and `T: Trait + ?Sized` for that matter do neither mean `T: Trait + Sized` nor `T: Trait + ?Sized` (for both!). Instead, whether `T` requires `Sized` or not entirely depends on the definition of `Trait`[^2]. Namely, whether or not it is bounded by `Sized`. 2. Given `type A<T: Trait<AssocA = ()>> = T::AssocB;`, while `X: Trait` gets checked given `A<X>` (by virtue of projection wfchecking post alias expansion[^2]), the associated type constraint `AssocA = ()` gets dropped entirely! While we could choose to warn on such cases, it would inevitably lead to a huge pile of special cases. 3. While it's common knowledge that the body / aliased type / RHS of an (eager) type alias does not get checked for well-formedness, I'm not sure if people would realize that that extends to bounds as well. Namely, `type A<T: Trait<[u8]>> = T::Proj;` compiles even if `Trait`'s generic parameter requires `Sized`. Of course, at usage sites `[u8]: Sized` would still end up getting checked[^2], so it's not a huge problem if you have full control over `A`. However, imagine that `A` was actually part of a public API and was never used inside the defining crate (not unreasonable). In such a scenario, downstream users would be presented with an impossible to use type alias! Remember, bounds may grow arbitrarily complex and nuanced in practice. 4. Even if we allowed trait bounds that "define" shorthand assoc type paths, we would still need to continue to warn in cases where the assoc ty comes from a supertrait despite the fact that the shorthand syntax can be used: `type A<T: Sub> = T::Assoc;` does compile given `trait Sub: Super {}` and `trait Super { type Assoc; }`. However, `A<X>` does not enforce `X: Sub`, only `X: Super`[^2]. All that to say, type alias bounds are simply not real and we shouldn't pretend they are! 5. Summarizing the points above, we would be legitimizing bounds that are completely broken! 2. It's infeasible to implement: Due to the lack of `TypeckResults` in `ItemCtxt` (and a way to propagate it to other parts of the compiler), the resolution of type-dependent paths in non-`Body` items (most notably type aliases) is not recoverable from the HIR alone which would be necessary because the information of whether an associated type path (projection) is a shorthand is only present pre&in-HIR and doesn't survive HIR ty lowering. Of course, I could rerun parts of HIR ty lowering inside the lint `type_alias_bounds` (namely, `probe_single_ty_param_bound_for_assoc_ty` which would need to be exposed or alternatively a stripped-down version of it). This likely has a performance impact and introduces complexity. In short, the "benefits" are not worth the costs. --- * 3rd commit: Update a diagnostic to avoid suggesting type alias bounds * 4th commit: Flag type alias bounds even if the RHS contains inherent associated types. * I started to allow them at some point in the past which was not correct (see commit for details) * 5th commit: Allow type alias bounds if the RHS contains const projections and GCEs are enabled * (and add a `FIXME(generic_const_exprs)` to be revisited before (M)GCE's stabilization) * As a matter of fact type alias bounds are enforced in this case because the contained AnonConsts do get checked for well-formedness and crucially they inherit the generics and predicates of their parent item (here: the type alias) * Remaining commits: Improve the lint `type_alias_bounds` itself --- Fixes rust-lang#125789 (sugg diag fix). Fixes rust-lang#125709 (wontfix, acknowledgement, sugg diag applic fix). Fixes rust-lang#104918 (sugg diag applic fix). Fixes rust-lang#100270 (wontfix, acknowledgement, sugg diag applic fix). Fixes rust-lang#94398 (true fix). r? `@compiler-errors` `@oli-obk` [^1]: From the perspective of the trait solver. [^2]: Given `type A<T: Trait> = T::Proj;`, the reason why the trait bound "`T: Trait`" gets *seemingly* enforced at usage sites of the type alias `A` is simply because `A<X>` gets expanded to "`<X as Trait>::Proj`" very early on and it's the *expansion* that gets checked for well-formedness, not the type alias reference.
2 parents 6ef11b8 + 5859dff commit eac4883

26 files changed

+848
-475
lines changed

compiler/rustc_hir_analysis/messages.ftl

+9-4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
hir_analysis_ambiguous_assoc_item = ambiguous associated {$assoc_kind} `{$assoc_name}` in bounds of `{$ty_param_name}`
1+
hir_analysis_ambiguous_assoc_item = ambiguous associated {$assoc_kind} `{$assoc_name}` in bounds of `{$qself}`
22
.label = ambiguous associated {$assoc_kind} `{$assoc_name}`
33
44
hir_analysis_ambiguous_lifetime_bound =
@@ -12,16 +12,21 @@ hir_analysis_assoc_item_is_private = {$kind} `{$name}` is private
1212
.label = private {$kind}
1313
.defined_here_label = the {$kind} is defined here
1414
15-
hir_analysis_assoc_item_not_found = associated {$assoc_kind} `{$assoc_name}` not found for `{$ty_param_name}`
15+
hir_analysis_assoc_item_not_found = associated {$assoc_kind} `{$assoc_name}` not found for `{$qself}`
1616
1717
hir_analysis_assoc_item_not_found_found_in_other_trait_label = there is {$identically_named ->
1818
[true] an
1919
*[false] a similarly named
2020
} associated {$assoc_kind} `{$suggested_name}` in the trait `{$trait_name}`
2121
hir_analysis_assoc_item_not_found_label = associated {$assoc_kind} `{$assoc_name}` not found
22-
hir_analysis_assoc_item_not_found_other_sugg = `{$ty_param_name}` has the following associated {$assoc_kind}
22+
hir_analysis_assoc_item_not_found_other_sugg = `{$qself}` has the following associated {$assoc_kind}
23+
hir_analysis_assoc_item_not_found_similar_in_other_trait_qpath_sugg =
24+
consider fully qualifying{$identically_named ->
25+
[true] {""}
26+
*[false] {" "}and renaming
27+
} the associated {$assoc_kind}
2328
hir_analysis_assoc_item_not_found_similar_in_other_trait_sugg = change the associated {$assoc_kind} name to use `{$suggested_name}` from `{$trait_name}`
24-
hir_analysis_assoc_item_not_found_similar_in_other_trait_with_bound_sugg = and also change the associated {$assoc_kind} name
29+
hir_analysis_assoc_item_not_found_similar_in_other_trait_with_bound_sugg = ...and changing the associated {$assoc_kind} name
2530
hir_analysis_assoc_item_not_found_similar_sugg = there is an associated {$assoc_kind} with a similar name
2631
2732
hir_analysis_assoc_kind_mismatch = expected {$expected}, found {$got}

compiler/rustc_hir_analysis/src/errors.rs

+25-6
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ pub struct AmbiguousAssocItem<'a> {
2222
pub span: Span,
2323
pub assoc_kind: &'static str,
2424
pub assoc_name: Ident,
25-
pub ty_param_name: &'a str,
25+
pub qself: &'a str,
2626
}
2727

2828
#[derive(Diagnostic)]
@@ -75,7 +75,7 @@ pub struct AssocItemNotFound<'a> {
7575
pub span: Span,
7676
pub assoc_name: Ident,
7777
pub assoc_kind: &'static str,
78-
pub ty_param_name: &'a str,
78+
pub qself: &'a str,
7979
#[subdiagnostic]
8080
pub label: Option<AssocItemNotFoundLabel<'a>>,
8181
#[subdiagnostic]
@@ -126,13 +126,32 @@ pub enum AssocItemNotFoundSugg<'a> {
126126
assoc_kind: &'static str,
127127
suggested_name: Symbol,
128128
},
129-
#[suggestion(hir_analysis_assoc_item_not_found_other_sugg, code = "{suggested_name}")]
129+
#[multipart_suggestion(
130+
hir_analysis_assoc_item_not_found_similar_in_other_trait_qpath_sugg,
131+
style = "verbose"
132+
)]
133+
SimilarInOtherTraitQPath {
134+
#[suggestion_part(code = "<")]
135+
lo: Span,
136+
#[suggestion_part(code = " as {trait_ref}>")]
137+
mi: Span,
138+
#[suggestion_part(code = "{suggested_name}")]
139+
hi: Option<Span>,
140+
trait_ref: String,
141+
suggested_name: Symbol,
142+
identically_named: bool,
143+
#[applicability]
144+
applicability: Applicability,
145+
},
146+
#[suggestion(
147+
hir_analysis_assoc_item_not_found_other_sugg,
148+
code = "{suggested_name}",
149+
applicability = "maybe-incorrect"
150+
)]
130151
Other {
131152
#[primary_span]
132153
span: Span,
133-
#[applicability]
134-
applicability: Applicability,
135-
ty_param_name: &'a str,
154+
qself: &'a str,
136155
assoc_kind: &'static str,
137156
suggested_name: Symbol,
138157
},

compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs

+3-6
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ use rustc_hir as hir;
66
use rustc_hir::def::{DefKind, Res};
77
use rustc_hir::def_id::{DefId, LocalDefId};
88
use rustc_middle::bug;
9-
use rustc_middle::ty::print::PrintTraitRefExt as _;
109
use rustc_middle::ty::{self as ty, IsSuggestable, Ty, TyCtxt};
1110
use rustc_span::symbol::Ident;
1211
use rustc_span::{ErrorGuaranteed, Span, Symbol};
@@ -16,9 +15,8 @@ use smallvec::SmallVec;
1615

1716
use crate::bounds::Bounds;
1817
use crate::errors;
19-
use crate::hir_ty_lowering::{HirTyLowerer, OnlySelfBounds, PredicateFilter};
20-
21-
use super::RegionInferReason;
18+
use crate::hir_ty_lowering::HirTyLowerer;
19+
use crate::hir_ty_lowering::{AssocItemQSelf, OnlySelfBounds, PredicateFilter, RegionInferReason};
2220

2321
impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
2422
/// Add a `Sized` bound to the `bounds` if appropriate.
@@ -288,8 +286,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
288286
// one that does define it.
289287
self.probe_single_bound_for_assoc_item(
290288
|| traits::supertraits(tcx, trait_ref),
291-
trait_ref.skip_binder().print_only_trait_name(),
292-
None,
289+
AssocItemQSelf::Trait(trait_ref.def_id()),
293290
assoc_kind,
294291
constraint.ident,
295292
path_span,

compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs

+78-49
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,17 @@ use crate::errors::{
33
ParenthesizedFnTraitExpansion, TraitObjectDeclaredWithNoTraits,
44
};
55
use crate::fluent_generated as fluent;
6-
use crate::hir_ty_lowering::HirTyLowerer;
6+
use crate::hir_ty_lowering::{AssocItemQSelf, HirTyLowerer};
77
use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
88
use rustc_data_structures::sorted_map::SortedMap;
99
use rustc_data_structures::unord::UnordMap;
1010
use rustc_errors::MultiSpan;
1111
use rustc_errors::{
1212
codes::*, pluralize, struct_span_code_err, Applicability, Diag, ErrorGuaranteed,
1313
};
14+
use rustc_hir as hir;
1415
use rustc_hir::def::{DefKind, Res};
15-
use rustc_hir::def_id::{DefId, LocalDefId};
16-
use rustc_hir::{self as hir, Node};
16+
use rustc_hir::def_id::DefId;
1717
use rustc_middle::bug;
1818
use rustc_middle::query::Key;
1919
use rustc_middle::ty::print::{PrintPolyTraitRefExt as _, PrintTraitRefExt as _};
@@ -116,8 +116,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
116116
pub(super) fn complain_about_assoc_item_not_found<I>(
117117
&self,
118118
all_candidates: impl Fn() -> I,
119-
ty_param_name: &str,
120-
ty_param_def_id: Option<LocalDefId>,
119+
qself: AssocItemQSelf,
121120
assoc_kind: ty::AssocKind,
122121
assoc_name: Ident,
123122
span: Span,
@@ -139,7 +138,8 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
139138
);
140139
}
141140

142-
let assoc_kind_str = super::assoc_kind_str(assoc_kind);
141+
let assoc_kind_str = assoc_kind_str(assoc_kind);
142+
let qself_str = qself.to_string(tcx);
143143

144144
// The fallback span is needed because `assoc_name` might be an `Fn()`'s `Output` without a
145145
// valid span, so we point at the whole path segment instead.
@@ -149,7 +149,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
149149
span: if is_dummy { span } else { assoc_name.span },
150150
assoc_name,
151151
assoc_kind: assoc_kind_str,
152-
ty_param_name,
152+
qself: &qself_str,
153153
label: None,
154154
sugg: None,
155155
};
@@ -219,19 +219,28 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
219219
suggested_name,
220220
identically_named: suggested_name == assoc_name.name,
221221
});
222-
let hir = tcx.hir();
223-
if let Some(def_id) = ty_param_def_id
224-
&& let parent = hir.get_parent_item(tcx.local_def_id_to_hir_id(def_id))
225-
&& let Some(generics) = hir.get_generics(parent.def_id)
222+
if let AssocItemQSelf::TyParam(ty_param_def_id, ty_param_span) = qself
223+
// Not using `self.item_def_id()` here as that would yield the opaque type itself if we're
224+
// inside an opaque type while we're interested in the overarching type alias (TAIT).
225+
// FIXME: However, for trait aliases, this incorrectly returns the enclosing module...
226+
&& let item_def_id =
227+
tcx.hir().get_parent_item(tcx.local_def_id_to_hir_id(ty_param_def_id))
228+
// FIXME: ...which obviously won't have any generics.
229+
&& let Some(generics) = tcx.hir().get_generics(item_def_id.def_id)
226230
{
227-
if generics.bounds_for_param(def_id).flat_map(|pred| pred.bounds.iter()).any(
228-
|b| match b {
231+
// FIXME: Suggest adding supertrait bounds if we have a `Self` type param.
232+
// FIXME(trait_alias): Suggest adding `Self: Trait` to
233+
// `trait Alias = where Self::Proj:;` with `trait Trait { type Proj; }`.
234+
if generics
235+
.bounds_for_param(ty_param_def_id)
236+
.flat_map(|pred| pred.bounds.iter())
237+
.any(|b| match b {
229238
hir::GenericBound::Trait(t, ..) => {
230239
t.trait_ref.trait_def_id() == Some(best_trait)
231240
}
232241
_ => false,
233-
},
234-
) {
242+
})
243+
{
235244
// The type param already has a bound for `trait_name`, we just need to
236245
// change the associated item.
237246
err.sugg = Some(errors::AssocItemNotFoundSugg::SimilarInOtherTrait {
@@ -242,48 +251,60 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
242251
return self.dcx().emit_err(err);
243252
}
244253

245-
let mut err = self.dcx().create_err(err);
246-
if suggest_constraining_type_param(
247-
tcx,
248-
generics,
249-
&mut err,
250-
&ty_param_name,
251-
&trait_name,
252-
None,
253-
None,
254-
) && suggested_name != assoc_name.name
254+
let trait_args = &ty::GenericArgs::identity_for_item(tcx, best_trait)[1..];
255+
let mut trait_ref = trait_name.clone();
256+
let applicability = if let [arg, args @ ..] = trait_args {
257+
use std::fmt::Write;
258+
write!(trait_ref, "</* {arg}").unwrap();
259+
args.iter().try_for_each(|arg| write!(trait_ref, ", {arg}")).unwrap();
260+
trait_ref += " */>";
261+
Applicability::HasPlaceholders
262+
} else {
263+
Applicability::MaybeIncorrect
264+
};
265+
266+
let identically_named = suggested_name == assoc_name.name;
267+
268+
if let DefKind::TyAlias = tcx.def_kind(item_def_id)
269+
&& !tcx.type_alias_is_lazy(item_def_id)
255270
{
256-
// We suggested constraining a type parameter, but the associated item on it
257-
// was also not an exact match, so we also suggest changing it.
258-
err.span_suggestion_verbose(
259-
assoc_name.span,
260-
fluent::hir_analysis_assoc_item_not_found_similar_in_other_trait_with_bound_sugg,
271+
err.sugg = Some(errors::AssocItemNotFoundSugg::SimilarInOtherTraitQPath {
272+
lo: ty_param_span.shrink_to_lo(),
273+
mi: ty_param_span.shrink_to_hi(),
274+
hi: (!identically_named).then_some(assoc_name.span),
275+
trait_ref,
276+
identically_named,
261277
suggested_name,
262-
Applicability::MaybeIncorrect,
263-
);
278+
applicability,
279+
});
280+
} else {
281+
let mut err = self.dcx().create_err(err);
282+
if suggest_constraining_type_param(
283+
tcx, generics, &mut err, &qself_str, &trait_ref, None, None,
284+
) && !identically_named
285+
{
286+
// We suggested constraining a type parameter, but the associated item on it
287+
// was also not an exact match, so we also suggest changing it.
288+
err.span_suggestion_verbose(
289+
assoc_name.span,
290+
fluent::hir_analysis_assoc_item_not_found_similar_in_other_trait_with_bound_sugg,
291+
suggested_name,
292+
Applicability::MaybeIncorrect,
293+
);
294+
}
295+
return err.emit();
264296
}
265-
return err.emit();
266297
}
267298
return self.dcx().emit_err(err);
268299
}
269300
}
270301

271302
// If we still couldn't find any associated item, and only one associated item exists,
272-
// suggests using it.
303+
// suggest using it.
273304
if let [candidate_name] = all_candidate_names.as_slice() {
274-
// This should still compile, except on `#![feature(associated_type_defaults)]`
275-
// where it could suggests `type A = Self::A`, thus recursing infinitely.
276-
let applicability =
277-
if assoc_kind == ty::AssocKind::Type && tcx.features().associated_type_defaults {
278-
Applicability::Unspecified
279-
} else {
280-
Applicability::MaybeIncorrect
281-
};
282-
283305
err.sugg = Some(errors::AssocItemNotFoundSugg::Other {
284306
span: assoc_name.span,
285-
applicability,
286-
ty_param_name,
307+
qself: &qself_str,
287308
assoc_kind: assoc_kind_str,
288309
suggested_name: *candidate_name,
289310
});
@@ -349,10 +370,10 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
349370

350371
self.dcx().emit_err(errors::AssocKindMismatch {
351372
span,
352-
expected: super::assoc_kind_str(expected),
353-
got: super::assoc_kind_str(got),
373+
expected: assoc_kind_str(expected),
374+
got: assoc_kind_str(got),
354375
expected_because_label,
355-
assoc_kind: super::assoc_kind_str(assoc_item.kind),
376+
assoc_kind: assoc_kind_str(assoc_item.kind),
356377
def_span: tcx.def_span(assoc_item.def_id),
357378
bound_on_assoc_const_label,
358379
wrap_in_braces_sugg,
@@ -746,7 +767,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
746767
if let ([], [bound]) = (&potential_assoc_types[..], &trait_bounds) {
747768
let grandparent = tcx.parent_hir_node(tcx.parent_hir_id(bound.trait_ref.hir_ref_id));
748769
in_expr_or_pat = match grandparent {
749-
Node::Expr(_) | Node::Pat(_) => true,
770+
hir::Node::Expr(_) | hir::Node::Pat(_) => true,
750771
_ => false,
751772
};
752773
match bound.trait_ref.path.segments {
@@ -1612,3 +1633,11 @@ fn generics_args_err_extend<'a>(
16121633
_ => {}
16131634
}
16141635
}
1636+
1637+
pub(super) fn assoc_kind_str(kind: ty::AssocKind) -> &'static str {
1638+
match kind {
1639+
ty::AssocKind::Fn => "function",
1640+
ty::AssocKind::Const => "constant",
1641+
ty::AssocKind::Type => "type",
1642+
}
1643+
}

0 commit comments

Comments
 (0)