Skip to content

Commit

Permalink
Unrolled build for rust-lang#122120
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#122120 - fmease:sugg-assoc-ty-bound-on-eq-bound, r=compiler-errors

Suggest associated type bounds on problematic associated equality bounds

Fixes rust-lang#105056. TL;DR: Suggest `Trait<Ty: Bound>` on `Trait<Ty = Bound>` in Rust >=2021.

~~Blocked on rust-lang#122055 (stabilization of `associated_type_bounds`), I'd say.~~ (merged)
  • Loading branch information
rust-timer authored Mar 27, 2024
2 parents 47ecded + 3879acb commit 455e977
Show file tree
Hide file tree
Showing 10 changed files with 287 additions and 99 deletions.
22 changes: 8 additions & 14 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2289,21 +2289,15 @@ pub enum ImplItemKind<'hir> {
Type(&'hir Ty<'hir>),
}

/// Bind a type to an associated type (i.e., `A = Foo`).
/// An associated item binding.
///
/// Bindings like `A: Debug` are represented as a special type `A =
/// $::Debug` that is understood by the HIR ty lowering code.
/// ### Examples
///
/// FIXME(alexreg): why have a separate type for the binding case,
/// wouldn't it be better to make the `ty` field an enum like the
/// following?
///
/// ```ignore (pseudo-rust)
/// enum TypeBindingKind {
/// Equals(...),
/// Binding(...),
/// }
/// ```
/// * `Trait<A = Ty, B = Ty>`
/// * `Trait<G<Ty> = Ty>`
/// * `Trait<A: Bound>`
/// * `Trait<C = { Ct }>` (under feature `associated_const_equality`)
/// * `Trait<f(): Bound>` (under feature `return_type_notation`)
#[derive(Debug, Clone, Copy, HashStable_Generic)]
pub struct TypeBinding<'hir> {
pub hir_id: HirId,
Expand Down Expand Up @@ -2336,7 +2330,7 @@ impl<'hir> From<AnonConst> for Term<'hir> {
pub enum TypeBindingKind<'hir> {
/// E.g., `Foo<Bar: Send>`.
Constraint { bounds: &'hir [GenericBound<'hir>] },
/// E.g., `Foo<Bar = ()>`, `Foo<Bar = ()>`
/// E.g., `Foo<Bar = ()>`.
Equality { term: Term<'hir> },
}

Expand Down
169 changes: 107 additions & 62 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,85 @@ use rustc_trait_selection::traits::error_reporting::suggestions::NextTypeParamNa
use super::HirTyLowerer;

impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
/// Prohibit or lint against *bare* trait object types depending on the edition.
///
/// *Bare* trait object types are ones that aren't preceeded by the keyword `dyn`.
/// In edition 2021 and onward we emit a hard error for them.
pub(super) fn prohibit_or_lint_bare_trait_object_ty(
&self,
self_ty: &hir::Ty<'_>,
in_path: bool,
) {
let tcx = self.tcx();

let hir::TyKind::TraitObject([poly_trait_ref, ..], _, TraitObjectSyntax::None) =
self_ty.kind
else {
return;
};

let needs_bracket = in_path
&& !tcx
.sess
.source_map()
.span_to_prev_source(self_ty.span)
.ok()
.is_some_and(|s| s.trim_end().ends_with('<'));

let is_global = poly_trait_ref.trait_ref.path.is_global();

let mut sugg = vec![(
self_ty.span.shrink_to_lo(),
format!(
"{}dyn {}",
if needs_bracket { "<" } else { "" },
if is_global { "(" } else { "" },
),
)];

if is_global || needs_bracket {
sugg.push((
self_ty.span.shrink_to_hi(),
format!(
"{}{}",
if is_global { ")" } else { "" },
if needs_bracket { ">" } else { "" },
),
));
}

if self_ty.span.edition().at_least_rust_2021() {
let msg = "trait objects must include the `dyn` keyword";
let label = "add `dyn` keyword before this trait";
let mut diag =
rustc_errors::struct_span_code_err!(tcx.dcx(), self_ty.span, E0782, "{}", msg);
if self_ty.span.can_be_used_for_suggestions()
&& !self.maybe_suggest_impl_trait(self_ty, &mut diag)
{
// FIXME: Only emit this suggestion if the trait is object safe.
diag.multipart_suggestion_verbose(label, sugg, Applicability::MachineApplicable);
}
// Check if the impl trait that we are considering is an impl of a local trait.
self.maybe_suggest_blanket_trait_impl(self_ty, &mut diag);
self.maybe_suggest_assoc_ty_bound(self_ty, &mut diag);
diag.stash(self_ty.span, StashKey::TraitMissingMethod);
} else {
let msg = "trait objects without an explicit `dyn` are deprecated";
tcx.node_span_lint(BARE_TRAIT_OBJECTS, self_ty.hir_id, self_ty.span, msg, |lint| {
if self_ty.span.can_be_used_for_suggestions() {
lint.multipart_suggestion_verbose(
"if this is an object-safe trait, use `dyn`",
sugg,
Applicability::MachineApplicable,
);
}
self.maybe_suggest_blanket_trait_impl(self_ty, lint);
});
}
}

/// Make sure that we are in the condition to suggest the blanket implementation.
pub(super) fn maybe_lint_blanket_trait_impl<G: EmissionGuarantee>(
fn maybe_suggest_blanket_trait_impl<G: EmissionGuarantee>(
&self,
self_ty: &hir::Ty<'_>,
diag: &mut Diag<'_, G>,
Expand Down Expand Up @@ -75,9 +152,11 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
}

/// Make sure that we are in the condition to suggest `impl Trait`.
fn maybe_lint_impl_trait(&self, self_ty: &hir::Ty<'_>, diag: &mut Diag<'_>) -> bool {
fn maybe_suggest_impl_trait(&self, self_ty: &hir::Ty<'_>, diag: &mut Diag<'_>) -> bool {
let tcx = self.tcx();
let parent_id = tcx.hir().get_parent_item(self_ty.hir_id).def_id;
// FIXME: If `type_alias_impl_trait` is enabled, also look for `Trait0<Ty = Trait1>`
// and suggest `Trait0<Ty = impl Trait1>`.
let (sig, generics, owner) = match tcx.hir_node_by_def_id(parent_id) {
hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(sig, generics, _), .. }) => {
(sig, generics, None)
Expand Down Expand Up @@ -186,71 +265,37 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
false
}

pub(super) fn maybe_lint_bare_trait(&self, self_ty: &hir::Ty<'_>, in_path: bool) {
let tcx = self.tcx();
if let hir::TyKind::TraitObject([poly_trait_ref, ..], _, TraitObjectSyntax::None) =
self_ty.kind
{
let needs_bracket = in_path
&& !tcx
.sess
.source_map()
.span_to_prev_source(self_ty.span)
.ok()
.is_some_and(|s| s.trim_end().ends_with('<'));

let is_global = poly_trait_ref.trait_ref.path.is_global();

let mut sugg = Vec::from_iter([(
self_ty.span.shrink_to_lo(),
format!(
"{}dyn {}",
if needs_bracket { "<" } else { "" },
if is_global { "(" } else { "" },
),
)]);
fn maybe_suggest_assoc_ty_bound(&self, self_ty: &hir::Ty<'_>, diag: &mut Diag<'_>) {
let mut parents = self.tcx().hir().parent_iter(self_ty.hir_id);

if is_global || needs_bracket {
sugg.push((
self_ty.span.shrink_to_hi(),
format!(
"{}{}",
if is_global { ")" } else { "" },
if needs_bracket { ">" } else { "" },
),
));
if let Some((_, hir::Node::TypeBinding(binding))) = parents.next()
&& let hir::TypeBindingKind::Equality { term: hir::Term::Ty(obj_ty) } = binding.kind
{
if let Some((_, hir::Node::TraitRef(..))) = parents.next()
&& let Some((_, hir::Node::Ty(ty))) = parents.next()
&& let hir::TyKind::TraitObject(..) = ty.kind
{
// Assoc ty bounds aren't permitted inside trait object types.
return;
}

if self_ty.span.edition().at_least_rust_2021() {
let msg = "trait objects must include the `dyn` keyword";
let label = "add `dyn` keyword before this trait";
let mut diag =
rustc_errors::struct_span_code_err!(tcx.dcx(), self_ty.span, E0782, "{}", msg);
if self_ty.span.can_be_used_for_suggestions()
&& !self.maybe_lint_impl_trait(self_ty, &mut diag)
{
diag.multipart_suggestion_verbose(
label,
sugg,
Applicability::MachineApplicable,
);
}
// check if the impl trait that we are considering is a impl of a local trait
self.maybe_lint_blanket_trait_impl(self_ty, &mut diag);
diag.stash(self_ty.span, StashKey::TraitMissingMethod);
let lo = if binding.gen_args.span_ext.is_dummy() {
binding.ident.span
} else {
let msg = "trait objects without an explicit `dyn` are deprecated";
tcx.node_span_lint(BARE_TRAIT_OBJECTS, self_ty.hir_id, self_ty.span, msg, |lint| {
if self_ty.span.can_be_used_for_suggestions() {
lint.multipart_suggestion_verbose(
"if this is an object-safe trait, use `dyn`",
sugg,
Applicability::MachineApplicable,
);
}
self.maybe_lint_blanket_trait_impl(self_ty, lint);
});
binding.gen_args.span_ext
};
let hi = obj_ty.span;

if !lo.eq_ctxt(hi) {
return;
}

diag.span_suggestion_verbose(
lo.between(hi),
"you might have meant to write a bound here",
": ",
Applicability::MaybeIncorrect,
);
}
}
}
4 changes: 2 additions & 2 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2339,12 +2339,12 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
)
}
hir::TyKind::TraitObject(bounds, lifetime, repr) => {
self.maybe_lint_bare_trait(hir_ty, in_path);
self.prohibit_or_lint_bare_trait_object_ty(hir_ty, in_path);

let repr = match repr {
TraitObjectSyntax::Dyn | TraitObjectSyntax::None => ty::Dyn,
TraitObjectSyntax::DynStar => ty::DynStar,
};

self.lower_trait_object_ty(
hir_ty.span,
hir_ty.hir_id,
Expand Down
10 changes: 6 additions & 4 deletions compiler/rustc_parse/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@ parse_array_index_offset_of = array indexing not supported in offset_of
parse_assignment_else_not_allowed = <assignment> ... else {"{"} ... {"}"} is not allowed
parse_assoc_lifetime = associated lifetimes are not supported
.label = the lifetime is given here
.help = if you meant to specify a trait object, write `dyn Trait + 'lifetime`
parse_associated_static_item_not_allowed = associated `static` items are not allowed
parse_async_block_in_2015 = `async` blocks are only allowed in Rust 2018 or later
Expand Down Expand Up @@ -445,6 +441,12 @@ parse_lifetime_in_borrow_expression = borrow expressions cannot be annotated wit
.suggestion = remove the lifetime annotation
.label = annotated with lifetime here
parse_lifetime_in_eq_constraint = lifetimes are not permitted in this context
.label = lifetime is not allowed here
.context_label = this introduces an associated item binding
.help = if you meant to specify a trait object, write `dyn /* Trait */ + {$lifetime}`
.colon_sugg = you might have meant to write a bound here
parse_lone_slash = invalid trailing slash in literal
.label = {parse_lone_slash}
Expand Down
17 changes: 13 additions & 4 deletions compiler/rustc_parse/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2631,13 +2631,22 @@ pub(crate) struct GenericsInPath {
}

#[derive(Diagnostic)]
#[diag(parse_assoc_lifetime)]
#[diag(parse_lifetime_in_eq_constraint)]
#[help]
pub(crate) struct AssocLifetime {
pub(crate) struct LifetimeInEqConstraint {
#[primary_span]
pub span: Span,
#[label]
pub lifetime: Span,
pub span: Span,
pub lifetime: Ident,
#[label(parse_context_label)]
pub binding_label: Span,
#[suggestion(
parse_colon_sugg,
style = "verbose",
applicability = "maybe-incorrect",
code = ": "
)]
pub colon_sugg: Span,
}

#[derive(Diagnostic)]
Expand Down
24 changes: 18 additions & 6 deletions compiler/rustc_parse/src/parser/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,11 @@ impl<'a> Parser<'a> {
let bounds = self.parse_generic_bounds()?;
AssocConstraintKind::Bound { bounds }
} else if self.eat(&token::Eq) {
self.parse_assoc_equality_term(ident, self.prev_token.span)?
self.parse_assoc_equality_term(
ident,
gen_args.as_ref(),
self.prev_token.span,
)?
} else {
unreachable!();
};
Expand Down Expand Up @@ -753,11 +757,13 @@ impl<'a> Parser<'a> {
}

/// Parse the term to the right of an associated item equality constraint.
/// That is, parse `<term>` in `Item = <term>`.
/// Right now, this only admits types in `<term>`.
///
/// That is, parse `$term` in `Item = $term` where `$term` is a type or
/// a const expression (wrapped in curly braces if complex).
fn parse_assoc_equality_term(
&mut self,
ident: Ident,
gen_args: Option<&GenericArgs>,
eq: Span,
) -> PResult<'a, AssocConstraintKind> {
let arg = self.parse_generic_arg(None)?;
Expand All @@ -769,9 +775,15 @@ impl<'a> Parser<'a> {
c.into()
}
Some(GenericArg::Lifetime(lt)) => {
let guar =
self.dcx().emit_err(errors::AssocLifetime { span, lifetime: lt.ident.span });
self.mk_ty(span, ast::TyKind::Err(guar)).into()
let guar = self.dcx().emit_err(errors::LifetimeInEqConstraint {
span: lt.ident.span,
lifetime: lt.ident,
binding_label: span,
colon_sugg: gen_args
.map_or(ident.span, |args| args.span())
.between(lt.ident.span),
});
self.mk_ty(lt.ident.span, ast::TyKind::Err(guar)).into()
}
None => {
let after_eq = eq.shrink_to_hi();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Regression test for issue #105056.
//@ edition: 2021

fn f(_: impl Trait<T = Copy>) {}
//~^ ERROR trait objects must include the `dyn` keyword
//~| HELP add `dyn` keyword before this trait
//~| HELP you might have meant to write a bound here
//~| ERROR the trait `Copy` cannot be made into an object

fn g(_: impl Trait<T = std::fmt::Debug + Eq>) {}
//~^ ERROR trait objects must include the `dyn` keyword
//~| HELP add `dyn` keyword before this trait
//~| HELP you might have meant to write a bound here
//~| ERROR only auto traits can be used as additional traits in a trait object
//~| HELP consider creating a new trait
//~| ERROR the trait `Eq` cannot be made into an object

fn h(_: impl Trait<T<> = 'static + for<'a> Fn(&'a ())>) {}
//~^ ERROR trait objects must include the `dyn` keyword
//~| HELP add `dyn` keyword before this trait
//~| HELP you might have meant to write a bound here

// Don't suggest assoc ty bound in trait object types, that's not valid:
type Obj = dyn Trait<T = Clone>;
//~^ ERROR trait objects must include the `dyn` keyword
//~| HELP add `dyn` keyword before this trait

trait Trait { type T; }

fn main() {}
Loading

0 comments on commit 455e977

Please sign in to comment.