-
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 3 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 |
---|---|---|
|
@@ -641,9 +641,29 @@ 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)) { | ||
// 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]", | ||
|
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 .