From 4f17dce4dc722e56d5d5069e7317feb17c1de5cc Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Tue, 31 Dec 2019 07:15:43 +0100 Subject: [PATCH 1/8] StripUnconfigured::in_cfg: simplify with slice patterns --- src/librustc_parse/config.rs | 52 +++++++++++------------------------- 1 file changed, 16 insertions(+), 36 deletions(-) diff --git a/src/librustc_parse/config.rs b/src/librustc_parse/config.rs index 8dec64c579e88..3c37501be4162 100644 --- a/src/librustc_parse/config.rs +++ b/src/librustc_parse/config.rs @@ -347,7 +347,13 @@ impl<'a> StripUnconfigured<'a> { if !is_cfg(attr) { return true; } - + let meta_item = match validate_attr::parse_meta(self.sess, attr) { + Ok(meta_item) => meta_item, + Err(mut err) => { + err.emit(); + return true; + } + }; let error = |span, msg, suggestion: &str| { let mut err = self.sess.span_diagnostic.struct_span_err(span, msg); if !suggestion.is_empty() { @@ -361,41 +367,15 @@ impl<'a> StripUnconfigured<'a> { err.emit(); true }; - - let meta_item = match validate_attr::parse_meta(self.sess, attr) { - Ok(meta_item) => meta_item, - Err(mut err) => { - err.emit(); - return true; - } - }; - let nested_meta_items = if let Some(nested_meta_items) = meta_item.meta_item_list() { - nested_meta_items - } else { - return error( - meta_item.span, - "`cfg` is not followed by parentheses", - "cfg(/* predicate */)", - ); - }; - - if nested_meta_items.is_empty() { - return error(meta_item.span, "`cfg` predicate is not specified", ""); - } else if nested_meta_items.len() > 1 { - return error( - nested_meta_items.last().unwrap().span(), - "multiple `cfg` predicates are specified", - "", - ); - } - - match nested_meta_items[0].meta_item() { - Some(meta_item) => attr::cfg_matches(meta_item, self.sess, self.features), - None => error( - nested_meta_items[0].span(), - "`cfg` predicate key cannot be a literal", - "", - ), + let span = meta_item.span; + match meta_item.meta_item_list() { + None => error(span, "`cfg` is not followed by parentheses", "cfg(/* predicate */)"), + Some([]) => error(span, "`cfg` predicate is not specified", ""), + Some([_, .., l]) => error(l.span(), "multiple `cfg` predicates are specified", ""), + Some([single]) => match single.meta_item() { + Some(meta_item) => attr::cfg_matches(meta_item, self.sess, self.features), + None => error(single.span(), "`cfg` predicate key cannot be a literal", ""), + }, } }) } From b8b32a9be341761c3927843c27042febbe8ea39a Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Tue, 31 Dec 2019 07:42:19 +0100 Subject: [PATCH 2/8] simplify config::features --- src/librustc_parse/config.rs | 37 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/src/librustc_parse/config.rs b/src/librustc_parse/config.rs index 3c37501be4162..3f544494704bc 100644 --- a/src/librustc_parse/config.rs +++ b/src/librustc_parse/config.rs @@ -207,30 +207,29 @@ pub fn features( edition: Edition, allow_features: &Option>, ) -> (ast::Crate, Features) { - let features; - { - let mut strip_unconfigured = StripUnconfigured { sess, features: None }; + let mut strip_unconfigured = StripUnconfigured { sess, features: None }; - let unconfigured_attrs = krate.attrs.clone(); - let err_count = sess.span_diagnostic.err_count(); - if let Some(attrs) = strip_unconfigured.configure(krate.attrs) { - krate.attrs = attrs; - } else { - // the entire crate is unconfigured + let unconfigured_attrs = krate.attrs.clone(); + let diag = &sess.span_diagnostic; + let err_count = diag.err_count(); + let features = match strip_unconfigured.configure(krate.attrs) { + None => { + // The entire crate is unconfigured. krate.attrs = Vec::new(); krate.module.items = Vec::new(); - return (krate, Features::default()); + Features::default() } - - features = get_features(&sess.span_diagnostic, &krate.attrs, edition, allow_features); - - // Avoid reconfiguring malformed `cfg_attr`s - if err_count == sess.span_diagnostic.err_count() { - strip_unconfigured.features = Some(&features); - strip_unconfigured.configure(unconfigured_attrs); + Some(attrs) => { + krate.attrs = attrs; + let features = get_features(diag, &krate.attrs, edition, allow_features); + if err_count == diag.err_count() { + // Avoid reconfiguring malformed `cfg_attr`s. + strip_unconfigured.features = Some(&features); + strip_unconfigured.configure(unconfigured_attrs); + } + features } - } - + }; (krate, features) } From 64ea295fe92236e5e5162722f5242f5837af7ee1 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Tue, 31 Dec 2019 08:14:31 +0100 Subject: [PATCH 3/8] expand: extract error_derive_forbidden_on_non_adt --- src/librustc_expand/expand.rs | 44 +++++++++++++++++------------------ 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/librustc_expand/expand.rs b/src/librustc_expand/expand.rs index 90692fe1ec9dd..c7c7f62918d72 100644 --- a/src/librustc_expand/expand.rs +++ b/src/librustc_expand/expand.rs @@ -451,28 +451,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { _ => unreachable!(), }; if !item.derive_allowed() { - let attr = attr::find_by_name(item.attrs(), sym::derive) - .expect("`derive` attribute should exist"); - let span = attr.span; - let mut err = self.cx.struct_span_err( - span, - "`derive` may only be applied to structs, enums and unions", - ); - if let ast::AttrStyle::Inner = attr.style { - let trait_list = derives - .iter() - .map(|t| pprust::path_to_string(t)) - .collect::>(); - let suggestion = format!("#[derive({})]", trait_list.join(", ")); - err.span_suggestion( - span, - "try an outer attribute", - suggestion, - // We don't 𝑘𝑛𝑜𝑤 that the following item is an ADT - Applicability::MaybeIncorrect, - ); - } - err.emit(); + self.error_derive_forbidden_on_non_adt(&derives, &item); } let mut item = self.fully_configure(item); @@ -521,6 +500,27 @@ impl<'a, 'b> MacroExpander<'a, 'b> { fragment_with_placeholders } + fn error_derive_forbidden_on_non_adt(&self, derives: &[Path], item: &Annotatable) { + let attr = + attr::find_by_name(item.attrs(), sym::derive).expect("`derive` attribute should exist"); + let span = attr.span; + let mut err = self + .cx + .struct_span_err(span, "`derive` may only be applied to structs, enums and unions"); + if let ast::AttrStyle::Inner = attr.style { + let trait_list = derives.iter().map(|t| pprust::path_to_string(t)).collect::>(); + let suggestion = format!("#[derive({})]", trait_list.join(", ")); + err.span_suggestion( + span, + "try an outer attribute", + suggestion, + // We don't 𝑘𝑛𝑜𝑤 that the following item is an ADT + Applicability::MaybeIncorrect, + ); + } + err.emit(); + } + fn resolve_imports(&mut self) { if self.monotonic { self.cx.resolver.resolve_imports(); From acad03342412961536641d53628d7ccaa189279b Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Tue, 31 Dec 2019 08:25:49 +0100 Subject: [PATCH 4/8] expand: extract error_recursion_limit_reached --- src/librustc_expand/expand.rs | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/librustc_expand/expand.rs b/src/librustc_expand/expand.rs index c7c7f62918d72..da9660184099e 100644 --- a/src/librustc_expand/expand.rs +++ b/src/librustc_expand/expand.rs @@ -606,21 +606,26 @@ impl<'a, 'b> MacroExpander<'a, 'b> { } } - fn expand_invoc(&mut self, invoc: Invocation, ext: &SyntaxExtensionKind) -> AstFragment { - if self.cx.current_expansion.depth > self.cx.ecfg.recursion_limit { - let expn_data = self.cx.current_expansion.id.expn_data(); - let suggested_limit = self.cx.ecfg.recursion_limit * 2; - let mut err = self.cx.struct_span_err( + fn error_recursion_limit_reached(&mut self) { + let expn_data = self.cx.current_expansion.id.expn_data(); + let suggested_limit = self.cx.ecfg.recursion_limit * 2; + self.cx + .struct_span_err( expn_data.call_site, &format!("recursion limit reached while expanding `{}`", expn_data.kind.descr()), - ); - err.help(&format!( + ) + .help(&format!( "consider adding a `#![recursion_limit=\"{}\"]` attribute to your crate (`{}`)", suggested_limit, self.cx.ecfg.crate_name, - )); - err.emit(); - self.cx.trace_macros_diag(); - FatalError.raise(); + )) + .emit(); + self.cx.trace_macros_diag(); + FatalError.raise(); + } + + fn expand_invoc(&mut self, invoc: Invocation, ext: &SyntaxExtensionKind) -> AstFragment { + if self.cx.current_expansion.depth > self.cx.ecfg.recursion_limit { + self.error_recursion_limit_reached(); } let (fragment_kind, span) = (invoc.fragment_kind, invoc.span()); From 7518492315346ac3821ad9264e08721a571742c8 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Tue, 31 Dec 2019 08:43:33 +0100 Subject: [PATCH 5/8] expand: extract error_wrong_fragment_kind --- src/librustc_expand/expand.rs | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/librustc_expand/expand.rs b/src/librustc_expand/expand.rs index da9660184099e..136cc1d94a078 100644 --- a/src/librustc_expand/expand.rs +++ b/src/librustc_expand/expand.rs @@ -623,6 +623,18 @@ impl<'a, 'b> MacroExpander<'a, 'b> { FatalError.raise(); } + /// A macro's expansion does not fit in this fragment kind. + /// For example, a non-type macro in a type position. + fn error_wrong_fragment_kind(&mut self, kind: AstFragmentKind, mac: &ast::Mac, span: Span) { + let msg = format!( + "non-{kind} macro in {kind} position: {path}", + kind = kind.name(), + path = pprust::path_to_string(&mac.path), + ); + self.cx.span_err(span, &msg); + self.cx.trace_macros_diag(); + } + fn expand_invoc(&mut self, invoc: Invocation, ext: &SyntaxExtensionKind) -> AstFragment { if self.cx.current_expansion.depth > self.cx.ecfg.recursion_limit { self.error_recursion_limit_reached(); @@ -643,13 +655,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { let result = if let Some(result) = fragment_kind.make_from(tok_result) { result } else { - let msg = format!( - "non-{kind} macro in {kind} position: {path}", - kind = fragment_kind.name(), - path = pprust::path_to_string(&mac.path), - ); - self.cx.span_err(span, &msg); - self.cx.trace_macros_diag(); + self.error_wrong_fragment_kind(fragment_kind, &mac, span); fragment_kind.dummy(span) }; self.cx.current_expansion.prior_type_ascription = prev; From fcce5fa6e7ba67db63674dc7369125875c7f39a2 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Tue, 31 Dec 2019 09:11:59 +0100 Subject: [PATCH 6/8] expand: simplify classify_* --- src/librustc_expand/expand.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/librustc_expand/expand.rs b/src/librustc_expand/expand.rs index 136cc1d94a078..91195d379d71c 100644 --- a/src/librustc_expand/expand.rs +++ b/src/librustc_expand/expand.rs @@ -1041,13 +1041,10 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { } /// If `item` is an attr invocation, remove and return the macro attribute and derive traits. - fn classify_item( + fn classify_item( &mut self, - item: &mut T, - ) -> (Option, Vec, /* after_derive */ bool) - where - T: HasAttrs, - { + item: &mut impl HasAttrs, + ) -> (Option, Vec, /* after_derive */ bool) { let (mut attr, mut traits, mut after_derive) = (None, Vec::new(), false); item.visit_attrs(|mut attrs| { @@ -1061,9 +1058,9 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { /// Alternative to `classify_item()` that ignores `#[derive]` so invocations fallthrough /// to the unused-attributes lint (making it an error on statements and expressions /// is a breaking change) - fn classify_nonitem( + fn classify_nonitem( &mut self, - nonitem: &mut T, + nonitem: &mut impl HasAttrs, ) -> (Option, /* after_derive */ bool) { let (mut attr, mut after_derive) = (None, false); From dc6bd6a12347f27c937c19adfda542f694fb61c5 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Tue, 31 Dec 2019 09:59:37 +0100 Subject: [PATCH 7/8] expand: simplify flat_map_item --- src/librustc_expand/expand.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/librustc_expand/expand.rs b/src/librustc_expand/expand.rs index 91195d379d71c..ee0b7fa00de26 100644 --- a/src/librustc_expand/expand.rs +++ b/src/librustc_expand/expand.rs @@ -1383,11 +1383,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { _ => unreachable!(), }) } - ast::ItemKind::Mod(ast::Mod { inner, .. }) => { - if item.ident == Ident::invalid() { - return noop_flat_map_item(item, self); - } - + ast::ItemKind::Mod(ast::Mod { inner, .. }) if item.ident != Ident::invalid() => { let orig_directory_ownership = self.cx.current_expansion.directory_ownership; let mut module = (*self.cx.current_expansion.module).clone(); module.mod_path.push(item.ident); From ec434500157c47143a9b5600a7e34522c49f4e8e Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Tue, 31 Dec 2019 10:06:52 +0100 Subject: [PATCH 8/8] expand: simplify flat_map_item wrt. inline module detection --- src/librustc_expand/expand.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/librustc_expand/expand.rs b/src/librustc_expand/expand.rs index ee0b7fa00de26..316dd02bd9581 100644 --- a/src/librustc_expand/expand.rs +++ b/src/librustc_expand/expand.rs @@ -1383,17 +1383,14 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { _ => unreachable!(), }) } - ast::ItemKind::Mod(ast::Mod { inner, .. }) if item.ident != Ident::invalid() => { + ast::ItemKind::Mod(ast::Mod { inner, inline, .. }) + if item.ident != Ident::invalid() => + { let orig_directory_ownership = self.cx.current_expansion.directory_ownership; let mut module = (*self.cx.current_expansion.module).clone(); module.mod_path.push(item.ident); - // Detect if this is an inline module (`mod m { ... }` as opposed to `mod m;`). - // In the non-inline case, `inner` is never the dummy span (cf. `parse_item_mod`). - // Thus, if `inner` is the dummy span, we know the module is inline. - let inline_module = item.span.contains(inner) || inner.is_dummy(); - - if inline_module { + if inline { if let Some(path) = attr::first_attr_value_str_by_name(&item.attrs, sym::path) { self.cx.current_expansion.directory_ownership = DirectoryOwnership::Owned { relative: None };