Skip to content

Silence errors in expressions caused by bare traits in paths in 2021 edition #125784

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
26cfeb1
Silence errors in experssions caused by bare traits in paths
estebank May 30, 2024
936b692
Silence unsized types errors in some expressions
estebank May 31, 2024
5028c30
Avoid potential for ICE by not stashing delayed as bug error and sile…
estebank May 31, 2024
f1f9842
Do not silence some sized errors as they would cause ICEs
estebank Jun 3, 2024
5441f71
Add more tracking for the HIR source of well-formedness requirement
estebank Jun 3, 2024
9a8f343
Add tests for various `dyn Trait` in path cases
estebank Jun 3, 2024
9ada8bc
Add more HIR tracking information to `ObligationCauseCode`
estebank Jun 3, 2024
5d85ffa
On object safety errors, point at specific object unsafe trait in path
estebank Jun 3, 2024
e2f2151
Move `FnCtxt::get_fn_decl` to `TyCtxt`
estebank Jun 3, 2024
230a787
Deduplicate more E0038 object safety errors by pointing at type more …
estebank Jun 3, 2024
e55ab93
Move suggestions out of main error logic
estebank Jun 8, 2024
c9e8029
Tweak wording and only suggest `dyn Trait` if `Trait` is object safe
estebank Jun 8, 2024
adc750f
Suggest `<_ as Default>::default()` when given `<Default>::default()`
estebank Jun 8, 2024
dd7b36f
review comment: `guard` -> `guar`
estebank Jun 8, 2024
e6d9050
review comment: unnecessary check for object safety on sized trait
estebank Jun 8, 2024
5f22c02
Add explanatory comment
estebank Jun 8, 2024
84be44b
Fix tidy
estebank Jun 10, 2024
d8f5b14
Remove conditional delay_as_bug from "missing `dyn` error"
estebank Jun 10, 2024
8ee6812
Remove `delay_as_bug` for fn call with unsized `Self` that return `Se…
estebank Jun 10, 2024
8140dcd
Emit method not found error for object safe traits without `dyn`
estebank Jun 10, 2024
f116d4d
Move tests that no longer ICE
estebank Jun 10, 2024
aec9eeb
Do not produce incorrect `dyn Trait` suggestion in `fn`
estebank Jun 10, 2024
05bffa0
Mention associated type with same name as trait in `E0782`
estebank Jun 11, 2024
88ccfa4
Account for `static` and `const` in suggestion
estebank Jun 11, 2024
a62bfcd
Use verbose format for fully-qualified path suggestion
estebank Jun 11, 2024
7f923f3
Fix tidy
estebank Jun 11, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4200,6 +4200,7 @@ dependencies = [
name = "rustc_infer"
version = "0.0.0"
dependencies = [
"rustc_ast",
"rustc_ast_ir",
"rustc_data_structures",
"rustc_errors",
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/wfcheck.rs
Copy link
Member

@fmease fmease Jun 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(note to self): I haven't properly reviewed commit "Add more tracking for HIR source well-formedness requirement" yet.

Original file line number Diff line number Diff line change
Expand Up @@ -1270,7 +1270,7 @@ fn check_item_type(
traits::ObligationCause::new(
ty_span,
wfcx.body_def_id,
ObligationCauseCode::WellFormed(None),
ObligationCauseCode::WellFormed(Some(WellFormedLoc::Ty(item_id))),
),
wfcx.param_env,
item_ty,
Expand Down
104 changes: 80 additions & 24 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use rustc_ast::TraitObjectSyntax;
use rustc_errors::{codes::*, Diag, EmissionGuarantee, StashKey};
use rustc_errors::{codes::*, Diag, EmissionGuarantee, ErrorGuaranteed, StashKey};
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_infer::traits::error_reporting::suggest_path_on_bare_trait;
use rustc_lint_defs::{builtin::BARE_TRAIT_OBJECTS, Applicability};
use rustc_span::Span;
use rustc_trait_selection::traits::error_reporting::suggestions::NextTypeParamName;
Expand All @@ -17,13 +18,14 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
&self,
self_ty: &hir::Ty<'_>,
in_path: bool,
) {
) -> Option<ErrorGuaranteed> {
let tcx = self.tcx();

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

let needs_bracket = in_path
Expand All @@ -36,6 +38,11 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {

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

let object_safe = traits.iter().all(|ptr| match ptr.trait_ref.path.res {
Res::Def(DefKind::Trait, def_id) => tcx.object_safety_violations(def_id).is_empty(),
_ => false,
});

let mut sugg = vec![(
self_ty.span.shrink_to_lo(),
format!(
Expand All @@ -61,28 +68,74 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
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);
if self_ty.span.can_be_used_for_suggestions() {
self.maybe_suggest_impl_trait(self_ty, &mut diag);
if object_safe {
// Only emit this suggestion if the trait is object safe.
diag.multipart_suggestion_verbose(
label,
sugg,
Applicability::MachineApplicable,
);
}
}

if !object_safe && self_ty.span.can_be_used_for_suggestions() {
let parent = tcx.parent_hir_node(self_ty.hir_id);
suggest_path_on_bare_trait(tcx, &mut diag, parent);
}

// 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);

if object_safe {
let parents = self.tcx().hir().parent_iter(self_ty.hir_id);
for (_, parent) in parents {
let hir::Node::Expr(expr) = parent else {
break;
};
if let hir::ExprKind::Path(hir::QPath::TypeRelative(_, segment)) = expr.kind
&& let Res::Err = segment.res
{
// If the trait is object safe *and* there's a path segment that couldn't be
// resolved, we know that we will have a resolve error later. If there's an
// unresolved segment *and* the trait is not object safe, then no other
// error would have been emitted, so we always emit an error in that case.
diag.emit();
return None;
}
}
}
diag.stash(self_ty.span, StashKey::TraitMissingMethod)
} else {
tcx.node_span_lint(BARE_TRAIT_OBJECTS, self_ty.hir_id, self_ty.span, |lint| {
lint.primary_message("trait objects without an explicit `dyn` are deprecated");
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,
);
match (object_safe, self_ty.span.can_be_used_for_suggestions()) {
(true, true) => {
lint.multipart_suggestion_verbose(
"as this is an \"object safe\" trait, write `dyn` in front of the \
trait",
sugg,
Applicability::MachineApplicable,
);
}
(true, false) => {
lint.note(
"as this is an \"object safe\" trait, you can write `dyn` in front of \
the trait",
);
}
(false, _) => {
lint.note(
"you can't use write a trait object here because the trait isn't \
\"object safe\"",
);
}
}
self.maybe_suggest_blanket_trait_impl(self_ty, lint);
});
None
}
}

Expand Down Expand Up @@ -152,6 +205,8 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
}

/// Make sure that we are in the condition to suggest `impl Trait`.
///
/// Returns whether a suggestion was provided and whether the error itself should not be emitted
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;
Expand All @@ -174,6 +229,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
};
let impl_sugg = vec![(self_ty.span.shrink_to_lo(), "impl ".to_string())];
let mut is_downgradable = true;
let mut downgrade = false;
let is_object_safe = match self_ty.kind {
hir::TyKind::TraitObject(objects, ..) => {
objects.iter().all(|o| match o.trait_ref.path.res {
Expand All @@ -182,7 +238,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
// For recursive traits, don't downgrade the error. (#119652)
is_downgradable = false;
}
tcx.is_object_safe(id)
tcx.object_safety_violations(id).is_empty()
}
_ => false,
})
Expand Down Expand Up @@ -213,9 +269,9 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
);
} else if is_downgradable {
// We'll emit the object safety error already, with a structured suggestion.
diag.downgrade_to_delayed_bug();
downgrade = true;
}
return true;
return downgrade;
}
for ty in sig.decl.inputs {
if ty.hir_id != self_ty.hir_id {
Expand All @@ -239,7 +295,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
diag.note(format!("`{trait_name}` it is not object safe, so it can't be `dyn`"));
if is_downgradable {
// We'll emit the object safety error already, with a structured suggestion.
diag.downgrade_to_delayed_bug();
downgrade = true;
}
} else {
let sugg = if let hir::TyKind::TraitObject([_, _, ..], _, _) = self_ty.kind {
Expand All @@ -260,9 +316,9 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
Applicability::MachineApplicable,
);
}
return true;
return downgrade;
}
false
downgrade
}

fn maybe_suggest_assoc_ty_bound(&self, self_ty: &hir::Ty<'_>, diag: &mut Diag<'_>) {
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2069,7 +2069,9 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
)
}
hir::TyKind::TraitObject(bounds, lifetime, repr) => {
self.prohibit_or_lint_bare_trait_object_ty(hir_ty, in_path);
if let Some(guar) = self.prohibit_or_lint_bare_trait_object_ty(hir_ty, in_path) {
return Ty::new_error(tcx, guar);
}

let repr = match repr {
TraitObjectSyntax::Dyn | TraitObjectSyntax::None => ty::Dyn,
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_hir_analysis/src/hir_wf_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ fn diagnostic_hir_wf_check<'tcx>(
let def_id = match loc {
WellFormedLoc::Ty(def_id) => def_id,
WellFormedLoc::Param { function, param_idx: _ } => function,
WellFormedLoc::Expr(_) => return None,
};
let hir_id = tcx.local_def_id_to_hir_id(def_id);

Expand Down Expand Up @@ -79,7 +80,9 @@ fn diagnostic_hir_wf_check<'tcx>(
let cause = traits::ObligationCause::new(
ty.span,
self.def_id,
traits::ObligationCauseCode::WellFormed(None),
traits::ObligationCauseCode::WellFormed(Some(traits::WellFormedLoc::Expr(
ty.hir_id,
))),
);

ocx.register_obligation(traits::Obligation::new(
Expand Down Expand Up @@ -191,6 +194,7 @@ fn diagnostic_hir_wf_check<'tcx>(
vec![&fn_decl.inputs[param_idx as usize]]
}
}
WellFormedLoc::Expr(_) => return None,
};
for ty in tys {
visitor.visit_ty(ty);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
{
// check that the `if` expr without `else` is the fn body's expr
if expr.span == sp {
return self.get_fn_decl(hir_id).map(|(_, fn_decl, _)| {
return self.tcx.get_fn_decl(hir_id).map(|(_, fn_decl, _)| {
let (ty, span) = match fn_decl.output {
hir::FnRetTy::DefaultReturn(span) => ("()".to_string(), span),
hir::FnRetTy::Return(ty) => (ty_to_string(&self.tcx, ty), ty.span),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.register_wf_obligation(
output.into(),
call_expr.span,
ObligationCauseCode::WellFormed(None),
ObligationCauseCode::WellFormed(Some(traits::WellFormedLoc::Expr(call_expr.hir_id))),
);

output
Expand Down
21 changes: 10 additions & 11 deletions compiler/rustc_hir_typeck/src/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
fn coerce_unsized(&self, mut source: Ty<'tcx>, mut target: Ty<'tcx>) -> CoerceResult<'tcx> {
source = self.shallow_resolve(source);
target = self.shallow_resolve(target);
debug!(?source, ?target);
debug!(?source, ?target, ?self.cause);

// We don't apply any coercions incase either the source or target
// aren't sufficiently well known but tend to instead just equate
Expand Down Expand Up @@ -565,11 +565,9 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
let mut selcx = traits::SelectionContext::new(self);

// Create an obligation for `Source: CoerceUnsized<Target>`.
let cause = ObligationCause::new(
self.cause.span,
self.body_id,
ObligationCauseCode::Coercion { source, target },
);
let mut cause =
ObligationCause::new(self.cause.span, self.body_id, self.cause.code().clone());
cause.map_code(|parent_code| ObligationCauseCode::Coercion { source, target, parent_code });

// Use a FIFO queue for this custom fulfillment procedure.
//
Expand Down Expand Up @@ -993,8 +991,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
debug!("coercion::try({:?}: {:?} -> {:?})", expr, source, target);

let cause =
cause.unwrap_or_else(|| self.cause(expr.span, ObligationCauseCode::ExprAssignable));
let cause = cause.unwrap_or_else(|| {
self.cause(expr.span, ObligationCauseCode::ExprAssignable(Some(expr.hir_id)))
});
let coerce = Coerce::new(self, cause, allow_two_phase);
let ok = self.commit_if_ok(|_| coerce.coerce(source, target))?;

Expand All @@ -1016,7 +1015,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let source = self.resolve_vars_with_obligations(expr_ty);
debug!("coercion::can_with_predicates({:?} -> {:?})", source, target);

let cause = self.cause(DUMMY_SP, ObligationCauseCode::ExprAssignable);
let cause = self.cause(DUMMY_SP, ObligationCauseCode::ExprAssignable(None));
// We don't ever need two-phase here since we throw out the result of the coercion
let coerce = Coerce::new(self, cause, AllowTwoPhase::No);
self.probe(|_| {
Expand All @@ -1033,7 +1032,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// how many dereference steps needed to achieve `expr_ty <: target`. If
/// it's not possible, return `None`.
pub fn deref_steps(&self, expr_ty: Ty<'tcx>, target: Ty<'tcx>) -> Option<usize> {
let cause = self.cause(DUMMY_SP, ObligationCauseCode::ExprAssignable);
let cause = self.cause(DUMMY_SP, ObligationCauseCode::ExprAssignable(None));
// We don't ever need two-phase here since we throw out the result of the coercion
let coerce = Coerce::new(self, cause, AllowTwoPhase::No);
coerce
Expand Down Expand Up @@ -1861,7 +1860,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
};

// If this is due to an explicit `return`, suggest adding a return type.
if let Some((fn_id, fn_decl, can_suggest)) = fcx.get_fn_decl(parent_id)
if let Some((fn_id, fn_decl, can_suggest)) = fcx.tcx.get_fn_decl(parent_id)
&& !due_to_block
{
fcx.suggest_missing_return_type(&mut err, fn_decl, expected, found, can_suggest, fn_id);
Expand Down
10 changes: 7 additions & 3 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.require_type_is_sized_deferred(
output,
call.map_or(expr.span, |e| e.span),
ObligationCauseCode::SizedCallReturnType,
ObligationCauseCode::SizedCallReturnType(did),
);
}

Expand Down Expand Up @@ -774,7 +774,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.ret_coercion_span.set(Some(expr.span));
}
let cause = self.cause(expr.span, ObligationCauseCode::ReturnNoExpression);
if let Some((_, fn_decl, _)) = self.get_fn_decl(expr.hir_id) {
if let Some((_, fn_decl, _)) = self.tcx.get_fn_decl(expr.hir_id) {
coercion.coerce_forced_unit(
self,
&cause,
Expand Down Expand Up @@ -1517,7 +1517,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

let ty = Ty::new_array_with_const_len(tcx, t, count);

self.register_wf_obligation(ty.into(), expr.span, ObligationCauseCode::WellFormed(None));
self.register_wf_obligation(
ty.into(),
expr.span,
ObligationCauseCode::WellFormed(Some(traits::WellFormedLoc::Expr(expr.hir_id))),
);

ty
}
Expand Down
Loading
Loading