From 8dc6e16933f2792a3c6210787b9916908e1e76d0 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 6 May 2015 22:08:36 +0530 Subject: [PATCH 1/4] Add support for registering attributes with rustc in plugins This lets plugin authors opt attributes out of the `custom_attribute` and `unused_attribute` checks. --- src/librustc/plugin/registry.rs | 23 +++++++++++++++++++ src/librustc/session/mod.rs | 3 +++ src/librustc_driver/driver.rs | 8 ++++--- src/librustc_lint/builtin.rs | 16 +++++++++++++- src/libsyntax/feature_gate.rs | 39 ++++++++++++++++++++++----------- 5 files changed, 72 insertions(+), 17 deletions(-) diff --git a/src/librustc/plugin/registry.rs b/src/librustc/plugin/registry.rs index 322b5d3a8cf..c85b30c811c 100644 --- a/src/librustc/plugin/registry.rs +++ b/src/librustc/plugin/registry.rs @@ -20,6 +20,7 @@ use syntax::codemap::Span; use syntax::parse::token; use syntax::ptr::P; use syntax::ast; +use syntax::feature_gate::AttributeType; use std::collections::HashMap; use std::borrow::ToOwned; @@ -54,6 +55,9 @@ pub struct Registry<'a> { #[doc(hidden)] pub llvm_passes: Vec, + + #[doc(hidden)] + pub attributes: Vec<(String, AttributeType)>, } impl<'a> Registry<'a> { @@ -67,6 +71,7 @@ impl<'a> Registry<'a> { lint_passes: vec!(), lint_groups: HashMap::new(), llvm_passes: vec!(), + attributes: vec!(), } } @@ -130,4 +135,22 @@ impl<'a> Registry<'a> { pub fn register_llvm_pass(&mut self, name: &str) { self.llvm_passes.push(name.to_owned()); } + + + /// Register an attribute with an attribute type + /// + /// Registered attributes will bypass the `custom_attribute` feature gate + /// + /// `Whitelisted` attributes will additionally not trigger the `unused_attribute` + /// lint + /// + /// `CrateLevel` attributes will not be allowed on anything other than a crate + pub fn register_attribute(&mut self, name: String, ty: AttributeType) { + if let AttributeType::Gated(..) = ty { + self.sess.err("plugin tried to register a gated attribute. \ + Only `Normal`, `Whitelisted`, and `CrateLevel` \ + attributes are allowed"); + } + self.attributes.push((name, ty)); + } } diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index 7a8ce1bf48e..780fff9b838 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -23,6 +23,7 @@ use syntax::parse; use syntax::parse::token; use syntax::parse::ParseSess; use syntax::{ast, codemap}; +use syntax::feature_gate::AttributeType; use rustc_back::target::Target; @@ -54,6 +55,7 @@ pub struct Session { pub lint_store: RefCell, pub lints: RefCell>>, pub plugin_llvm_passes: RefCell>, + pub plugin_attributes: RefCell>, pub crate_types: RefCell>, pub crate_metadata: RefCell>, pub features: RefCell, @@ -416,6 +418,7 @@ pub fn build_session_(sopts: config::Options, lint_store: RefCell::new(lint::LintStore::new()), lints: RefCell::new(NodeMap()), plugin_llvm_passes: RefCell::new(Vec::new()), + plugin_attributes: RefCell::new(Vec::new()), crate_types: RefCell::new(Vec::new()), crate_metadata: RefCell::new(Vec::new()), delayed_span_bug: RefCell::new(None), diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 45d81ff0f65..70ee041b719 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -444,7 +444,8 @@ pub fn phase_2_configure_and_expand(sess: &Session, } }); - let Registry { syntax_exts, lint_passes, lint_groups, llvm_passes, .. } = registry; + let Registry { syntax_exts, lint_passes, lint_groups, + llvm_passes, attributes, .. } = registry; { let mut ls = sess.lint_store.borrow_mut(); @@ -457,6 +458,7 @@ pub fn phase_2_configure_and_expand(sess: &Session, } *sess.plugin_llvm_passes.borrow_mut() = llvm_passes; + *sess.plugin_attributes.borrow_mut() = attributes.clone(); } // Lint plugins are registered; now we can process command line flags. @@ -511,7 +513,7 @@ pub fn phase_2_configure_and_expand(sess: &Session, let features = syntax::feature_gate::check_crate(sess.codemap(), &sess.parse_sess.span_diagnostic, - &krate); + &krate, &attributes); *sess.features.borrow_mut() = features; sess.abort_if_errors(); }); @@ -541,7 +543,7 @@ pub fn phase_2_configure_and_expand(sess: &Session, let features = syntax::feature_gate::check_crate(sess.codemap(), &sess.parse_sess.span_diagnostic, - &krate); + &krate, &attributes); *sess.features.borrow_mut() = features; sess.abort_if_errors(); }); diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 19e71d6e40d..9242274a7a3 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -641,9 +641,23 @@ impl LintPass for UnusedAttributes { } } + let plugin_attributes = cx.sess().plugin_attributes.borrow_mut(); + for &(ref name, ty) in plugin_attributes.iter() { + match ty { + AttributeType::Whitelisted if attr.check_name(&*name) => { + break; + }, + _ => () + } + } + if !attr::is_used(attr) { cx.span_lint(UNUSED_ATTRIBUTES, attr.span, "unused attribute"); - if KNOWN_ATTRIBUTES.contains(&(&attr.name(), AttributeType::CrateLevel)) { + if KNOWN_ATTRIBUTES.contains(&(&attr.name(), AttributeType::CrateLevel)) || + plugin_attributes.iter() + .find(|&&(ref x, t)| &*attr.name() == &*x && + AttributeType::CrateLevel == t) + .is_some() { let msg = match attr.node.style { ast::AttrOuter => "crate-level attribute should be an inner \ attribute: add an exclamation mark: #![foo]", diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 28deb4eec3f..abb4ce15996 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -359,6 +359,7 @@ struct Context<'a> { features: Vec<&'static str>, span_handler: &'a SpanHandler, cm: &'a CodeMap, + plugin_attributes: &'a [(String, AttributeType)], } impl<'a> Context<'a> { @@ -373,7 +374,7 @@ impl<'a> Context<'a> { self.features.iter().any(|&n| n == feature) } - fn check_attribute(&self, attr: &ast::Attribute) { + fn check_attribute(&self, attr: &ast::Attribute, is_macro: bool) { debug!("check_attribute(attr = {:?})", attr); let name = &*attr.name(); for &(n, ty) in KNOWN_ATTRIBUTES { @@ -385,6 +386,13 @@ impl<'a> Context<'a> { return; } } + for &(ref n, ref ty) in self.plugin_attributes.iter() { + if &*n == name { + // Plugins can't gate attributes, so we don't check for it + debug!("check_attribute: {:?} is registered by a plugin, {:?}", name, ty); + return; + } + } if name.starts_with("rustc_") { self.gate_feature("rustc_attrs", attr.span, "unless otherwise specified, attributes \ @@ -395,12 +403,15 @@ impl<'a> Context<'a> { "attributes of the form `#[derive_*]` are reserved \ for the compiler"); } else { - self.gate_feature("custom_attribute", attr.span, - &format!("The attribute `{}` is currently \ - unknown to the compiler and \ - may have meaning \ - added to it in the future", - name)); + // Only do the custom attribute lint post-expansion + if !is_macro { + self.gate_feature("custom_attribute", attr.span, + &format!("The attribute `{}` is currently \ + unknown to the compiler and \ + may have meaning \ + added to it in the future", + name)); + } } } } @@ -479,7 +490,7 @@ impl<'a, 'v> Visitor<'v> for MacroVisitor<'a> { } fn visit_attribute(&mut self, attr: &'v ast::Attribute) { - self.context.check_attribute(attr); + self.context.check_attribute(attr, true); } } @@ -498,7 +509,7 @@ impl<'a> PostExpansionVisitor<'a> { impl<'a, 'v> Visitor<'v> for PostExpansionVisitor<'a> { fn visit_attribute(&mut self, attr: &ast::Attribute) { if !self.context.cm.span_allows_unstable(attr.span) { - self.context.check_attribute(attr); + self.context.check_attribute(attr, false); } } @@ -685,6 +696,7 @@ impl<'a, 'v> Visitor<'v> for PostExpansionVisitor<'a> { fn check_crate_inner(cm: &CodeMap, span_handler: &SpanHandler, krate: &ast::Crate, + plugin_attributes: &[(String, AttributeType)], check: F) -> Features where F: FnOnce(&mut Context, &ast::Crate) @@ -693,6 +705,7 @@ fn check_crate_inner(cm: &CodeMap, span_handler: &SpanHandler, features: Vec::new(), span_handler: span_handler, cm: cm, + plugin_attributes: plugin_attributes, }; let mut accepted_features = Vec::new(); @@ -765,14 +778,14 @@ fn check_crate_inner(cm: &CodeMap, span_handler: &SpanHandler, pub fn check_crate_macros(cm: &CodeMap, span_handler: &SpanHandler, krate: &ast::Crate) -> Features { - check_crate_inner(cm, span_handler, krate, + check_crate_inner(cm, span_handler, krate, &[] as &'static [_], |ctx, krate| visit::walk_crate(&mut MacroVisitor { context: ctx }, krate)) } -pub fn check_crate(cm: &CodeMap, span_handler: &SpanHandler, krate: &ast::Crate) - -> Features +pub fn check_crate(cm: &CodeMap, span_handler: &SpanHandler, krate: &ast::Crate, + plugin_attributes: &[(String, AttributeType)]) -> Features { - check_crate_inner(cm, span_handler, krate, + check_crate_inner(cm, span_handler, krate, plugin_attributes, |ctx, krate| visit::walk_crate(&mut PostExpansionVisitor { context: ctx }, krate)) } From d46eef9cf3bf2f1a4ca5448c7f02e3c749e73bd9 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Thu, 7 May 2015 14:58:36 +0530 Subject: [PATCH 2/4] test for register_attribute() --- src/test/auxiliary/attr_plugin_test.rs | 30 +++++++++++++++++++ .../plugin-attr-register-deny.rs | 30 +++++++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 src/test/auxiliary/attr_plugin_test.rs create mode 100644 src/test/compile-fail-fulldeps/plugin-attr-register-deny.rs diff --git a/src/test/auxiliary/attr_plugin_test.rs b/src/test/auxiliary/attr_plugin_test.rs new file mode 100644 index 00000000000..a6cae743ceb --- /dev/null +++ b/src/test/auxiliary/attr_plugin_test.rs @@ -0,0 +1,30 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// force-host + +#![feature(plugin_registrar)] +#![feature(rustc_private)] + +extern crate syntax; + +extern crate rustc; + +use syntax::feature_gate::AttributeType; +use rustc::plugin::Registry; + + + +#[plugin_registrar] +pub fn plugin_registrar(reg: &mut Registry) { + reg.register_attribute("foo".to_owned(), AttributeType::Normal); + reg.register_attribute("bar".to_owned(), AttributeType::CrateLevel); + reg.register_attribute("baz".to_owned(), AttributeType::Whitelisted); +} diff --git a/src/test/compile-fail-fulldeps/plugin-attr-register-deny.rs b/src/test/compile-fail-fulldeps/plugin-attr-register-deny.rs new file mode 100644 index 00000000000..0d2a5a30c10 --- /dev/null +++ b/src/test/compile-fail-fulldeps/plugin-attr-register-deny.rs @@ -0,0 +1,30 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// aux-build:attr_plugin_test.rs +// ignore-stage1 + +#![feature(plugin)] +#![plugin(attr_plugin_test)] +#![deny(unused_attributes)] + +#[baz] +fn baz() { } // no error + +#[foo] +pub fn main() { + //~^^ ERROR unused + #[bar] + fn inner() {} + //~^^ ERROR crate + //~^^^ ERROR unused + baz(); + inner(); +} From 96e1cf3b0693e5724797abd566bd4913be4f6d79 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Thu, 7 May 2015 15:31:20 +0530 Subject: [PATCH 3/4] address review comments --- src/librustc/plugin/registry.rs | 13 +++++-------- src/librustc_lint/builtin.rs | 16 +++++++++++----- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/librustc/plugin/registry.rs b/src/librustc/plugin/registry.rs index c85b30c811c..d532ad38d04 100644 --- a/src/librustc/plugin/registry.rs +++ b/src/librustc/plugin/registry.rs @@ -139,17 +139,14 @@ impl<'a> Registry<'a> { /// Register an attribute with an attribute type /// - /// Registered attributes will bypass the `custom_attribute` feature gate - /// + /// Registered attributes will bypass the `custom_attribute` feature gate. /// `Whitelisted` attributes will additionally not trigger the `unused_attribute` - /// lint - /// - /// `CrateLevel` attributes will not be allowed on anything other than a crate + /// lint. `CrateLevel` attributes will not be allowed on anything other than a crate. pub fn register_attribute(&mut self, name: String, ty: AttributeType) { if let AttributeType::Gated(..) = ty { - self.sess.err("plugin tried to register a gated attribute. \ - Only `Normal`, `Whitelisted`, and `CrateLevel` \ - attributes are allowed"); + self.sess.span_err(self.krate_span, "plugin tried to register a gated \ + attribute. Only `Normal`, `Whitelisted`, \ + and `CrateLevel` attributes are allowed"); } self.attributes.push((name, ty)); } diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 9242274a7a3..27c70213151 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -653,11 +653,17 @@ impl LintPass for UnusedAttributes { if !attr::is_used(attr) { cx.span_lint(UNUSED_ATTRIBUTES, attr.span, "unused attribute"); - if KNOWN_ATTRIBUTES.contains(&(&attr.name(), AttributeType::CrateLevel)) || - plugin_attributes.iter() - .find(|&&(ref x, t)| &*attr.name() == &*x && - AttributeType::CrateLevel == t) - .is_some() { + // Is it a builtin attribute that must be used at the crate level? + let known_crate = KNOWN_ATTRIBUTES.contains(&(&attr.name(), + AttributeType::CrateLevel)); + // Has a plugin registered this attribute as one which must be used at + // the crate level? + let plugin_crate = plugin_attributes.iter() + .find(|&&(ref x, t)| { + &*attr.name() == &*x && + AttributeType::CrateLevel == t + }).is_some(); + if known_crate || plugin_crate { let msg = match attr.node.style { ast::AttrOuter => "crate-level attribute should be an inner \ attribute: add an exclamation mark: #![foo]", From 22b720a920612211d83f7176d7cf2f184c74d294 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 13 May 2015 12:23:43 +0530 Subject: [PATCH 4/4] address more review comments --- src/librustc/plugin/registry.rs | 2 +- src/librustc_lint/builtin.rs | 7 ++----- src/libsyntax/feature_gate.rs | 7 ++++++- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/librustc/plugin/registry.rs b/src/librustc/plugin/registry.rs index d532ad38d04..8fc032572c3 100644 --- a/src/librustc/plugin/registry.rs +++ b/src/librustc/plugin/registry.rs @@ -137,7 +137,7 @@ impl<'a> Registry<'a> { } - /// Register an attribute with an attribute type + /// Register an attribute with an attribute type. /// /// Registered attributes will bypass the `custom_attribute` feature gate. /// `Whitelisted` attributes will additionally not trigger the `unused_attribute` diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 27c70213151..8d8fbd5bf9a 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -643,11 +643,8 @@ impl LintPass for UnusedAttributes { let plugin_attributes = cx.sess().plugin_attributes.borrow_mut(); for &(ref name, ty) in plugin_attributes.iter() { - match ty { - AttributeType::Whitelisted if attr.check_name(&*name) => { - break; - }, - _ => () + if ty == AttributeType::Whitelisted && attr.check_name(&*name) { + break; } } diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index abb4ce15996..495f152d315 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -389,6 +389,8 @@ impl<'a> Context<'a> { for &(ref n, ref ty) in self.plugin_attributes.iter() { if &*n == name { // 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, {:?}", name, ty); return; } @@ -403,7 +405,10 @@ impl<'a> Context<'a> { "attributes of the form `#[derive_*]` are reserved \ for the compiler"); } else { - // Only do the custom attribute lint post-expansion + // Only run the custom attribute lint during regular + // feature gate checking. Macro gating runs + // before the plugin attributes are registered + // so we skip this then if !is_macro { self.gate_feature("custom_attribute", attr.span, &format!("The attribute `{}` is currently \