-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Add support for registering attributes with rustc in plugins #25168
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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<String>, | ||
|
||
#[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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This and the next two lines can be one paragraph. Also, all these sentences need |
||
/// | ||
/// `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. \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be a |
||
Only `Normal`, `Whitelisted`, and `CrateLevel` \ | ||
attributes are allowed"); | ||
} | ||
self.attributes.push((name, ty)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could just be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we exceed the line limit here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The limit is 100, this will be around 80. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, can't this be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
}, | ||
_ => () | ||
} | ||
} | ||
|
||
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)) || | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now that this is multiple lines, could you break (parts of) it out into separate variables? |
||
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]", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't it more that plugins register "known" attributes, i.e. they no longer count as custom? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code does feature gating of specific attrs; even if custom_attribute is on, attributes like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code below doesn't look at There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, so the comment is just contrasting to the behaviour of the code above. Makes sense. Could you still point out that the loop is still necessary to short-circuit to avoid the checks below. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
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<F>(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<F>(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<F>(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)) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 <LICENSE-APACHE or | ||
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license | ||
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, what's the point of registering a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, it gets an unused attribute warning, but it doesn't get the custom_attribute feature gate. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep. |
||
reg.register_attribute("bar".to_owned(), AttributeType::CrateLevel); | ||
reg.register_attribute("baz".to_owned(), AttributeType::Whitelisted); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 <LICENSE-APACHE or | ||
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license | ||
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs a .