Skip to content
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

Support registering tool attributes from command line and un-support Registry::register_attribute #57921

Closed
wants to merge 2 commits into from
Closed
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
3 changes: 0 additions & 3 deletions src/librustc/hir/def.rs
Original file line number Diff line number Diff line change
@@ -26,8 +26,6 @@ pub enum NonMacroAttrKind {
Tool,
/// Single-segment custom attribute registered by a derive macro (`#[serde(default)]`).
DeriveHelper,
/// Single-segment custom attribute registered by a legacy plugin (`register_attribute`).
LegacyPluginHelper,
/// Single-segment custom attribute not registered in any way (`#[my_attr]`).
Custom,
}
@@ -245,7 +243,6 @@ impl NonMacroAttrKind {
NonMacroAttrKind::Builtin => "built-in attribute",
NonMacroAttrKind::Tool => "tool attribute",
NonMacroAttrKind::DeriveHelper => "derive helper attribute",
NonMacroAttrKind::LegacyPluginHelper => "legacy plugin helper attribute",
NonMacroAttrKind::Custom => "custom attribute",
}
}
1 change: 0 additions & 1 deletion src/librustc/ich/impls_hir.rs
Original file line number Diff line number Diff line change
@@ -1016,7 +1016,6 @@ impl_stable_hash_for!(enum hir::def::NonMacroAttrKind {
Builtin,
Tool,
DeriveHelper,
LegacyPluginHelper,
Custom,
});

2 changes: 2 additions & 0 deletions src/librustc/session/config.rs
Original file line number Diff line number Diff line change
@@ -1399,6 +1399,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
merge_functions: Option<MergeFunctions> = (None, parse_merge_functions, [TRACKED],
"control the operation of the MergeFunctions LLVM pass, taking
the same values as the target option of the same name"),
attr_tool: Vec<String> = (Vec::new(), parse_string_push, [TRACKED],
"add a tool name for resolving tool attributes `#[tool::anything]`"),
}

pub fn default_lib_output() -> CrateType {
4 changes: 1 addition & 3 deletions src/librustc/session/mod.rs
Original file line number Diff line number Diff line change
@@ -25,7 +25,7 @@ use errors::{self, DiagnosticBuilder, DiagnosticId, Applicability};
use errors::emitter::{Emitter, EmitterWriter};
use syntax::ast::{self, NodeId};
use syntax::edition::Edition;
use syntax::feature_gate::{self, AttributeType};
use syntax::feature_gate;
use syntax::json::JsonEmitter;
use syntax::source_map;
use syntax::parse::{self, ParseSess};
@@ -85,7 +85,6 @@ pub struct Session {
/// in order to avoid redundantly verbose output (Issue #24690, #44953).
pub one_time_diagnostics: Lock<FxHashSet<(DiagnosticMessageId, Option<Span>, String)>>,
pub plugin_llvm_passes: OneThread<RefCell<Vec<String>>>,
pub plugin_attributes: OneThread<RefCell<Vec<(String, AttributeType)>>>,
pub crate_types: Once<Vec<config::CrateType>>,
pub dependency_formats: Once<dependency_format::Dependencies>,
/// The crate_disambiguator is constructed out of all the `-C metadata`
@@ -1178,7 +1177,6 @@ pub fn build_session_(
buffered_lints: Lock::new(Some(Default::default())),
one_time_diagnostics: Default::default(),
plugin_llvm_passes: OneThread::new(RefCell::new(Vec::new())),
plugin_attributes: OneThread::new(RefCell::new(Vec::new())),
crate_types: Once::new(),
dependency_formats: Once::new(),
crate_disambiguator: Once::new(),
3 changes: 0 additions & 3 deletions src/librustc_driver/driver.rs
Original file line number Diff line number Diff line change
@@ -877,7 +877,6 @@ where
late_lint_passes,
lint_groups,
llvm_passes,
attributes,
..
} = registry;

@@ -895,7 +894,6 @@ where
}

*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.
@@ -1079,7 +1077,6 @@ where
&krate,
&sess.parse_sess,
&sess.features_untracked(),
&attributes,
sess.opts.unstable_features,
);
});
21 changes: 3 additions & 18 deletions src/librustc_lint/unused.rs
Original file line number Diff line number Diff line change
@@ -254,29 +254,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedAttributes {
}
}

let plugin_attributes = cx.sess().plugin_attributes.borrow_mut();
for &(ref name, ty) in plugin_attributes.iter() {
if ty == AttributeType::Whitelisted && attr.check_name(&name) {
debug!("{:?} (plugin attr) is whitelisted with ty {:?}", name, ty);
break;
}
}

let name = attr.name();
if !attr::is_used(attr) {
debug!("Emitting warning for: {:?}", attr);
cx.span_lint(UNUSED_ATTRIBUTES, attr.span, "unused attribute");
// Is it a builtin attribute that must be used at the crate level?
let known_crate = BUILTIN_ATTRIBUTES.iter()
.find(|&&(builtin, ty, ..)| name == builtin && ty == AttributeType::CrateLevel)
.is_some();

// Has a plugin registered this attribute as one that must be used at
// the crate level?
let plugin_crate = plugin_attributes.iter()
.find(|&&(ref x, t)| name == &**x && AttributeType::CrateLevel == t)
.is_some();
if known_crate || plugin_crate {
if BUILTIN_ATTRIBUTES.iter().any(|(builtin, ty, ..)| {
&name == builtin && ty == &AttributeType::CrateLevel
}) {
let msg = match attr.style {
ast::AttrStyle::Outer => {
"crate-level attribute should be an inner attribute: add an exclamation \
14 changes: 0 additions & 14 deletions src/librustc_plugin/registry.rs
Original file line number Diff line number Diff line change
@@ -9,7 +9,6 @@ use syntax::ext::base::MacroExpanderFn;
use syntax::ext::hygiene;
use syntax::symbol::Symbol;
use syntax::ast;
use syntax::feature_gate::AttributeType;
use syntax_pos::Span;

use std::borrow::ToOwned;
@@ -47,9 +46,6 @@ pub struct Registry<'a> {

#[doc(hidden)]
pub llvm_passes: Vec<String>,

#[doc(hidden)]
pub attributes: Vec<(String, AttributeType)>,
}

impl<'a> Registry<'a> {
@@ -64,7 +60,6 @@ impl<'a> Registry<'a> {
late_lint_passes: vec![],
lint_groups: FxHashMap::default(),
llvm_passes: vec![],
attributes: vec![],
}
}

@@ -163,13 +158,4 @@ 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) {
self.attributes.push((name, ty));
}
}
16 changes: 8 additions & 8 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
@@ -85,10 +85,6 @@ mod check_unused;
mod build_reduced_graph;
mod resolve_imports;

fn is_known_tool(name: Name) -> bool {
["clippy", "rustfmt"].contains(&&*name.as_str())
}

enum Weak {
Yes,
No,
@@ -1246,7 +1242,6 @@ enum AmbiguityKind {
AbsolutePath,
BuiltinAttr,
DeriveHelper,
LegacyHelperVsPrelude,
LegacyVsModern,
GlobVsOuter,
GlobVsGlob,
@@ -1265,8 +1260,6 @@ impl AmbiguityKind {
"built-in attribute vs any other name",
AmbiguityKind::DeriveHelper =>
"derive helper attribute vs any other name",
AmbiguityKind::LegacyHelperVsPrelude =>
"legacy plugin helper attribute vs name from prelude",
AmbiguityKind::LegacyVsModern =>
"`macro_rules` vs non-`macro_rules` from other module",
AmbiguityKind::GlobVsOuter =>
@@ -1477,6 +1470,7 @@ pub struct Resolver<'a> {

prelude: Option<Module<'a>>,
pub extern_prelude: FxHashMap<Ident, ExternPreludeEntry<'a>>,
attr_tools: FxHashSet<Name>,

/// n.b. This is used only for better diagnostics, not name resolution itself.
has_self: FxHashSet<DefId>,
@@ -1823,6 +1817,11 @@ impl<'a> Resolver<'a> {
}
}

let mut attr_tools: FxHashSet<Name> =
session.opts.debugging_opts.attr_tool.iter().map(|tool| Symbol::intern(tool)).collect();
attr_tools.insert(Symbol::intern("rustfmt"));
attr_tools.insert(Symbol::intern("clippy"));

let mut invocations = FxHashMap::default();
invocations.insert(Mark::root(),
arenas.alloc_invocation_data(InvocationData::root(graph_root)));
@@ -1842,6 +1841,7 @@ impl<'a> Resolver<'a> {
graph_root,
prelude: None,
extern_prelude,
attr_tools,

has_self: FxHashSet::default(),
field_names: FxHashMap::default(),
@@ -2119,7 +2119,7 @@ impl<'a> Resolver<'a> {
return Some(LexicalScopeBinding::Item(binding));
}
}
if ns == TypeNS && is_known_tool(ident.name) {
if ns == TypeNS && self.attr_tools.contains(&ident.name) {
let binding = (Def::ToolMod, ty::Visibility::Public,
DUMMY_SP, Mark::root()).to_name_binding(self.arenas);
return Some(LexicalScopeBinding::Item(binding));
30 changes: 3 additions & 27 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use {AmbiguityError, AmbiguityKind, AmbiguityErrorMisc};
use {CrateLint, Resolver, ResolutionError, ScopeSet, Weak};
use {Module, ModuleKind, NameBinding, NameBindingKind, PathResult, Segment, ToNameBinding};
use {is_known_tool, resolve_error};
use resolve_error;
use ModuleOrUniformRoot;
use Namespace::*;
use build_reduced_graph::{BuildReducedGraphVisitor, IsMacroExport};
@@ -451,9 +451,6 @@ impl<'a> Resolver<'a> {
// 4b. Standard library prelude is currently implemented as `macro-use` (closed, controlled)
// 5. Language prelude: builtin macros (closed, controlled, except for legacy plugins).
// 6. Language prelude: builtin attributes (closed, controlled).
// 4-6. Legacy plugin helpers (open, not controlled). Similar to derive helpers,
// but introduced by legacy plugins using `register_attribute`. Priority is somewhere
// in prelude, not sure where exactly (creates ambiguities with any other prelude names).

enum WhereToResolve<'a> {
DeriveHelpers,
@@ -463,7 +460,6 @@ impl<'a> Resolver<'a> {
MacroUsePrelude,
BuiltinMacros,
BuiltinAttrs,
LegacyPluginHelpers,
ExternPrelude,
ToolPrelude,
StdLibPrelude,
@@ -630,18 +626,6 @@ impl<'a> Resolver<'a> {
Err(Determinacy::Determined)
}
}
WhereToResolve::LegacyPluginHelpers => {
if (use_prelude || rust_2015) &&
self.session.plugin_attributes.borrow().iter()
.any(|(name, _)| ident.name == &**name) {
let binding = (Def::NonMacroAttr(NonMacroAttrKind::LegacyPluginHelper),
ty::Visibility::Public, DUMMY_SP, Mark::root())
.to_name_binding(self.arenas);
Ok((binding, Flags::PRELUDE))
} else {
Err(Determinacy::Determined)
}
}
WhereToResolve::ExternPrelude => {
if use_prelude || is_absolute_path {
match self.extern_prelude_get(ident, !record_used) {
@@ -655,7 +639,7 @@ impl<'a> Resolver<'a> {
}
}
WhereToResolve::ToolPrelude => {
if use_prelude && is_known_tool(ident.name) {
if use_prelude && self.attr_tools.contains(&ident.name) {
let binding = (Def::ToolMod, ty::Visibility::Public,
DUMMY_SP, Mark::root()).to_name_binding(self.arenas);
Ok((binding, Flags::PRELUDE))
@@ -704,8 +688,6 @@ impl<'a> Resolver<'a> {
if def != innermost_def {
let builtin = Def::NonMacroAttr(NonMacroAttrKind::Builtin);
let derive_helper = Def::NonMacroAttr(NonMacroAttrKind::DeriveHelper);
let legacy_helper =
Def::NonMacroAttr(NonMacroAttrKind::LegacyPluginHelper);

let ambiguity_error_kind = if is_import {
Some(AmbiguityKind::Import)
@@ -715,11 +697,6 @@ impl<'a> Resolver<'a> {
Some(AmbiguityKind::BuiltinAttr)
} else if innermost_def == derive_helper || def == derive_helper {
Some(AmbiguityKind::DeriveHelper)
} else if innermost_def == legacy_helper &&
flags.contains(Flags::PRELUDE) ||
def == legacy_helper &&
innermost_flags.contains(Flags::PRELUDE) {
Some(AmbiguityKind::LegacyHelperVsPrelude)
} else if innermost_flags.contains(Flags::MACRO_RULES) &&
flags.contains(Flags::MODULE) &&
!self.disambiguate_legacy_vs_modern(innermost_binding,
@@ -807,8 +784,7 @@ impl<'a> Resolver<'a> {
}
WhereToResolve::MacroUsePrelude => WhereToResolve::BuiltinMacros,
WhereToResolve::BuiltinMacros => WhereToResolve::BuiltinAttrs,
WhereToResolve::BuiltinAttrs => WhereToResolve::LegacyPluginHelpers,
WhereToResolve::LegacyPluginHelpers => break, // nowhere else to search
WhereToResolve::BuiltinAttrs => break, // nowhere else to search
WhereToResolve::ExternPrelude if is_absolute_path => break,
WhereToResolve::ExternPrelude => WhereToResolve::ToolPrelude,
WhereToResolve::ToolPrelude => WhereToResolve::StdLibPrelude,
26 changes: 5 additions & 21 deletions src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
@@ -1286,7 +1286,6 @@ impl GatedCfg {
struct Context<'a> {
features: &'a Features,
parse_sess: &'a ParseSess,
plugin_attributes: &'a [(String, AttributeType)],
}

macro_rules! gate_feature_fn {
@@ -1334,15 +1333,7 @@ impl<'a> Context<'a> {
return;
}
}
for &(ref n, ref 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 !attr::is_known(attr) {
if name.starts_with("rustc_") {
let msg = "unless otherwise specified, attributes with the prefix `rustc_` \
@@ -1361,8 +1352,7 @@ impl<'a> Context<'a> {
}

pub fn check_attribute(attr: &ast::Attribute, parse_sess: &ParseSess, features: &Features) {
let cx = Context { features: features, parse_sess: parse_sess, plugin_attributes: &[] };
cx.check_attribute(attr, true);
Context { features, parse_sess }.check_attribute(attr, true);
}

fn find_lang_feature_issue(feature: &str) -> Option<u32> {
@@ -2101,17 +2091,11 @@ pub fn get_features(span_handler: &Handler, krate_attrs: &[ast::Attribute],
}

pub fn check_crate(krate: &ast::Crate,
sess: &ParseSess,
parse_sess: &ParseSess,
features: &Features,
plugin_attributes: &[(String, 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 ctx = Context { features, parse_sess };
let visitor = &mut PostExpansionVisitor { context: &ctx };
visit::walk_crate(visitor, krate);
}
13 changes: 4 additions & 9 deletions src/test/run-pass-fulldeps/auxiliary/issue-40001-plugin.rs
Original file line number Diff line number Diff line change
@@ -7,23 +7,15 @@ extern crate rustc_plugin;
extern crate syntax;

use rustc_plugin::Registry;
use syntax::attr;
use syntax::ext::base::*;
use syntax::feature_gate::AttributeType::Whitelisted;
use syntax::symbol::Symbol;

use rustc::hir;
use rustc::hir::intravisit;
use rustc::hir::map as hir_map;
use hir::Node;
use rustc::lint::{LateContext, LintPass, LintArray, LateLintPass, LintContext};
use rustc::ty;
use syntax::{ast, source_map};

#[plugin_registrar]
pub fn plugin_registrar(reg: &mut Registry) {
reg.register_late_lint_pass(box MissingWhitelistedAttrPass);
reg.register_attribute("whitelisted_attr".to_string(), Whitelisted);
}

declare_lint!(MISSING_WHITELISTED_ATTR, Deny,
@@ -55,7 +47,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingWhitelistedAttrPass {
_ => cx.tcx.hir().expect_item(cx.tcx.hir().get_parent(id)),
};

if !attr::contains_name(&item.attrs, "whitelisted_attr") {
if item.attrs.iter().all(|attr| {
attr.path.segments.last().map(|seg| seg.ident.name.to_string()) !=
Some("whitelisted_attr".to_owned())
}) {
cx.span_lint(MISSING_WHITELISTED_ATTR, span,
"Missing 'whitelisted_attr' attribute");
}
Loading