From 530fc207df1c62e1a1646e57650339fd4658f503 Mon Sep 17 00:00:00 2001 From: Devin R Date: Fri, 13 Mar 2020 18:54:32 -0400 Subject: [PATCH] use hashset not map for keeping track of seen macro refs --- clippy_lints/src/macro_use.rs | 64 +++++++++++++++-------------------- 1 file changed, 27 insertions(+), 37 deletions(-) diff --git a/clippy_lints/src/macro_use.rs b/clippy_lints/src/macro_use.rs index 4c89647a5741..9bd5badaa4c3 100644 --- a/clippy_lints/src/macro_use.rs +++ b/clippy_lints/src/macro_use.rs @@ -2,7 +2,7 @@ use crate::utils::{in_macro, snippet, span_lint_and_sugg}; use hir::def::{DefKind, Res}; use if_chain::if_chain; use rustc_ast::ast; -use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::fx::FxHashSet; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::{LateContext, LateLintPass, LintContext}; @@ -42,7 +42,7 @@ impl MacroRefData { let mut path = ecx.sess().source_map().span_to_filename(span).to_string(); // std lib paths are <::std::module::file type> - // so remove brackets and space + // so remove brackets, space and type. if path.contains('<') { path = path.replace(BRACKETS, ""); } @@ -60,10 +60,8 @@ impl MacroRefData { pub struct MacroUseImports { /// the actual import path used and the span of the attribute above it. imports: Vec<(String, Span)>, - /// the span of the macro reference and the `MacroRefData` - /// for the use of the macro. - /// TODO make this FxHashSet to guard against inserting already found macros - collected: FxHashMap, + /// the span of the macro reference, kept to ensure only one reference is used per macro call. + collected: FxHashSet, mac_refs: Vec<(Span, MacroRefData)>, } @@ -80,14 +78,9 @@ impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports { .find(|attr| attr.ident().map(|s| s.to_string()) == Some("macro_use".to_string())); if let Res::Def(DefKind::Mod, id) = path.res; then { - // println!("{:#?}", lcx.tcx.def_path_str(id)); for kid in lcx.tcx.item_children(id).iter() { - // println!("{:#?}", kid); if let Res::Def(DefKind::Macro(_mac_type), mac_id) = kid.res { let span = mac_attr.span.clone(); - - // println!("{:#?}", lcx.tcx.def_path_str(mac_id)); - self.imports.push((lcx.tcx.def_path_str(mac_id), span)); } } @@ -96,10 +89,9 @@ impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports { let call_site = item.span.source_callsite(); let name = snippet(lcx, lcx.sess().source_map().span_until_char(call_site, '!'), "_"); if let Some(callee) = item.span.source_callee() { - if !self.collected.contains_key(&call_site) { - let mac = MacroRefData::new(name.to_string(), callee.def_site, lcx); - self.mac_refs.push((call_site, mac.clone())); - self.collected.insert(call_site, mac); + if !self.collected.contains(&call_site) { + self.mac_refs.push((call_site, MacroRefData::new(name.into(), callee.def_site, lcx))); + self.collected.insert(call_site); } } } @@ -111,18 +103,16 @@ impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports { let call_site = attr.span.source_callsite(); let name = snippet(lcx, lcx.sess().source_map().span_until_char(call_site, '!'), "_"); if let Some(callee) = attr.span.source_callee() { - if !self.collected.contains_key(&call_site) { - println!("{:?}\n{:#?}", call_site, attr); - + if !self.collected.contains(&call_site) { let name = if name.contains("::") { name.split("::").last().unwrap().to_string() } else { name.to_string() }; - let mac = MacroRefData::new(name, callee.def_site, lcx); - self.mac_refs.push((call_site, mac.clone())); - self.collected.insert(call_site, mac); + self.mac_refs + .push((call_site, MacroRefData::new(name, callee.def_site, lcx))); + self.collected.insert(call_site); } } } @@ -132,16 +122,16 @@ impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports { let call_site = expr.span.source_callsite(); let name = snippet(lcx, lcx.sess().source_map().span_until_char(call_site, '!'), "_"); if let Some(callee) = expr.span.source_callee() { - if !self.collected.contains_key(&call_site) { + if !self.collected.contains(&call_site) { let name = if name.contains("::") { name.split("::").last().unwrap().to_string() } else { name.to_string() }; - let mac = MacroRefData::new(name, callee.def_site, lcx); - self.mac_refs.push((call_site, mac.clone())); - self.collected.insert(call_site, mac); + self.mac_refs + .push((call_site, MacroRefData::new(name, callee.def_site, lcx))); + self.collected.insert(call_site); } } } @@ -151,16 +141,16 @@ impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports { let call_site = stmt.span.source_callsite(); let name = snippet(lcx, lcx.sess().source_map().span_until_char(call_site, '!'), "_"); if let Some(callee) = stmt.span.source_callee() { - if !self.collected.contains_key(&call_site) { + if !self.collected.contains(&call_site) { let name = if name.contains("::") { name.split("::").last().unwrap().to_string() } else { name.to_string() }; - let mac = MacroRefData::new(name, callee.def_site, lcx); - self.mac_refs.push((call_site, mac.clone())); - self.collected.insert(call_site, mac); + self.mac_refs + .push((call_site, MacroRefData::new(name, callee.def_site, lcx))); + self.collected.insert(call_site); } } } @@ -170,10 +160,10 @@ impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports { let call_site = pat.span.source_callsite(); let name = snippet(lcx, lcx.sess().source_map().span_until_char(call_site, '!'), "_"); if let Some(callee) = pat.span.source_callee() { - if !self.collected.contains_key(&call_site) { - let mac = MacroRefData::new(name.to_string(), callee.def_site, lcx); - self.mac_refs.push((call_site, mac.clone())); - self.collected.insert(call_site, mac); + if !self.collected.contains(&call_site) { + self.mac_refs + .push((call_site, MacroRefData::new(name.to_string(), callee.def_site, lcx))); + self.collected.insert(call_site); } } } @@ -183,10 +173,10 @@ impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports { let call_site = ty.span.source_callsite(); let name = snippet(lcx, lcx.sess().source_map().span_until_char(call_site, '!'), "_"); if let Some(callee) = ty.span.source_callee() { - if !self.collected.contains_key(&call_site) { - let mac = MacroRefData::new(name.to_string(), callee.def_site, lcx); - self.mac_refs.push((call_site, mac.clone())); - self.collected.insert(call_site, mac); + if !self.collected.contains(&call_site) { + self.mac_refs + .push((call_site, MacroRefData::new(name.to_string(), callee.def_site, lcx))); + self.collected.insert(call_site); } } }