Skip to content
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

Use get_diagnostic_name more #90985

Merged
merged 1 commit into from
Dec 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 15 additions & 20 deletions compiler/rustc_borrowck/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -742,15 +742,13 @@ impl BorrowedContentSource<'tcx> {
BorrowedContentSource::DerefRawPointer => "a raw pointer".to_string(),
BorrowedContentSource::DerefSharedRef => "a shared reference".to_string(),
BorrowedContentSource::DerefMutableRef => "a mutable reference".to_string(),
BorrowedContentSource::OverloadedDeref(ty) => match ty.kind() {
ty::Adt(def, _) if tcx.is_diagnostic_item(sym::Rc, def.did) => {
"an `Rc`".to_string()
}
ty::Adt(def, _) if tcx.is_diagnostic_item(sym::Arc, def.did) => {
"an `Arc`".to_string()
}
_ => format!("dereference of `{}`", ty),
},
BorrowedContentSource::OverloadedDeref(ty) => ty
.ty_adt_def()
.and_then(|adt| match tcx.get_diagnostic_name(adt.did)? {
Copy link
Member

Choose a reason for hiding this comment

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

Kind of a nit, but I think I prefer having a separate and_then for get_diagnostic_name, rather than using the question mark.

name @ (sym::Rc | sym::Arc) => Some(format!("an `{}`", name)),
_ => None,
})
.unwrap_or_else(|| format!("dereference of `{}`", ty)),
BorrowedContentSource::OverloadedIndex(ty) => format!("index of `{}`", ty),
}
}
Expand All @@ -774,15 +772,13 @@ impl BorrowedContentSource<'tcx> {
BorrowedContentSource::DerefMutableRef => {
bug!("describe_for_immutable_place: DerefMutableRef isn't immutable")
}
BorrowedContentSource::OverloadedDeref(ty) => match ty.kind() {
ty::Adt(def, _) if tcx.is_diagnostic_item(sym::Rc, def.did) => {
"an `Rc`".to_string()
}
ty::Adt(def, _) if tcx.is_diagnostic_item(sym::Arc, def.did) => {
"an `Arc`".to_string()
}
_ => format!("a dereference of `{}`", ty),
},
BorrowedContentSource::OverloadedDeref(ty) => ty
.ty_adt_def()
.and_then(|adt| match tcx.get_diagnostic_name(adt.did)? {
Copy link
Member

Choose a reason for hiding this comment

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

Same here.

name @ (sym::Rc | sym::Arc) => Some(format!("an `{}`", name)),
_ => None,
})
.unwrap_or_else(|| format!("dereference of `{}`", ty)),
BorrowedContentSource::OverloadedIndex(ty) => format!("an index of `{}`", ty),
}
}
Expand Down Expand Up @@ -966,8 +962,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
_ => None,
});
let is_option_or_result = parent_self_ty.map_or(false, |def_id| {
tcx.is_diagnostic_item(sym::Option, def_id)
|| tcx.is_diagnostic_item(sym::Result, def_id)
matches!(tcx.get_diagnostic_name(def_id), Some(sym::Option | sym::Result))
});
FnSelfUseKind::Normal { self_arg, implicit_into_iter, is_option_or_result }
});
Expand Down
18 changes: 8 additions & 10 deletions compiler/rustc_lint/src/enum_intrinsics_non_enums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,16 +91,14 @@ fn enforce_mem_variant_count(cx: &LateContext<'_>, func_expr: &hir::Expr<'_>, sp

impl<'tcx> LateLintPass<'tcx> for EnumIntrinsicsNonEnums {
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) {
if let hir::ExprKind::Call(ref func, ref args) = expr.kind {
if let hir::ExprKind::Path(ref qpath) = func.kind {
if let Some(def_id) = cx.qpath_res(qpath, func.hir_id).opt_def_id() {
if cx.tcx.is_diagnostic_item(sym::mem_discriminant, def_id) {
enforce_mem_discriminant(cx, func, expr.span, args[0].span);
} else if cx.tcx.is_diagnostic_item(sym::mem_variant_count, def_id) {
enforce_mem_variant_count(cx, func, expr.span);
}
}
}
let hir::ExprKind::Call(func, args) = &expr.kind else { return };
let hir::ExprKind::Path(qpath) = &func.kind else { return };
let Some(def_id) = cx.qpath_res(qpath, func.hir_id).opt_def_id() else { return };
let Some(name) = cx.tcx.get_diagnostic_name(def_id) else { return };
match name {
sym::mem_discriminant => enforce_mem_discriminant(cx, func, expr.span, args[0].span),
sym::mem_variant_count => enforce_mem_variant_count(cx, func, expr.span),
_ => {}
}
}
}
1 change: 1 addition & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#![cfg_attr(bootstrap, feature(format_args_capture))]
#![feature(iter_order_by)]
#![feature(iter_zip)]
#![feature(let_else)]
#![feature(never_type)]
#![feature(nll)]
#![feature(control_flow_enum)]
Expand Down
19 changes: 13 additions & 6 deletions compiler/rustc_lint/src/non_fmt_panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,14 +309,21 @@ fn panic_call<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>) -> (Span,
// Unwrap more levels of macro expansion, as panic_2015!()
// was likely expanded from panic!() and possibly from
// [debug_]assert!().
for &i in
&[sym::std_panic_macro, sym::core_panic_macro, sym::assert_macro, sym::debug_assert_macro]
{
loop {
Copy link
Member

Choose a reason for hiding this comment

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

This...isn't exactly the same, right?

Previously, we can only match each diagnostic item once? But now we can match the same on continously?

Also, now we can ascend many times.

I'm not sure there are actually any semantic changes, just worth mentioning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the current implementation, I think the order of the list is significant because assert contains a call to panic, and we want to climb to the outermost macro call. This implementation works differently but should have the same result AFAICT.

let parent = expn.call_site.ctxt().outer_expn_data();
if parent.macro_def_id.map_or(false, |id| cx.tcx.is_diagnostic_item(i, id)) {
expn = parent;
panic_macro = i;
let Some(id) = parent.macro_def_id else { break };
let Some(name) = cx.tcx.get_diagnostic_name(id) else { break };
if !matches!(
name,
sym::core_panic_macro
| sym::std_panic_macro
| sym::assert_macro
| sym::debug_assert_macro
) {
break;
}
expn = parent;
panic_macro = name;
}

let macro_symbol =
Expand Down
62 changes: 30 additions & 32 deletions compiler/rustc_lint/src/noop_method_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,38 +75,36 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
_ => return,
};
// (Re)check that it implements the noop diagnostic.
for s in [sym::noop_method_clone, sym::noop_method_deref, sym::noop_method_borrow].iter() {
if cx.tcx.is_diagnostic_item(*s, i.def_id()) {
let method = &call.ident.name;
let receiver = &elements[0];
let receiver_ty = cx.typeck_results().expr_ty(receiver);
let expr_ty = cx.typeck_results().expr_ty_adjusted(expr);
if receiver_ty != expr_ty {
// This lint will only trigger if the receiver type and resulting expression \
// type are the same, implying that the method call is unnecessary.
return;
}
let expr_span = expr.span;
let note = format!(
"the type `{:?}` which `{}` is being called on is the same as \
the type returned from `{}`, so the method call does not do \
anything and can be removed",
receiver_ty, method, method,
);

let span = expr_span.with_lo(receiver.span.hi());
cx.struct_span_lint(NOOP_METHOD_CALL, span, |lint| {
let method = &call.ident.name;
let message = format!(
"call to `.{}()` on a reference in this situation does nothing",
&method,
);
lint.build(&message)
.span_label(span, "unnecessary method call")
.note(&note)
.emit()
});
}
let Some(name) = cx.tcx.get_diagnostic_name(i.def_id()) else { return };
if !matches!(
name,
sym::noop_method_borrow | sym::noop_method_clone | sym::noop_method_deref
) {
return;
}
let method = &call.ident.name;
let receiver = &elements[0];
let receiver_ty = cx.typeck_results().expr_ty(receiver);
let expr_ty = cx.typeck_results().expr_ty_adjusted(expr);
if receiver_ty != expr_ty {
// This lint will only trigger if the receiver type and resulting expression \
// type are the same, implying that the method call is unnecessary.
return;
}
let expr_span = expr.span;
let note = format!(
"the type `{:?}` which `{}` is being called on is the same as \
the type returned from `{}`, so the method call does not do \
anything and can be removed",
receiver_ty, method, method,
);

let span = expr_span.with_lo(receiver.span.hi());
cx.struct_span_lint(NOOP_METHOD_CALL, span, |lint| {
let method = &call.ident.name;
let message =
format!("call to `.{}()` on a reference in this situation does nothing", &method,);
lint.build(&message).span_label(span, "unnecessary method call").note(&note).emit()
});
}
}
12 changes: 6 additions & 6 deletions compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -539,11 +539,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
// is otherwise overwhelming and unhelpful (see #85844 for an
// example).

let trait_is_debug =
self.tcx.is_diagnostic_item(sym::Debug, trait_ref.def_id());
let trait_is_display =
self.tcx.is_diagnostic_item(sym::Display, trait_ref.def_id());

let in_std_macro =
match obligation.cause.span.ctxt().outer_expn_data().macro_def_id {
Some(macro_def_id) => {
Expand All @@ -553,7 +548,12 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
None => false,
};

if in_std_macro && (trait_is_debug || trait_is_display) {
if in_std_macro
&& matches!(
self.tcx.get_diagnostic_name(trait_ref.def_id()),
Some(sym::Debug | sym::Display)
)
{
err.emit();
return;
}
Expand Down