Skip to content

feature_gate: Remove dead code from attribute checking #64462

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

Merged
merged 3 commits into from
Sep 15, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside: In a follow-up, I think we should consider making this infra not use macros since it tends to be infectious atm and not very readable.

($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