Skip to content

Commit

Permalink
Rollup merge of rust-lang#64462 - petrochenkov:remattr, r=Centril
Browse files Browse the repository at this point in the history
feature_gate: Remove dead code from attribute checking

rust-lang#63468 is merged, so all attributes go through name resolution now, so we can remove code that previously performed some checks for attributes not going through resolution.
  • Loading branch information
Centril committed Sep 14, 2019
2 parents 5f9b9b2 + cb771fd commit 45e50e2
Show file tree
Hide file tree
Showing 13 changed files with 63 additions and 161 deletions.
11 changes: 2 additions & 9 deletions src/librustc_interface/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ use syntax::mut_visit::MutVisitor;
use syntax::parse::{self, PResult};
use syntax::util::node_count::NodeCounter;
use syntax::symbol::Symbol;
use syntax::feature_gate::AttributeType;
use syntax_pos::FileName;
use syntax_ext;

Expand Down Expand Up @@ -219,7 +218,6 @@ impl BoxedResolver {

pub struct PluginInfo {
syntax_exts: Vec<NamedSyntaxExtension>,
attributes: Vec<(Symbol, AttributeType)>,
}

pub fn register_plugins<'a>(
Expand Down Expand Up @@ -312,12 +310,9 @@ pub fn register_plugins<'a>(
}

*sess.plugin_llvm_passes.borrow_mut() = llvm_passes;
*sess.plugin_attributes.borrow_mut() = attributes.clone();
*sess.plugin_attributes.borrow_mut() = attributes;

Ok((krate, PluginInfo {
syntax_exts,
attributes,
}))
Ok((krate, PluginInfo { syntax_exts }))
}

fn configure_and_expand_inner<'a>(
Expand All @@ -329,7 +324,6 @@ fn configure_and_expand_inner<'a>(
crate_loader: &'a mut CrateLoader<'a>,
plugin_info: PluginInfo,
) -> Result<(ast::Crate, Resolver<'a>)> {
let attributes = plugin_info.attributes;
time(sess, "pre ast expansion lint checks", || {
lint::check_ast_crate(
sess,
Expand Down Expand Up @@ -522,7 +516,6 @@ fn configure_and_expand_inner<'a>(
&krate,
&sess.parse_sess,
&sess.features_untracked(),
&attributes,
sess.opts.unstable_features,
);
});
Expand Down
15 changes: 3 additions & 12 deletions src/libsyntax/ext/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use syntax_pos::{Span, DUMMY_SP, FileName};
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::sync::Lrc;
use std::io::ErrorKind;
use std::{iter, mem};
use std::{iter, mem, slice};
use std::ops::DerefMut;
use std::rc::Rc;
use std::path::PathBuf;
Expand Down Expand Up @@ -1019,7 +1019,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
fn check_attributes(&mut self, attrs: &[ast::Attribute]) {
let features = self.cx.ecfg.features.unwrap();
for attr in attrs.iter() {
self.check_attribute_inner(attr, features);
feature_gate::check_attribute(attr, self.cx.parse_sess, features);

// macros are expanded before any lint passes so this warning has to be hardcoded
if attr.path == sym::derive {
Expand All @@ -1029,15 +1029,6 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
}
}
}

fn check_attribute(&mut self, at: &ast::Attribute) {
let features = self.cx.ecfg.features.unwrap();
self.check_attribute_inner(at, features);
}

fn check_attribute_inner(&mut self, at: &ast::Attribute, features: &Features) {
feature_gate::check_attribute(at, self.cx.parse_sess, features);
}
}

impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
Expand Down Expand Up @@ -1445,7 +1436,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {

if let Some(file) = it.value_str() {
let err_count = self.cx.parse_sess.span_diagnostic.err_count();
self.check_attribute(&at);
self.check_attributes(slice::from_ref(at));
if self.cx.parse_sess.span_diagnostic.err_count() > err_count {
// avoid loading the file if they haven't enabled the feature
return noop_visit_attribute(at, self);
Expand Down
1 change: 1 addition & 0 deletions src/libsyntax/feature_gate/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ pub enum AttributeType {
CrateLevel,
}

#[derive(Clone, Copy)]
pub enum AttributeGate {
/// Is gated by a given feature gate, reason
/// and function to check if enabled
Expand Down
178 changes: 47 additions & 131 deletions src/libsyntax/feature_gate/check.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use super::{active::{ACTIVE_FEATURES, Features}, Feature, State as FeatureState};
use super::accepted::ACCEPTED_FEATURES;
use super::removed::{REMOVED_FEATURES, STABLE_REMOVED_FEATURES};
use super::builtin_attrs::{AttributeGate, AttributeType, BuiltinAttribute, BUILTIN_ATTRIBUTE_MAP};
use super::builtin_attrs::{AttributeGate, BUILTIN_ATTRIBUTE_MAP};

use crate::ast::{
self, AssocTyConstraint, AssocTyConstraintKind, NodeId, GenericParam, GenericParamKind,
Expand Down Expand Up @@ -32,16 +32,10 @@ pub enum Stability {
Deprecated(&'static str, Option<&'static str>),
}

struct Context<'a> {
features: &'a Features,
parse_sess: &'a ParseSess,
plugin_attributes: &'a [(Symbol, AttributeType)],
}

macro_rules! gate_feature_fn {
($cx: expr, $has_feature: expr, $span: expr, $name: expr, $explain: expr, $level: expr) => {{
let (cx, has_feature, span,
name, explain, level) = ($cx, $has_feature, $span, $name, $explain, $level);
name, explain, level) = (&*$cx, $has_feature, $span, $name, $explain, $level);
let has_feature: bool = has_feature(&$cx.features);
debug!("gate_feature(feature = {:?}, span = {:?}); has? {}", name, span, has_feature);
if !has_feature && !span.allows_unstable($name) {
Expand All @@ -62,68 +56,8 @@ macro_rules! gate_feature {
};
}

impl<'a> Context<'a> {
fn check_attribute(
&self,
attr: &ast::Attribute,
attr_info: Option<&BuiltinAttribute>,
is_macro: bool
) {
debug!("check_attribute(attr = {:?})", attr);
if let Some(&(name, ty, _template, ref gateage)) = attr_info {
if let AttributeGate::Gated(_, name, desc, ref has_feature) = *gateage {
if !attr.span.allows_unstable(name) {
gate_feature_fn!(
self, has_feature, attr.span, name, desc, GateStrength::Hard
);
}
} else if name == sym::doc {
if let Some(content) = attr.meta_item_list() {
if content.iter().any(|c| c.check_name(sym::include)) {
gate_feature!(self, external_doc, attr.span,
"`#[doc(include = \"...\")]` is experimental"
);
}
}
}
debug!("check_attribute: {:?} is builtin, {:?}, {:?}", attr.path, ty, gateage);
return;
} else {
for segment in &attr.path.segments {
if segment.ident.as_str().starts_with("rustc") {
let msg = "attributes starting with `rustc` are \
reserved for use by the `rustc` compiler";
gate_feature!(self, rustc_attrs, segment.ident.span, msg);
}
}
}
for &(n, ty) in self.plugin_attributes {
if attr.path == n {
// Plugins can't gate attributes, so we don't check for it
// unlike the code above; we only use this loop to
// short-circuit to avoid the checks below.
debug!("check_attribute: {:?} is registered by a plugin, {:?}", attr.path, ty);
return;
}
}
if !is_macro && !attr::is_known(attr) {
// Only run the custom attribute lint during regular feature gate
// checking. Macro gating runs before the plugin attributes are
// registered, so we skip this in that case.
let msg = format!("the attribute `{}` is currently unknown to the compiler and \
may have meaning added to it in the future", attr.path);
gate_feature!(self, custom_attribute, attr.span, &msg);
}
}
}

pub fn check_attribute(attr: &ast::Attribute, parse_sess: &ParseSess, features: &Features) {
let cx = Context { features, parse_sess, plugin_attributes: &[] };
cx.check_attribute(
attr,
attr.ident().and_then(|ident| BUILTIN_ATTRIBUTE_MAP.get(&ident.name).map(|a| *a)),
true
);
crate fn check_attribute(attr: &ast::Attribute, parse_sess: &ParseSess, features: &Features) {
PostExpansionVisitor { parse_sess, features }.visit_attribute(attr)
}

fn find_lang_feature_issue(feature: Symbol) -> Option<u32> {
Expand Down Expand Up @@ -238,21 +172,21 @@ pub const EXPLAIN_UNSIZED_TUPLE_COERCION: &str =
"unsized tuple coercion is not stable enough for use and is subject to change";

struct PostExpansionVisitor<'a> {
context: &'a Context<'a>,
builtin_attributes: &'static FxHashMap<Symbol, &'static BuiltinAttribute>,
parse_sess: &'a ParseSess,
features: &'a Features,
}

macro_rules! gate_feature_post {
($cx: expr, $feature: ident, $span: expr, $explain: expr) => {{
let (cx, span) = ($cx, $span);
if !span.allows_unstable(sym::$feature) {
gate_feature!(cx.context, $feature, span, $explain)
gate_feature!(cx, $feature, span, $explain)
}
}};
($cx: expr, $feature: ident, $span: expr, $explain: expr, $level: expr) => {{
let (cx, span) = ($cx, $span);
if !span.allows_unstable(sym::$feature) {
gate_feature!(cx.context, $feature, span, $explain, $level)
gate_feature!(cx, $feature, span, $explain, $level)
}
}}
}
Expand Down Expand Up @@ -316,58 +250,52 @@ impl<'a> PostExpansionVisitor<'a> {

impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
fn visit_attribute(&mut self, attr: &ast::Attribute) {
let attr_info = attr.ident().and_then(|ident| {
self.builtin_attributes.get(&ident.name).map(|a| *a)
});

// Check for gated attributes.
self.context.check_attribute(attr, attr_info, false);

if attr.check_name(sym::doc) {
if let Some(content) = attr.meta_item_list() {
if content.len() == 1 && content[0].check_name(sym::cfg) {
gate_feature_post!(&self, doc_cfg, attr.span,
"`#[doc(cfg(...))]` is experimental"
);
} else if content.iter().any(|c| c.check_name(sym::masked)) {
gate_feature_post!(&self, doc_masked, attr.span,
"`#[doc(masked)]` is experimental"
);
} else if content.iter().any(|c| c.check_name(sym::spotlight)) {
gate_feature_post!(&self, doc_spotlight, attr.span,
"`#[doc(spotlight)]` is experimental"
);
} else if content.iter().any(|c| c.check_name(sym::alias)) {
gate_feature_post!(&self, doc_alias, attr.span,
"`#[doc(alias = \"...\")]` is experimental"
);
} else if content.iter().any(|c| c.check_name(sym::keyword)) {
gate_feature_post!(&self, doc_keyword, attr.span,
"`#[doc(keyword = \"...\")]` is experimental"
);
}
}
let attr_info =
attr.ident().and_then(|ident| BUILTIN_ATTRIBUTE_MAP.get(&ident.name)).map(|a| **a);
// Check feature gates for built-in attributes.
if let Some((.., AttributeGate::Gated(_, name, descr, has_feature))) = attr_info {
gate_feature_fn!(self, has_feature, attr.span, name, descr, GateStrength::Hard);
}

// Check input tokens for built-in and key-value attributes.
match attr_info {
// `rustc_dummy` doesn't have any restrictions specific to built-in attributes.
Some(&(name, _, template, _)) if name != sym::rustc_dummy =>
check_builtin_attribute(self.context.parse_sess, attr, name, template),
Some((name, _, template, _)) if name != sym::rustc_dummy =>
check_builtin_attribute(self.parse_sess, attr, name, template),
_ => if let Some(TokenTree::Token(token)) = attr.tokens.trees().next() {
if token == token::Eq {
// All key-value attributes are restricted to meta-item syntax.
attr.parse_meta(self.context.parse_sess).map_err(|mut err| err.emit()).ok();
attr.parse_meta(self.parse_sess).map_err(|mut err| err.emit()).ok();
}
}
}
// Check unstable flavors of the `#[doc]` attribute.
if attr.check_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.check_name(sym::$name) {
let msg = concat!("`#[doc(", stringify!($name), ")]` is experimental");
gate_feature!(self, $feature, attr.span, msg);
})*
}}

gate_doc!(
include => external_doc
cfg => doc_cfg
masked => doc_masked
spotlight => doc_spotlight
alias => doc_alias
keyword => doc_keyword
);
}
}
}

fn visit_name(&mut self, sp: Span, name: ast::Name) {
if !name.as_str().is_ascii() {
gate_feature_post!(
&self,
non_ascii_idents,
self.context.parse_sess.source_map().def_span(sp),
self.parse_sess.source_map().def_span(sp),
"non-ascii idents are not fully supported"
);
}
Expand Down Expand Up @@ -423,12 +351,9 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
}
}

let has_feature = self.context.features.arbitrary_enum_discriminant;
let has_feature = self.features.arbitrary_enum_discriminant;
if !has_feature && !i.span.allows_unstable(sym::arbitrary_enum_discriminant) {
Parser::maybe_report_invalid_custom_discriminants(
self.context.parse_sess,
&variants,
);
Parser::maybe_report_invalid_custom_discriminants(self.parse_sess, &variants);
}
}

Expand Down Expand Up @@ -538,7 +463,7 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
ast::ExprKind::Type(..) => {
// To avoid noise about type ascription in common syntax errors, only emit if it
// is the *only* error.
if self.context.parse_sess.span_diagnostic.err_count() == 0 {
if self.parse_sess.span_diagnostic.err_count() == 0 {
gate_feature_post!(&self, type_ascription, e.span,
"type ascription is experimental");
}
Expand Down Expand Up @@ -872,22 +797,17 @@ fn active_features_up_to(edition: Edition) -> impl Iterator<Item=&'static Featur
}

pub fn check_crate(krate: &ast::Crate,
sess: &ParseSess,
parse_sess: &ParseSess,
features: &Features,
plugin_attributes: &[(Symbol, AttributeType)],
unstable: UnstableFeatures) {
maybe_stage_features(&sess.span_diagnostic, krate, unstable);
let ctx = Context {
features,
parse_sess: sess,
plugin_attributes,
};
maybe_stage_features(&parse_sess.span_diagnostic, krate, unstable);
let mut visitor = PostExpansionVisitor { parse_sess, features };

macro_rules! gate_all {
($gate:ident, $msg:literal) => { gate_all!($gate, $gate, $msg); };
($spans:ident, $gate:ident, $msg:literal) => {
for span in &*sess.gated_spans.$spans.borrow() {
gate_feature!(&ctx, $gate, *span, $msg);
for span in &*parse_sess.gated_spans.$spans.borrow() {
gate_feature!(&visitor, $gate, *span, $msg);
}
}
}
Expand All @@ -898,11 +818,7 @@ pub fn check_crate(krate: &ast::Crate,
gate_all!(yields, generators, "yield syntax is experimental");
gate_all!(or_patterns, "or-patterns syntax is experimental");

let visitor = &mut PostExpansionVisitor {
context: &ctx,
builtin_attributes: &*BUILTIN_ATTRIBUTE_MAP,
};
visit::walk_crate(visitor, krate);
visit::walk_crate(&mut visitor, krate);
}

#[derive(Clone, Copy, Hash)]
Expand Down
3 changes: 2 additions & 1 deletion src/libsyntax/feature_gate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ pub use builtin_attrs::{
deprecated_attributes, is_builtin_attr, is_builtin_attr_name,
};
pub use check::{
check_attribute, check_crate, get_features, feature_err, emit_feature_err,
check_crate, get_features, feature_err, emit_feature_err,
Stability, GateIssue, UnstableFeatures,
EXPLAIN_STMT_ATTR_SYNTAX, EXPLAIN_UNSIZED_TUPLE_COERCION,
};
crate use check::check_attribute;
2 changes: 1 addition & 1 deletion src/test/ui/feature-gates/feature-gate-doc_alias.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#[doc(alias = "foo")] //~ ERROR: `#[doc(alias = "...")]` is experimental
#[doc(alias = "foo")] //~ ERROR: `#[doc(alias)]` is experimental
pub struct Foo;

fn main() {}
2 changes: 1 addition & 1 deletion src/test/ui/feature-gates/feature-gate-doc_alias.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error[E0658]: `#[doc(alias = "...")]` is experimental
error[E0658]: `#[doc(alias)]` is experimental
--> $DIR/feature-gate-doc_alias.rs:1:1
|
LL | #[doc(alias = "foo")]
Expand Down
Loading

0 comments on commit 45e50e2

Please sign in to comment.