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

Fix Clippy handling of ExpnKind::Desugaring #73468

Closed
Closed
Show file tree
Hide file tree
Changes from 3 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
5 changes: 1 addition & 4 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,10 +554,7 @@ impl Step for Clippy {

builder.add_rustc_lib_path(compiler, &mut cargo);

// FIXME: Disable clippy tests for now, they're failing on master
// (generally this would mean a toolstate failure but we don't have
// toolstate for clippy anymore).
// builder.run(&mut cargo.into());
builder.run(&mut cargo.into());
}
}

Expand Down
27 changes: 15 additions & 12 deletions src/librustc_middle/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use rustc_errors::{DiagnosticBuilder, DiagnosticId};
use rustc_hir::HirId;
use rustc_session::lint::{builtin, Level, Lint, LintId};
use rustc_session::{DiagnosticMessageId, Session};
use rustc_span::hygiene::MacroKind;
use rustc_span::source_map::{DesugaringKind, ExpnKind, MultiSpan};
use rustc_span::hygiene::{ClosestAstOrMacro, MacroKind};
use rustc_span::source_map::{ExpnKind, MultiSpan};
use rustc_span::{Span, Symbol};

/// How a lint level was set.
Expand Down Expand Up @@ -337,16 +337,19 @@ pub fn struct_lint_level<'s, 'd>(
/// This is used to test whether a lint should not even begin to figure out whether it should
/// be reported on the current node.
pub fn in_external_macro(sess: &Session, span: Span) -> bool {
let expn_data = span.ctxt().outer_expn_data();
match expn_data.kind {
ExpnKind::Root
| ExpnKind::Desugaring(DesugaringKind::ForLoop(_))
| ExpnKind::Desugaring(DesugaringKind::Operator) => false,
ExpnKind::AstPass(_) | ExpnKind::Desugaring(_) => true, // well, it's "external"
ExpnKind::Macro(MacroKind::Bang, _) => {
// Dummy span for the `def_site` means it's an external macro.
expn_data.def_site.is_dummy() || sess.source_map().is_imported(expn_data.def_site)
match span.ctxt().outer_expn().closest_ast_or_macro() {
ClosestAstOrMacro::Expn(expn_id) => {
let data = expn_id.expn_data();
match data.kind {
ExpnKind::Macro(MacroKind::Bang, _) => {
// Dummy span for the `def_site` means it's an external macro.
data.def_site.is_dummy() || sess.source_map().is_imported(data.def_site)
}
ExpnKind::Macro(_, _) => true, // definitely a plugin
ExpnKind::AstPass(_) => true, // well, it's "External"
_ => unreachable!("unexpected ExpnData {:?}", data),
}
}
ExpnKind::Macro(..) => true, // definitely a plugin
ClosestAstOrMacro::None => false,
}
}
60 changes: 59 additions & 1 deletion src/librustc_span/hygiene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,19 @@ impl ExpnId {
HygieneData::with(|data| data.expn_data(self).clone())
}

#[inline]
pub fn closest_ast_or_macro(self) -> ClosestAstOrMacro {
Aaron1011 marked this conversation as resolved.
Show resolved Hide resolved
HygieneData::with(|data| data.closest_ast_or_macro(self))
}

#[inline]
pub fn set_expn_data(self, expn_data: ExpnData) {
HygieneData::with(|data| {
let closest = data.determine_closest_ast_or_macro(self, &expn_data);
let old_closest = &mut data.closest_ast_or_macro[self.0 as usize];
assert!(old_closest.is_none(), "closest ast/macro data reset for an expansion ID");
*old_closest = Some(closest);

let old_expn_data = &mut data.expn_data[self.0 as usize];
assert!(old_expn_data.is_none(), "expansion data is reset for an expansion ID");
*old_expn_data = Some(expn_data);
Expand Down Expand Up @@ -149,6 +159,10 @@ crate struct HygieneData {
/// between creation of an expansion ID and obtaining its data (e.g. macros are collected
/// first and then resolved later), so we use an `Option` here.
expn_data: Vec<Option<ExpnData>>,
/// Stores the computed `ClosestAstOrMacro` for each `ExpnId`. This is updated
/// at the same time as `expn_data`, and its contents it determined entirely
/// by the `ExpnData` - this field is just a cache.
closest_ast_or_macro: Vec<Option<ClosestAstOrMacro>>,
syntax_context_data: Vec<SyntaxContextData>,
syntax_context_map: FxHashMap<(SyntaxContext, ExpnId, Transparency), SyntaxContext>,
}
Expand All @@ -162,6 +176,7 @@ impl HygieneData {
edition,
Some(DefId::local(CRATE_DEF_INDEX)),
))],
closest_ast_or_macro: vec![Some(ClosestAstOrMacro::None)],
syntax_context_data: vec![SyntaxContextData {
outer_expn: ExpnId::root(),
outer_transparency: Transparency::Opaque,
Expand All @@ -178,9 +193,36 @@ impl HygieneData {
GLOBALS.with(|globals| f(&mut *globals.hygiene_data.borrow_mut()))
}

fn determine_closest_ast_or_macro(
&self,
id: ExpnId,
expn_data: &ExpnData,
) -> ClosestAstOrMacro {
match expn_data.kind {
ExpnKind::Macro(_, _) | ExpnKind::AstPass(_) => ClosestAstOrMacro::Expn(id),
ExpnKind::Desugaring(_) | ExpnKind::Root => {
// Avoid using `HygieneData` when construction root
// `ExpnData`
if expn_data.call_site.ctxt() == SyntaxContext::root() {
ClosestAstOrMacro::None
} else {
self.closest_ast_or_macro(self.outer_expn(expn_data.call_site.ctxt()))
}
}
}
}

fn closest_ast_or_macro(&self, expn_id: ExpnId) -> ClosestAstOrMacro {
self.closest_ast_or_macro[expn_id.0 as usize].as_ref().copied().unwrap()
}

fn fresh_expn(&mut self, expn_data: Option<ExpnData>) -> ExpnId {
let expn_id = ExpnId(self.expn_data.len() as u32);
self.closest_ast_or_macro.push(
expn_data.as_ref().map(|data| self.determine_closest_ast_or_macro(expn_id, data)),
);
self.expn_data.push(expn_data);
ExpnId(self.expn_data.len() as u32 - 1)
expn_id
}

fn expn_data(&self, expn_id: ExpnId) -> &ExpnData {
Expand Down Expand Up @@ -684,6 +726,22 @@ pub struct ExpnData {
pub macro_def_id: Option<DefId>,
}

/// The closest `ExpnKind::AstPass` or `ExpnKind::Macro` to an `ExpnData`.
/// 'Closest' is determined by starting at the current `ExpnData`,
/// and walking up the `call_site` tree. If an `ExpnData` with
/// `Expn::AstPass` or `ExpnKind::Macro` is found, it is represented
/// by `ClosestAstOrMacro::Expn(id)`, where `id` is the `EpxId` of
/// the found `ExpnData`.
///
/// A `ClosestAstOrMacro` implies that no `ExpnKind::AstPass` or `ExpnKind::Macro`
/// are found anywhere in the `call_site` tree - that is, there no macro
/// expansions or ast pass expansions.
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub enum ClosestAstOrMacro {
None,
Expn(ExpnId),
}

impl ExpnData {
/// Constructs expansion data with default properties.
pub fn default(
Expand Down
6 changes: 3 additions & 3 deletions src/tools/clippy/clippy_lints/src/assertions_on_constants.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::consts::{constant, Constant};
use crate::utils::paths;
use crate::utils::{paths, in_macro};
use crate::utils::{is_direct_expn_of, is_expn_of, match_function_call, snippet_opt, span_lint_and_help};
use if_chain::if_chain;
use rustc_ast::ast::LitKind;
Expand Down Expand Up @@ -67,7 +67,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants {
};

if let Some(debug_assert_span) = is_expn_of(e.span, "debug_assert") {
if debug_assert_span.from_expansion() {
if in_macro(debug_assert_span) {
return;
}
if_chain! {
Expand All @@ -79,7 +79,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants {
}
};
} else if let Some(assert_span) = is_direct_expn_of(e.span, "assert") {
if assert_span.from_expansion() {
if in_macro(assert_span) {
return;
}
if let Some(assert_match) = match_assert_with_message(&cx, e) {
Expand Down
7 changes: 2 additions & 5 deletions src/tools/clippy/clippy_lints/src/attrs.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
//! checks for attributes

use crate::reexport::Name;
use crate::utils::{
first_line_of_span, is_present_in_source, match_def_path, paths, snippet_opt, span_lint, span_lint_and_sugg,
span_lint_and_then, without_block_comments,
};
use crate::utils::{first_line_of_span, is_present_in_source, match_def_path, paths, snippet_opt, span_lint, span_lint_and_sugg, span_lint_and_then, without_block_comments, in_macro};
use if_chain::if_chain;
use rustc_ast::ast::{AttrKind, AttrStyle, Attribute, Lit, LitKind, MetaItemKind, NestedMetaItem};
use rustc_ast::util::lev_distance::find_best_match_for_name;
Expand Down Expand Up @@ -474,7 +471,7 @@ fn is_relevant_expr(cx: &LateContext<'_, '_>, tables: &ty::TypeckTables<'_>, exp
}

fn check_attrs(cx: &LateContext<'_, '_>, span: Span, name: Name, attrs: &[Attribute]) {
if span.from_expansion() {
if in_macro(span) {
return;
}

Expand Down
8 changes: 4 additions & 4 deletions src/tools/clippy/clippy_lints/src/blocks_in_if_conditions.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::{differing_macro_contexts, higher, snippet_block_with_applicability, span_lint, span_lint_and_sugg};
use crate::utils::{differing_macro_contexts, higher, snippet_block_with_applicability, span_lint, span_lint_and_sugg, in_macro};
use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
use rustc_hir::{BlockCheckMode, Expr, ExprKind};
Expand Down Expand Up @@ -55,7 +55,7 @@ impl<'a, 'tcx> Visitor<'tcx> for ExVisitor<'a, 'tcx> {
if let ExprKind::Closure(_, _, eid, _, _) = expr.kind {
let body = self.cx.tcx.hir().body(eid);
let ex = &body.value;
if matches!(ex.kind, ExprKind::Block(_, _)) && !body.value.span.from_expansion() {
if matches!(ex.kind, ExprKind::Block(_, _)) && !in_macro(body.value.span) {
self.found_block = Some(ex);
return;
}
Expand Down Expand Up @@ -83,7 +83,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlocksInIfConditions {
if let Some(ex) = &block.expr {
// don't dig into the expression here, just suggest that they remove
// the block
if expr.span.from_expansion() || differing_macro_contexts(expr.span, ex.span) {
if in_macro(expr.span) || differing_macro_contexts(expr.span, ex.span) {
return;
}
let mut applicability = Applicability::MachineApplicable;
Expand All @@ -108,7 +108,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlocksInIfConditions {
}
} else {
let span = block.expr.as_ref().map_or_else(|| block.stmts[0].span, |e| e.span);
if span.from_expansion() || differing_macro_contexts(expr.span, span) {
if in_macro(span) || differing_macro_contexts(expr.span, span) {
return;
}
// move block higher
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/booleans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ impl<'a, 'tcx, 'v> Hir2Qmm<'a, 'tcx, 'v> {
}

// prevent folding of `cfg!` macros and the like
if !e.span.from_expansion() {
if !in_macro(e.span) {
match &e.kind {
ExprKind::Unary(UnOp::UnNot, inner) => return Ok(Bool::Not(box self.run(inner)?)),
ExprKind::Binary(binop, lhs, rhs) => match &binop.node {
Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/cognitive_complexity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::source_map::Span;
use rustc_span::BytePos;

use crate::utils::{is_type_diagnostic_item, snippet_opt, span_lint_and_help, LimitStack};
use crate::utils::{is_type_diagnostic_item, snippet_opt, span_lint_and_help, LimitStack, in_macro};

declare_clippy_lint! {
/// **What it does:** Checks for methods with high cognitive complexity.
Expand Down Expand Up @@ -51,7 +51,7 @@ impl CognitiveComplexity {
body: &'tcx Body<'_>,
body_span: Span,
) {
if body_span.from_expansion() {
if in_macro(body_span) {
return;
}

Expand Down
6 changes: 3 additions & 3 deletions src/tools/clippy/clippy_lints/src/collapsible_if.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};

use crate::utils::sugg::Sugg;
use crate::utils::{snippet_block, snippet_block_with_applicability, span_lint_and_sugg, span_lint_and_then};
use crate::utils::{snippet_block, snippet_block_with_applicability, span_lint_and_sugg, span_lint_and_then, in_macro};
use rustc_errors::Applicability;

declare_clippy_lint! {
Expand Down Expand Up @@ -75,7 +75,7 @@ declare_lint_pass!(CollapsibleIf => [COLLAPSIBLE_IF]);

impl EarlyLintPass for CollapsibleIf {
fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) {
if !expr.span.from_expansion() {
if !in_macro(expr.span) {
check_if(cx, expr)
}
}
Expand Down Expand Up @@ -106,7 +106,7 @@ fn check_collapsible_maybe_if_let(cx: &EarlyContext<'_>, else_: &ast::Expr) {
if let ast::ExprKind::Block(ref block, _) = else_.kind;
if !block_starts_with_comment(cx, block);
if let Some(else_) = expr_block(block);
if !else_.span.from_expansion();
if !in_macro(else_.span);
if let ast::ExprKind::If(..) = else_.kind;
then {
let mut applicability = Applicability::MachineApplicable;
Expand Down
6 changes: 2 additions & 4 deletions src/tools/clippy/clippy_lints/src/comparison_chain.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
use crate::utils::{
get_trait_def_id, if_sequence, implements_trait, parent_node_is_if_expr, paths, span_lint_and_help, SpanlessEq,
};
use crate::utils::{get_trait_def_id, if_sequence, implements_trait, parent_node_is_if_expr, paths, span_lint_and_help, SpanlessEq, in_macro};
use rustc_hir::{BinOpKind, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
Expand Down Expand Up @@ -54,7 +52,7 @@ declare_lint_pass!(ComparisonChain => [COMPARISON_CHAIN]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ComparisonChain {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
if expr.span.from_expansion() {
if in_macro(expr.span) {
return;
}

Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/copies.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::{get_parent_expr, higher, if_sequence, snippet, span_lint_and_note, span_lint_and_then};
use crate::utils::{get_parent_expr, higher, if_sequence, snippet, span_lint_and_note, span_lint_and_then, in_macro};
use crate::utils::{SpanlessEq, SpanlessHash};
use rustc_data_structures::fx::FxHashMap;
use rustc_hir::{Arm, Block, Expr, ExprKind, MatchSource, Pat, PatKind};
Expand Down Expand Up @@ -153,7 +153,7 @@ declare_lint_pass!(CopyAndPaste => [IFS_SAME_COND, SAME_FUNCTIONS_IN_IF_CONDITIO

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CopyAndPaste {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
if !expr.span.from_expansion() {
if !in_macro(expr.span) {
// skip ifs directly in else, it will be checked in the parent if
if let Some(expr) = get_parent_expr(cx, expr) {
if let Some((_, _, Some(ref else_expr))) = higher::if_block(&expr) {
Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/dereference.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::{get_parent_expr, implements_trait, snippet, span_lint_and_sugg};
use crate::utils::{get_parent_expr, implements_trait, snippet, span_lint_and_sugg, in_macro};
use if_chain::if_chain;
use rustc_ast::util::parser::{ExprPrecedence, PREC_POSTFIX, PREC_PREFIX};
use rustc_errors::Applicability;
Expand Down Expand Up @@ -41,7 +41,7 @@ declare_lint_pass!(Dereferencing => [
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
if_chain! {
if !expr.span.from_expansion();
if !in_macro(expr.span);
if let ExprKind::MethodCall(ref method_name, _, ref args, _) = &expr.kind;
if args.len() == 1;

Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/double_parens.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::span_lint;
use crate::utils::{span_lint, in_macro};
use rustc_ast::ast::{Expr, ExprKind};
use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
Expand Down Expand Up @@ -41,7 +41,7 @@ declare_lint_pass!(DoubleParens => [DOUBLE_PARENS]);

impl EarlyLintPass for DoubleParens {
fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
if expr.span.from_expansion() {
if in_macro(expr.span) {
return;
}

Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/enum_variants.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! lint on enum variants that are prefixed or suffixed by the same characters

use crate::utils::{camel_case, is_present_in_source};
use crate::utils::{camel_case, is_present_in_source, in_macro};
use crate::utils::{span_lint, span_lint_and_help};
use rustc_ast::ast::{EnumDef, Item, ItemKind, VisibilityKind};
use rustc_lint::{EarlyContext, EarlyLintPass, Lint};
Expand Down Expand Up @@ -271,7 +271,7 @@ impl EarlyLintPass for EnumVariantNames {
let item_name = item.ident.name.as_str();
let item_name_chars = item_name.chars().count();
let item_camel = to_camel_case(&item_name);
if !item.span.from_expansion() && is_present_in_source(cx, item.span) {
if !in_macro(item.span) && is_present_in_source(cx, item.span) {
if let Some(&(ref mod_name, ref mod_camel)) = self.modules.last() {
// constants don't have surrounding modules
if !mod_camel.is_empty() {
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/eq_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EqOp {
#[allow(clippy::similar_names, clippy::too_many_lines)]
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr<'_>) {
if let ExprKind::Binary(op, ref left, ref right) = e.kind {
if e.span.from_expansion() {
if in_macro(e.span) {
return;
}
let macro_with_not_op = |expr_kind: &ExprKind<'_>| {
Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/erasing_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span;

use crate::consts::{constant_simple, Constant};
use crate::utils::span_lint;
use crate::utils::{span_lint, in_macro};

declare_clippy_lint! {
/// **What it does:** Checks for erasing operations, e.g., `x * 0`.
Expand All @@ -31,7 +31,7 @@ declare_lint_pass!(ErasingOp => [ERASING_OP]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ErasingOp {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr<'_>) {
if e.span.from_expansion() {
if in_macro(e.span) {
return;
}
if let ExprKind::Binary(ref cmp, ref left, ref right) = e.kind {
Expand Down
Loading