Skip to content

Commit

Permalink
Auto merge of #87739 - Aaron1011:remove-used-attrs, r=wesleywiser
Browse files Browse the repository at this point in the history
Remove `Session.used_attrs` and move logic to `CheckAttrVisitor`

Instead of updating global state to mark attributes as used,
we now explicitly emit a warning when an attribute is used in
an unsupported position. As a side effect, we are to emit more
detailed warning messages (instead of just a generic "unused" message).

`Session.check_name` is removed, since its only purpose was to mark
the attribute as used. All of the callers are modified to use
`Attribute.has_name`

Additionally, `AttributeType::AssumedUsed` is removed - an 'assumed
used' attribute is implemented by simply not performing any checks
in `CheckAttrVisitor` for a particular attribute.

We no longer emit unused attribute warnings for the `#[rustc_dummy]`
attribute - it's an internal attribute used for tests, so it doesn't
mark sense to treat it as 'unused'.

With this commit, a large source of global untracked state is removed.
  • Loading branch information
bors committed Aug 24, 2021
2 parents 5ca596f + 62aea8c commit f66e825
Show file tree
Hide file tree
Showing 64 changed files with 893 additions and 1,382 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4115,10 +4115,12 @@ dependencies = [
"rustc_attr",
"rustc_data_structures",
"rustc_errors",
"rustc_feature",
"rustc_hir",
"rustc_index",
"rustc_lexer",
"rustc_middle",
"rustc_parse",
"rustc_serialize",
"rustc_session",
"rustc_span",
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2281,7 +2281,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
synthetic: param
.attrs
.iter()
.filter(|attr| self.sess.check_name(attr, sym::rustc_synthetic))
.filter(|attr| attr.has_name(sym::rustc_synthetic))
.map(|_| hir::SyntheticTyParamKind::FromAttr)
.next(),
};
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_ast_passes/src/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
gate_feature_fn!(self, has_feature, attr.span, name, descr);
}
// Check unstable flavors of the `#[doc]` attribute.
if self.sess.check_name(attr, sym::doc) {
if attr.has_name(sym::doc) {
for nested_meta in attr.meta_item_list().unwrap_or_default() {
macro_rules! gate_doc { ($($name:ident => $feature:ident)*) => {
$(if nested_meta.has_name(sym::$name) {
Expand All @@ -287,7 +287,7 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
}

// Check for unstable modifiers on `#[link(..)]` attribute
if self.sess.check_name(attr, sym::link) {
if attr.has_name(sym::link) {
for nested_meta in attr.meta_item_list().unwrap_or_default() {
if nested_meta.has_name(sym::modifiers) {
gate_feature_post!(
Expand Down Expand Up @@ -709,7 +709,7 @@ fn maybe_stage_features(sess: &Session, krate: &ast::Crate) {

if !sess.opts.unstable_features.is_nightly_build() {
let lang_features = &sess.features_untracked().declared_lang_features;
for attr in krate.attrs.iter().filter(|attr| sess.check_name(attr, sym::feature)) {
for attr in krate.attrs.iter().filter(|attr| attr.has_name(sym::feature)) {
let mut err = struct_span_err!(
sess.parse_sess.span_diagnostic,
attr.span,
Expand Down
25 changes: 9 additions & 16 deletions compiler/rustc_attr/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,6 @@ where
continue; // not a stability level
}

sess.mark_attr_used(attr);

let meta = attr.meta();

if attr.has_name(sym::rustc_promotable) {
Expand Down Expand Up @@ -636,8 +634,7 @@ where
let diagnostic = &sess.parse_sess.span_diagnostic;

'outer: for attr in attrs_iter {
if !(sess.check_name(attr, sym::deprecated) || sess.check_name(attr, sym::rustc_deprecated))
{
if !(attr.has_name(sym::deprecated) || attr.has_name(sym::rustc_deprecated)) {
continue;
}

Expand Down Expand Up @@ -700,17 +697,17 @@ where
continue 'outer;
}
}
sym::note if sess.check_name(attr, sym::deprecated) => {
sym::note if attr.has_name(sym::deprecated) => {
if !get(mi, &mut note) {
continue 'outer;
}
}
sym::reason if sess.check_name(attr, sym::rustc_deprecated) => {
sym::reason if attr.has_name(sym::rustc_deprecated) => {
if !get(mi, &mut note) {
continue 'outer;
}
}
sym::suggestion if sess.check_name(attr, sym::rustc_deprecated) => {
sym::suggestion if attr.has_name(sym::rustc_deprecated) => {
if !get(mi, &mut suggestion) {
continue 'outer;
}
Expand All @@ -721,7 +718,7 @@ where
meta.span(),
AttrError::UnknownMetaItem(
pprust::path_to_string(&mi.path),
if sess.check_name(attr, sym::deprecated) {
if attr.has_name(sym::deprecated) {
&["since", "note"]
} else {
&["since", "reason", "suggestion"]
Expand All @@ -747,11 +744,11 @@ where
}
}

if suggestion.is_some() && sess.check_name(attr, sym::deprecated) {
if suggestion.is_some() && attr.has_name(sym::deprecated) {
unreachable!("only allowed on rustc_deprecated")
}

if sess.check_name(attr, sym::rustc_deprecated) {
if attr.has_name(sym::rustc_deprecated) {
if since.is_none() {
handle_errors(&sess.parse_sess, attr.span, AttrError::MissingSince);
continue;
Expand All @@ -763,9 +760,7 @@ where
}
}

sess.mark_attr_used(&attr);

let is_since_rustc_version = sess.check_name(attr, sym::rustc_deprecated);
let is_since_rustc_version = attr.has_name(sym::rustc_deprecated);
depr = Some((Deprecation { since, note, suggestion, is_since_rustc_version }, attr.span));
}

Expand Down Expand Up @@ -816,7 +811,6 @@ pub fn find_repr_attrs(sess: &Session, attr: &Attribute) -> Vec<ReprAttr> {
let diagnostic = &sess.parse_sess.span_diagnostic;
if attr.has_name(sym::repr) {
if let Some(items) = attr.meta_item_list() {
sess.mark_attr_used(attr);
for item in items {
let mut recognised = false;
if item.is_word() {
Expand Down Expand Up @@ -1015,14 +1009,13 @@ pub enum TransparencyError {
}

pub fn find_transparency(
sess: &Session,
attrs: &[Attribute],
macro_rules: bool,
) -> (Transparency, Option<TransparencyError>) {
let mut transparency = None;
let mut error = None;
for attr in attrs {
if sess.check_name(attr, sym::rustc_macro_transparency) {
if attr.has_name(sym::rustc_macro_transparency) {
if let Some((_, old_span)) = transparency {
error = Some(TransparencyError::MultipleTransparencyAttrs(old_span, attr.span));
break;
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_builtin_macros/src/deriving/generic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -677,8 +677,6 @@ impl<'a> TraitDef<'a> {
let self_type = cx.ty_path(path);

let attr = cx.attribute(cx.meta_word(self.span, sym::automatically_derived));
// Just mark it now since we know that it'll end up used downstream
cx.sess.mark_attr_used(&attr);
let opt_trait_ref = Some(trait_ref);
let unused_qual = {
let word = rustc_ast::attr::mk_nested_word_item(Ident::new(
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_builtin_macros/src/proc_macro_harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,11 +260,11 @@ impl<'a> Visitor<'a> for CollectProcMacros<'a> {
return;
}

if self.sess.check_name(attr, sym::proc_macro_derive) {
if attr.has_name(sym::proc_macro_derive) {
self.collect_custom_derive(item, attr);
} else if self.sess.check_name(attr, sym::proc_macro_attribute) {
} else if attr.has_name(sym::proc_macro_attribute) {
self.collect_attr_proc_macro(item);
} else if self.sess.check_name(attr, sym::proc_macro) {
} else if attr.has_name(sym::proc_macro) {
self.collect_bang_proc_macro(item);
};

Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_builtin_macros/src/test_harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,7 @@ impl<'a> MutVisitor for EntryPointCleaner<'a> {
let attrs = attrs
.into_iter()
.filter(|attr| {
!self.sess.check_name(attr, sym::rustc_main)
&& !self.sess.check_name(attr, sym::start)
!attr.has_name(sym::rustc_main) && !attr.has_name(sym::start)
})
.chain(iter::once(allow_dead_code))
.collect();
Expand Down
11 changes: 4 additions & 7 deletions compiler/rustc_expand/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -654,10 +654,7 @@ pub enum SyntaxExtensionKind {
/// A trivial attribute "macro" that does nothing,
/// only keeps the attribute and marks it as inert,
/// thus making it ineligible for further expansion.
NonMacroAttr {
/// Suppresses the `unused_attributes` lint for this attribute.
mark_used: bool,
},
NonMacroAttr,

/// A token-based derive macro.
Derive(
Expand Down Expand Up @@ -706,7 +703,7 @@ impl SyntaxExtension {
SyntaxExtensionKind::Bang(..) | SyntaxExtensionKind::LegacyBang(..) => MacroKind::Bang,
SyntaxExtensionKind::Attr(..)
| SyntaxExtensionKind::LegacyAttr(..)
| SyntaxExtensionKind::NonMacroAttr { .. } => MacroKind::Attr,
| SyntaxExtensionKind::NonMacroAttr => MacroKind::Attr,
SyntaxExtensionKind::Derive(..) | SyntaxExtensionKind::LegacyDerive(..) => {
MacroKind::Derive
}
Expand Down Expand Up @@ -812,8 +809,8 @@ impl SyntaxExtension {
SyntaxExtension::default(SyntaxExtensionKind::Derive(Box::new(expander)), edition)
}

pub fn non_macro_attr(mark_used: bool, edition: Edition) -> SyntaxExtension {
SyntaxExtension::default(SyntaxExtensionKind::NonMacroAttr { mark_used }, edition)
pub fn non_macro_attr(edition: Edition) -> SyntaxExtension {
SyntaxExtension::default(SyntaxExtensionKind::NonMacroAttr, edition)
}

pub fn expn_data(
Expand Down
69 changes: 13 additions & 56 deletions compiler/rustc_expand/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rustc_ast::token::{DelimToken, Token, TokenKind};
use rustc_ast::tokenstream::{AttrAnnotatedTokenStream, AttrAnnotatedTokenTree};
use rustc_ast::tokenstream::{DelimSpan, Spacing};
use rustc_ast::tokenstream::{LazyTokenStream, TokenTree};
use rustc_ast::{self as ast, AstLike, AttrItem, AttrStyle, Attribute, MetaItem};
use rustc_ast::{self as ast, AstLike, AttrStyle, Attribute, MetaItem};
use rustc_attr as attr;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::map_in_place::MapInPlace;
Expand All @@ -14,7 +14,7 @@ use rustc_feature::{Feature, Features, State as FeatureState};
use rustc_feature::{
ACCEPTED_FEATURES, ACTIVE_FEATURES, REMOVED_FEATURES, STABLE_REMOVED_FEATURES,
};
use rustc_parse::{parse_in, validate_attr};
use rustc_parse::validate_attr;
use rustc_session::parse::feature_err;
use rustc_session::Session;
use rustc_span::edition::{Edition, ALL_EDITIONS};
Expand Down Expand Up @@ -75,7 +75,7 @@ fn get_features(
// Process the edition umbrella feature-gates first, to ensure
// `edition_enabled_features` is completed before it's queried.
for attr in krate_attrs {
if !sess.check_name(attr, sym::feature) {
if !attr.has_name(sym::feature) {
continue;
}

Expand Down Expand Up @@ -108,7 +108,7 @@ fn get_features(
}

for attr in krate_attrs {
if !sess.check_name(attr, sym::feature) {
if !attr.has_name(sym::feature) {
continue;
}

Expand Down Expand Up @@ -237,11 +237,6 @@ macro_rules! configure {
};
}

const CFG_ATTR_GRAMMAR_HELP: &str = "#[cfg_attr(condition, attribute, other_attribute, ...)]";
const CFG_ATTR_NOTE_REF: &str = "for more information, visit \
<https://doc.rust-lang.org/reference/conditional-compilation.html\
#the-cfg_attr-attribute>";

impl<'a> StripUnconfigured<'a> {
pub fn configure<T: AstLike>(&mut self, mut node: T) -> Option<T> {
self.process_cfg_attrs(&mut node);
Expand Down Expand Up @@ -349,19 +344,17 @@ impl<'a> StripUnconfigured<'a> {
return vec![attr];
}

let (cfg_predicate, expanded_attrs) = match self.parse_cfg_attr(&attr) {
None => return vec![],
Some(r) => r,
};
let (cfg_predicate, expanded_attrs) =
match rustc_parse::parse_cfg_attr(&attr, &self.sess.parse_sess) {
None => return vec![],
Some(r) => r,
};

// Lint on zero attributes in source.
if expanded_attrs.is_empty() {
return vec![attr];
}

// At this point we know the attribute is considered used.
self.sess.mark_attr_used(&attr);

if !attr::cfg_matches(&cfg_predicate, &self.sess.parse_sess, self.features) {
return vec![];
}
Expand Down Expand Up @@ -415,46 +408,10 @@ impl<'a> StripUnconfigured<'a> {
.collect()
}

fn parse_cfg_attr(&self, attr: &Attribute) -> Option<(MetaItem, Vec<(AttrItem, Span)>)> {
match attr.get_normal_item().args {
ast::MacArgs::Delimited(dspan, delim, ref tts) if !tts.is_empty() => {
let msg = "wrong `cfg_attr` delimiters";
validate_attr::check_meta_bad_delim(&self.sess.parse_sess, dspan, delim, msg);
match parse_in(&self.sess.parse_sess, tts.clone(), "`cfg_attr` input", |p| {
p.parse_cfg_attr()
}) {
Ok(r) => return Some(r),
Err(mut e) => {
e.help(&format!("the valid syntax is `{}`", CFG_ATTR_GRAMMAR_HELP))
.note(CFG_ATTR_NOTE_REF)
.emit();
}
}
}
_ => self.error_malformed_cfg_attr_missing(attr.span),
}
None
}

fn error_malformed_cfg_attr_missing(&self, span: Span) {
self.sess
.parse_sess
.span_diagnostic
.struct_span_err(span, "malformed `cfg_attr` attribute input")
.span_suggestion(
span,
"missing condition and attribute",
CFG_ATTR_GRAMMAR_HELP.to_string(),
Applicability::HasPlaceholders,
)
.note(CFG_ATTR_NOTE_REF)
.emit();
}

/// Determines if a node with the given attributes should be included in this configuration.
fn in_cfg(&self, attrs: &[Attribute]) -> bool {
attrs.iter().all(|attr| {
if !is_cfg(self.sess, attr) {
if !is_cfg(attr) {
return true;
}
let meta_item = match validate_attr::parse_meta(&self.sess.parse_sess, attr) {
Expand Down Expand Up @@ -500,7 +457,7 @@ impl<'a> StripUnconfigured<'a> {
//
// N.B., this is intentionally not part of the visit_expr() function
// in order for filter_map_expr() to be able to avoid this check
if let Some(attr) = expr.attrs().iter().find(|a| is_cfg(self.sess, a)) {
if let Some(attr) = expr.attrs().iter().find(|a| is_cfg(*a)) {
let msg = "removing an expression is not supported in this position";
self.sess.parse_sess.span_diagnostic.span_err(attr.span, msg);
}
Expand Down Expand Up @@ -536,6 +493,6 @@ pub fn parse_cfg<'a>(meta_item: &'a MetaItem, sess: &Session) -> Option<&'a Meta
}
}

fn is_cfg(sess: &Session, attr: &Attribute) -> bool {
sess.check_name(attr, sym::cfg)
fn is_cfg(attr: &Attribute) -> bool {
attr.has_name(sym::cfg)
}
5 changes: 1 addition & 4 deletions compiler/rustc_expand/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -753,11 +753,8 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
}
}
}
SyntaxExtensionKind::NonMacroAttr { mark_used } => {
SyntaxExtensionKind::NonMacroAttr => {
self.cx.expanded_inert_attrs.mark(&attr);
if *mark_used {
self.cx.sess.mark_attr_used(&attr);
}
item.visit_attrs(|attrs| attrs.insert(pos, attr));
fragment_kind.expect_from_annotatables(iter::once(item))
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_expand/src/mbe/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ pub fn compile_declarative_macro(

valid &= macro_check::check_meta_variables(&sess.parse_sess, def.id, def.span, &lhses, &rhses);

let (transparency, transparency_error) = attr::find_transparency(sess, &def.attrs, macro_rules);
let (transparency, transparency_error) = attr::find_transparency(&def.attrs, macro_rules);
match transparency_error {
Some(TransparencyError::UnknownTransparency(value, span)) => {
diag.span_err(span, &format!("unknown macro transparency: `{}`", value))
Expand Down
Loading

0 comments on commit f66e825

Please sign in to comment.