Skip to content

Commit

Permalink
Rollup merge of rust-lang#81176 - camsteffen:qpath-res, r=oli-obk
Browse files Browse the repository at this point in the history
Improve safety of `LateContext::qpath_res`

This is my first rustc code change, inspired by hacking on clippy!

The first change is to clear cached `TypeckResults` from `LateContext` when visiting a nested item. I took a hint from [here](https://github.com/rust-lang/rust/blob/5e91c4ecc09312d8b63d250a432b0f3ef83f1df7/compiler/rustc_privacy/src/lib.rs#L1300).

Clippy has a `qpath_res` util function to avoid a possible ICE in `LateContext::qpath_res`. But the docs of `LateContext::qpath_res` promise no ICE. So this updates the `LateContext` method to keep its promises, and removes the util function.

Related: rust-lang/rust-clippy#4545

CC `````@eddyb````` since you've done related work
CC `````@flip1995````` FYI
  • Loading branch information
Dylan-DPC authored Jan 27, 2021
2 parents 37c692f + eaba3da commit 3436ad0
Show file tree
Hide file tree
Showing 16 changed files with 57 additions and 58 deletions.
8 changes: 8 additions & 0 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -746,6 +746,14 @@ impl<'tcx> LateContext<'tcx> {
hir::QPath::Resolved(_, ref path) => path.res,
hir::QPath::TypeRelative(..) | hir::QPath::LangItem(..) => self
.maybe_typeck_results()
.filter(|typeck_results| typeck_results.hir_owner == id.owner)
.or_else(|| {
if self.tcx.has_typeck_results(id.owner.to_def_id()) {
Some(self.tcx.typeck(id.owner))
} else {
None
}
})
.and_then(|typeck_results| typeck_results.type_dependent_def(id))
.map_or(Res::Err, |(kind, def_id)| Res::Def(kind, def_id)),
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_lint/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,17 @@ impl<'tcx, T: LateLintPass<'tcx>> hir_visit::Visitor<'tcx> for LateContextAndPas
fn visit_item(&mut self, it: &'tcx hir::Item<'tcx>) {
let generics = self.context.generics.take();
self.context.generics = it.kind.generics();
let old_cached_typeck_results = self.context.cached_typeck_results.take();
let old_enclosing_body = self.context.enclosing_body.take();
self.with_lint_attrs(it.hir_id, &it.attrs, |cx| {
cx.with_param_env(it.hir_id, |cx| {
lint_callback!(cx, check_item, it);
hir_visit::walk_item(cx, it);
lint_callback!(cx, check_item_post, it);
});
});
self.context.enclosing_body = old_enclosing_body;
self.context.cached_typeck_results.set(old_cached_typeck_results);
self.context.generics = generics;
}

Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/default.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::utils::{
any_parent_is_automatically_derived, contains_name, match_def_path, paths, qpath_res, snippet_with_macro_callsite,
any_parent_is_automatically_derived, contains_name, match_def_path, paths, snippet_with_macro_callsite,
};
use crate::utils::{span_lint_and_note, span_lint_and_sugg};
use if_chain::if_chain;
Expand Down Expand Up @@ -231,7 +231,7 @@ fn is_expr_default<'tcx>(expr: &'tcx Expr<'tcx>, cx: &LateContext<'tcx>) -> bool
if_chain! {
if let ExprKind::Call(ref fn_expr, _) = &expr.kind;
if let ExprKind::Path(qpath) = &fn_expr.kind;
if let Res::Def(_, def_id) = qpath_res(cx, qpath, fn_expr.hir_id);
if let Res::Def(_, def_id) = cx.qpath_res(qpath, fn_expr.hir_id);
then {
// right hand side of assignment is `Default::default`
match_def_path(cx, def_id, &paths::DEFAULT_TRAIT_METHOD)
Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/drop_forget_ref.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::{is_copy, match_def_path, paths, qpath_res, span_lint_and_note};
use crate::utils::{is_copy, match_def_path, paths, span_lint_and_note};
use if_chain::if_chain;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
Expand Down Expand Up @@ -114,7 +114,7 @@ impl<'tcx> LateLintPass<'tcx> for DropForgetRef {
if let ExprKind::Call(ref path, ref args) = expr.kind;
if let ExprKind::Path(ref qpath) = path.kind;
if args.len() == 1;
if let Some(def_id) = qpath_res(cx, qpath, path.hir_id).opt_def_id();
if let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id();
then {
let lint;
let msg;
Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/exit.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::{is_entrypoint_fn, match_def_path, paths, qpath_res, span_lint};
use crate::utils::{is_entrypoint_fn, match_def_path, paths, span_lint};
use if_chain::if_chain;
use rustc_hir::{Expr, ExprKind, Item, ItemKind, Node};
use rustc_lint::{LateContext, LateLintPass};
Expand Down Expand Up @@ -29,7 +29,7 @@ impl<'tcx> LateLintPass<'tcx> for Exit {
if_chain! {
if let ExprKind::Call(ref path_expr, ref _args) = e.kind;
if let ExprKind::Path(ref path) = path_expr.kind;
if let Some(def_id) = qpath_res(cx, path, path_expr.hir_id).opt_def_id();
if let Some(def_id) = cx.qpath_res(path, path_expr.hir_id).opt_def_id();
if match_def_path(cx, def_id, &paths::EXIT);
then {
let parent = cx.tcx.hir().get_parent_item(e.hir_id);
Expand Down
8 changes: 4 additions & 4 deletions src/tools/clippy/clippy_lints/src/functions.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::utils::{
attr_by_name, attrs::is_proc_macro, is_must_use_ty, is_trait_impl_item, is_type_diagnostic_item, iter_input_pats,
last_path_segment, match_def_path, must_use_attr, qpath_res, return_ty, snippet, snippet_opt, span_lint,
span_lint_and_help, span_lint_and_then, trait_ref_of_method, type_is_unsafe_function,
last_path_segment, match_def_path, must_use_attr, return_ty, snippet, snippet_opt, span_lint, span_lint_and_help,
span_lint_and_then, trait_ref_of_method, type_is_unsafe_function,
};
use if_chain::if_chain;
use rustc_ast::ast::Attribute;
Expand Down Expand Up @@ -659,7 +659,7 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for DerefVisitor<'a, 'tcx> {
impl<'a, 'tcx> DerefVisitor<'a, 'tcx> {
fn check_arg(&self, ptr: &hir::Expr<'_>) {
if let hir::ExprKind::Path(ref qpath) = ptr.kind {
if let Res::Local(id) = qpath_res(self.cx, qpath, ptr.hir_id) {
if let Res::Local(id) = self.cx.qpath_res(qpath, ptr.hir_id) {
if self.ptrs.contains(&id) {
span_lint(
self.cx,
Expand Down Expand Up @@ -722,7 +722,7 @@ fn is_mutated_static(cx: &LateContext<'_>, e: &hir::Expr<'_>) -> bool {
use hir::ExprKind::{Field, Index, Path};

match e.kind {
Path(ref qpath) => !matches!(qpath_res(cx, qpath, e.hir_id), Res::Local(_)),
Path(ref qpath) => !matches!(cx.qpath_res(qpath, e.hir_id), Res::Local(_)),
Field(ref inner, _) | Index(ref inner, _) => is_mutated_static(cx, inner),
_ => false,
}
Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/let_if_seq.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::{qpath_res, snippet, span_lint_and_then, visitors::LocalUsedVisitor};
use crate::utils::{snippet, span_lint_and_then, visitors::LocalUsedVisitor};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
Expand Down Expand Up @@ -145,7 +145,7 @@ fn check_assign<'tcx>(
if let hir::StmtKind::Semi(ref expr) = expr.kind;
if let hir::ExprKind::Assign(ref var, ref value, _) = expr.kind;
if let hir::ExprKind::Path(ref qpath) = var.kind;
if let Res::Local(local_id) = qpath_res(cx, qpath, var.hir_id);
if let Res::Local(local_id) = cx.qpath_res(qpath, var.hir_id);
if decl == local_id;
then {
let mut v = LocalUsedVisitor::new(decl);
Expand Down
22 changes: 11 additions & 11 deletions src/tools/clippy/clippy_lints/src/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ use crate::utils::visitors::LocalUsedVisitor;
use crate::utils::{
contains_name, get_enclosing_block, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait,
indent_of, is_in_panic_handler, is_integer_const, is_no_std_crate, is_refutable, is_type_diagnostic_item,
last_path_segment, match_trait_method, match_type, match_var, multispan_sugg, qpath_res, single_segment_path,
snippet, snippet_with_applicability, snippet_with_macro_callsite, span_lint, span_lint_and_help,
span_lint_and_sugg, span_lint_and_then, sugg, SpanlessEq,
last_path_segment, match_trait_method, match_type, match_var, multispan_sugg, single_segment_path, snippet,
snippet_with_applicability, snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg,
span_lint_and_then, sugg, SpanlessEq,
};
use if_chain::if_chain;
use rustc_ast::ast;
Expand Down Expand Up @@ -848,7 +848,7 @@ fn same_var<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, var: HirId) -> bool {
if let ExprKind::Path(qpath) = &expr.kind;
if let QPath::Resolved(None, path) = qpath;
if path.segments.len() == 1;
if let Res::Local(local_id) = qpath_res(cx, qpath, expr.hir_id);
if let Res::Local(local_id) = cx.qpath_res(qpath, expr.hir_id);
then {
// our variable!
local_id == var
Expand Down Expand Up @@ -1420,7 +1420,7 @@ fn detect_same_item_push<'tcx>(
// Make sure that the push does not involve possibly mutating values
match pushed_item.kind {
ExprKind::Path(ref qpath) => {
match qpath_res(cx, qpath, pushed_item.hir_id) {
match cx.qpath_res(qpath, pushed_item.hir_id) {
// immutable bindings that are initialized with literal or constant
Res::Local(hir_id) => {
if_chain! {
Expand All @@ -1437,7 +1437,7 @@ fn detect_same_item_push<'tcx>(
ExprKind::Lit(..) => emit_lint(cx, vec, pushed_item),
// immutable bindings that are initialized with constant
ExprKind::Path(ref path) => {
if let Res::Def(DefKind::Const, ..) = qpath_res(cx, path, init.hir_id) {
if let Res::Def(DefKind::Const, ..) = cx.qpath_res(path, init.hir_id) {
emit_lint(cx, vec, pushed_item);
}
}
Expand Down Expand Up @@ -2028,7 +2028,7 @@ fn check_for_mutability(cx: &LateContext<'_>, bound: &Expr<'_>) -> Option<HirId>
if let ExprKind::Path(ref qpath) = bound.kind;
if let QPath::Resolved(None, _) = *qpath;
then {
let res = qpath_res(cx, qpath, bound.hir_id);
let res = cx.qpath_res(qpath, bound.hir_id);
if let Res::Local(hir_id) = res {
let node_str = cx.tcx.hir().get(hir_id);
if_chain! {
Expand Down Expand Up @@ -2120,7 +2120,7 @@ impl<'a, 'tcx> VarVisitor<'a, 'tcx> {
if self.prefer_mutable {
self.indexed_mut.insert(seqvar.segments[0].ident.name);
}
let res = qpath_res(self.cx, seqpath, seqexpr.hir_id);
let res = self.cx.qpath_res(seqpath, seqexpr.hir_id);
match res {
Res::Local(hir_id) => {
let parent_id = self.cx.tcx.hir().get_parent_item(expr.hir_id);
Expand Down Expand Up @@ -2184,7 +2184,7 @@ impl<'a, 'tcx> Visitor<'tcx> for VarVisitor<'a, 'tcx> {
if let QPath::Resolved(None, ref path) = *qpath;
if path.segments.len() == 1;
then {
if let Res::Local(local_id) = qpath_res(self.cx, qpath, expr.hir_id) {
if let Res::Local(local_id) = self.cx.qpath_res(qpath, expr.hir_id) {
if local_id == self.var {
self.nonindex = true;
} else {
Expand Down Expand Up @@ -2589,7 +2589,7 @@ impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> {

fn var_def_id(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<HirId> {
if let ExprKind::Path(ref qpath) = expr.kind {
let path_res = qpath_res(cx, qpath, expr.hir_id);
let path_res = cx.qpath_res(qpath, expr.hir_id);
if let Res::Local(hir_id) = path_res {
return Some(hir_id);
}
Expand Down Expand Up @@ -2819,7 +2819,7 @@ impl<'a, 'tcx> VarCollectorVisitor<'a, 'tcx> {
if_chain! {
if let ExprKind::Path(ref qpath) = ex.kind;
if let QPath::Resolved(None, _) = *qpath;
let res = qpath_res(self.cx, qpath, ex.hir_id);
let res = self.cx.qpath_res(qpath, ex.hir_id);
then {
match res {
Res::Local(hir_id) => {
Expand Down
8 changes: 4 additions & 4 deletions src/tools/clippy/clippy_lints/src/manual_strip.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::consts::{constant, Constant};
use crate::utils::usage::mutated_variables;
use crate::utils::{
eq_expr_value, higher, match_def_path, meets_msrv, multispan_sugg, paths, qpath_res, snippet, span_lint_and_then,
eq_expr_value, higher, match_def_path, meets_msrv, multispan_sugg, paths, snippet, span_lint_and_then,
};

use if_chain::if_chain;
Expand Down Expand Up @@ -92,7 +92,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualStrip {
} else {
return;
};
let target_res = qpath_res(cx, &target_path, target_arg.hir_id);
let target_res = cx.qpath_res(&target_path, target_arg.hir_id);
if target_res == Res::Err {
return;
};
Expand Down Expand Up @@ -221,7 +221,7 @@ fn find_stripping<'tcx>(
if let ExprKind::Index(indexed, index) = &unref.kind;
if let Some(higher::Range { start, end, .. }) = higher::range(index);
if let ExprKind::Path(path) = &indexed.kind;
if qpath_res(self.cx, path, ex.hir_id) == self.target;
if self.cx.qpath_res(path, ex.hir_id) == self.target;
then {
match (self.strip_kind, start, end) {
(StripKind::Prefix, Some(start), None) => {
Expand All @@ -235,7 +235,7 @@ fn find_stripping<'tcx>(
if let ExprKind::Binary(Spanned { node: BinOpKind::Sub, .. }, left, right) = end.kind;
if let Some(left_arg) = len_arg(self.cx, left);
if let ExprKind::Path(left_path) = &left_arg.kind;
if qpath_res(self.cx, left_path, left_arg.hir_id) == self.target;
if self.cx.qpath_res(left_path, left_arg.hir_id) == self.target;
if eq_pattern_length(self.cx, self.pattern, right);
then {
self.results.push(ex.span);
Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/mem_forget.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::{match_def_path, paths, qpath_res, span_lint};
use crate::utils::{match_def_path, paths, span_lint};
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
Expand Down Expand Up @@ -29,7 +29,7 @@ impl<'tcx> LateLintPass<'tcx> for MemForget {
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
if let ExprKind::Call(ref path_expr, ref args) = e.kind {
if let ExprKind::Path(ref qpath) = path_expr.kind {
if let Some(def_id) = qpath_res(cx, qpath, path_expr.hir_id).opt_def_id() {
if let Some(def_id) = cx.qpath_res(qpath, path_expr.hir_id).opt_def_id() {
if match_def_path(cx, def_id, &paths::MEM_FORGET) {
let forgot_ty = cx.typeck_results().expr_ty(&args[0]);

Expand Down
6 changes: 3 additions & 3 deletions src/tools/clippy/clippy_lints/src/no_effect.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::{has_drop, qpath_res, snippet_opt, span_lint, span_lint_and_sugg};
use crate::utils::{has_drop, snippet_opt, span_lint, span_lint_and_sugg};
use rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::{BinOpKind, BlockCheckMode, Expr, ExprKind, Stmt, StmtKind, UnsafeSource};
Expand Down Expand Up @@ -67,7 +67,7 @@ fn has_no_effect(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
},
ExprKind::Call(ref callee, ref args) => {
if let ExprKind::Path(ref qpath) = callee.kind {
let res = qpath_res(cx, qpath, callee.hir_id);
let res = cx.qpath_res(qpath, callee.hir_id);
match res {
Res::Def(DefKind::Struct | DefKind::Variant | DefKind::Ctor(..), ..) => {
!has_drop(cx, cx.typeck_results().expr_ty(expr))
Expand Down Expand Up @@ -146,7 +146,7 @@ fn reduce_expression<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<Vec
},
ExprKind::Call(ref callee, ref args) => {
if let ExprKind::Path(ref qpath) = callee.kind {
let res = qpath_res(cx, qpath, callee.hir_id);
let res = cx.qpath_res(qpath, callee.hir_id);
match res {
Res::Def(DefKind::Struct | DefKind::Variant | DefKind::Ctor(..), ..)
if !has_drop(cx, cx.typeck_results().expr_ty(expr)) =>
Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/non_copy_const.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{InnerSpan, Span, DUMMY_SP};
use rustc_typeck::hir_ty_to_ty;

use crate::utils::{in_constant, qpath_res, span_lint_and_then};
use crate::utils::{in_constant, span_lint_and_then};
use if_chain::if_chain;

// FIXME: this is a correctness problem but there's no suitable
Expand Down Expand Up @@ -339,7 +339,7 @@ impl<'tcx> LateLintPass<'tcx> for NonCopyConst {
}

// Make sure it is a const item.
let item_def_id = match qpath_res(cx, qpath, expr.hir_id) {
let item_def_id = match cx.qpath_res(qpath, expr.hir_id) {
Res::Def(DefKind::Const | DefKind::AssocConst, did) => did,
_ => return,
};
Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/to_string_in_display.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::{match_def_path, match_trait_method, paths, qpath_res, span_lint};
use crate::utils::{match_def_path, match_trait_method, paths, span_lint};
use if_chain::if_chain;
use rustc_hir::def::Res;
use rustc_hir::{Expr, ExprKind, HirId, Impl, ImplItem, ImplItemKind, Item, ItemKind};
Expand Down Expand Up @@ -94,7 +94,7 @@ impl LateLintPass<'_> for ToStringInDisplay {
if match_trait_method(cx, expr, &paths::TO_STRING);
if self.in_display_impl;
if let ExprKind::Path(ref qpath) = args[0].kind;
if let Res::Local(hir_id) = qpath_res(cx, qpath, args[0].hir_id);
if let Res::Local(hir_id) = cx.qpath_res(qpath, args[0].hir_id);
if let Some(self_hir_id) = self.self_hir_id;
if hir_id == self_hir_id;
then {
Expand Down
Loading

0 comments on commit 3436ad0

Please sign in to comment.