Skip to content

Commit

Permalink
Auto merge of #66070 - petrochenkov:regattr, r=matthewjasper
Browse files Browse the repository at this point in the history
Support registering inert attributes and attribute tools using crate-level attributes

And remove `#[feature(custom_attribute)]`.
(`rustc_plugin::Registry::register_attribute` is not removed yet, I'll do it in a follow up PR.)

```rust
#![register_attr(my_attr)]
#![register_tool(my_tool)]

#[my_attr] // OK
#[my_tool::anything] // OK
fn main() {}
```

---
Some tools (`rustfmt` and `clippy`) used in tool attributes are hardcoded in the compiler.
We need some way to introduce them without hardcoding as well.

This PR introduces a way to do it with a crate level attribute.
The previous attempt to introduce them through command line (#57921) met some resistance.

This probably needs to go through an RFC before stabilization.
However, I'd prefer to land *this* PR without an RFC to able to remove `#[feature(custom_attribute)]` and `Registry::register_attribute` while also providing a replacement.

---
`register_attr` is a direct replacement for `#![feature(custom_attribute)]` (#29642), except it doesn't rely on implicit fallback from unresolved attributes to custom attributes (which was always hacky and is the primary reason for the removal of `custom_attribute`) and requires registering the attribute explicitly.
It's not clear whether it should go through stabilization or not.
It's quite possible that all the uses should migrate to `#![register_tool]` (#66079) instead.

---

Details:
- The naming is `register_attr`/`register_tool` rather than some `register_attributes` (plural, no abbreviation) for consistency with already existing attributes like `cfg_attr`, or `feature`, etc.
---
Previous attempt: #57921
cc #44690
Tracking issues: #66079 (`register_tool`), #66080 (`register_attr`)
Closes #29642
  • Loading branch information
bors committed Nov 10, 2019
2 parents a3b6e57 + 83f553c commit 3fc30d8
Show file tree
Hide file tree
Showing 33 changed files with 411 additions and 118 deletions.
23 changes: 20 additions & 3 deletions src/librustc/hir/def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ pub enum NonMacroAttrKind {
Tool,
/// Single-segment custom attribute registered by a derive macro (`#[serde(default)]`).
DeriveHelper,
/// Single-segment custom attribute registered with `#[register_attr]`.
Registered,
/// Single-segment custom attribute registered by a legacy plugin (`register_attribute`).
LegacyPluginHelper,
/// Single-segment custom attribute not registered in any way (`#[my_attr]`).
Custom,
}

#[derive(Clone, Copy, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, HashStable)]
Expand Down Expand Up @@ -329,8 +329,24 @@ impl NonMacroAttrKind {
NonMacroAttrKind::Builtin => "built-in attribute",
NonMacroAttrKind::Tool => "tool attribute",
NonMacroAttrKind::DeriveHelper => "derive helper attribute",
NonMacroAttrKind::Registered => "explicitly registered attribute",
NonMacroAttrKind::LegacyPluginHelper => "legacy plugin helper attribute",
NonMacroAttrKind::Custom => "custom attribute",
}
}

pub fn article(self) -> &'static str {
match self {
NonMacroAttrKind::Registered => "an",
_ => "a",
}
}

/// Users of some attributes cannot mark them as used, so they are considered always used.
pub fn is_used(self) -> bool {
match self {
NonMacroAttrKind::Tool | NonMacroAttrKind::DeriveHelper => true,
NonMacroAttrKind::Builtin | NonMacroAttrKind::Registered |
NonMacroAttrKind::LegacyPluginHelper => false,
}
}
}
Expand Down Expand Up @@ -389,6 +405,7 @@ impl<Id> Res<Id> {
pub fn article(&self) -> &'static str {
match *self {
Res::Def(kind, _) => kind.article(),
Res::NonMacroAttr(kind) => kind.article(),
Res::Err => "an",
_ => "a",
}
Expand Down
1 change: 0 additions & 1 deletion src/librustc_plugin/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ impl<'a> Registry<'a> {

/// 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: Symbol, ty: AttributeType) {
Expand Down
3 changes: 1 addition & 2 deletions src/librustc_resolve/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,7 @@ impl<'a> Resolver<'a> {
crate fn get_macro(&mut self, res: Res) -> Option<Lrc<SyntaxExtension>> {
match res {
Res::Def(DefKind::Macro(..), def_id) => self.get_macro_by_def_id(def_id),
Res::NonMacroAttr(attr_kind) =>
Some(self.non_macro_attr(attr_kind == NonMacroAttrKind::Tool)),
Res::NonMacroAttr(attr_kind) => Some(self.non_macro_attr(attr_kind.is_used())),
_ => None,
}
}
Expand Down
14 changes: 11 additions & 3 deletions src/librustc_resolve/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use syntax_pos::hygiene::MacroKind;
use syntax_pos::{BytePos, Span, MultiSpan};

use crate::resolve_imports::{ImportDirective, ImportDirectiveSubclass, ImportResolver};
use crate::{path_names_to_string, KNOWN_TOOLS};
use crate::path_names_to_string;
use crate::{BindingError, CrateLint, HasGenericParams, LegacyScope, Module, ModuleOrUniformRoot};
use crate::{PathResult, ParentScope, ResolutionError, Resolver, Scope, ScopeSet, Segment};

Expand Down Expand Up @@ -400,6 +400,14 @@ impl<'a> Resolver<'a> {
Scope::Module(module) => {
this.add_module_candidates(module, &mut suggestions, filter_fn);
}
Scope::RegisteredAttrs => {
let res = Res::NonMacroAttr(NonMacroAttrKind::Registered);
if filter_fn(res) {
suggestions.extend(this.registered_attrs.iter().map(|ident| {
TypoSuggestion::from_res(ident.name, res)
}));
}
}
Scope::MacroUsePrelude => {
suggestions.extend(this.macro_use_prelude.iter().filter_map(|(name, binding)| {
let res = binding.res();
Expand Down Expand Up @@ -439,8 +447,8 @@ impl<'a> Resolver<'a> {
}
Scope::ToolPrelude => {
let res = Res::NonMacroAttr(NonMacroAttrKind::Tool);
suggestions.extend(KNOWN_TOOLS.iter().map(|name| {
TypoSuggestion::from_res(*name, res)
suggestions.extend(this.registered_tools.iter().map(|ident| {
TypoSuggestion::from_res(ident.name, res)
}));
}
Scope::StdLibPrelude => {
Expand Down
27 changes: 16 additions & 11 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ mod check_unused;
mod build_reduced_graph;
mod resolve_imports;

const KNOWN_TOOLS: &[Name] = &[sym::clippy, sym::rustfmt];

enum Weak {
Yes,
No,
Expand All @@ -102,6 +100,7 @@ enum Scope<'a> {
MacroRules(LegacyScope<'a>),
CrateRoot,
Module(Module<'a>),
RegisteredAttrs,
MacroUsePrelude,
BuiltinAttrs,
LegacyPluginHelpers,
Expand Down Expand Up @@ -621,7 +620,6 @@ enum AmbiguityKind {
Import,
BuiltinAttr,
DeriveHelper,
LegacyHelperVsPrelude,
LegacyVsModern,
GlobVsOuter,
GlobVsGlob,
Expand All @@ -638,8 +636,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 =>
Expand Down Expand Up @@ -916,6 +912,8 @@ pub struct Resolver<'a> {
crate_loader: CrateLoader<'a>,
macro_names: FxHashSet<Ident>,
builtin_macros: FxHashMap<Name, SyntaxExtension>,
registered_attrs: FxHashSet<Ident>,
registered_tools: FxHashSet<Ident>,
macro_use_prelude: FxHashMap<Name, &'a NameBinding<'a>>,
all_macros: FxHashMap<Name, Res>,
macro_map: FxHashMap<DefId, Lrc<SyntaxExtension>>,
Expand Down Expand Up @@ -1138,6 +1136,9 @@ impl<'a> Resolver<'a> {
}
}

let (registered_attrs, registered_tools) =
macros::registered_attrs_and_tools(session, &krate.attrs);

let mut invocation_parent_scopes = FxHashMap::default();
invocation_parent_scopes.insert(ExpnId::root(), ParentScope::module(graph_root));

Expand Down Expand Up @@ -1207,6 +1208,8 @@ impl<'a> Resolver<'a> {
crate_loader: CrateLoader::new(session, metadata_loader, crate_name),
macro_names: FxHashSet::default(),
builtin_macros: Default::default(),
registered_attrs,
registered_tools,
macro_use_prelude: FxHashMap::default(),
all_macros: FxHashMap::default(),
macro_map: FxHashMap::default(),
Expand Down Expand Up @@ -1484,6 +1487,7 @@ impl<'a> Resolver<'a> {
Scope::MacroRules(..) => true,
Scope::CrateRoot => true,
Scope::Module(..) => true,
Scope::RegisteredAttrs => use_prelude,
Scope::MacroUsePrelude => use_prelude || rust_2015,
Scope::BuiltinAttrs => true,
Scope::LegacyPluginHelpers => use_prelude || rust_2015,
Expand Down Expand Up @@ -1528,11 +1532,12 @@ impl<'a> Resolver<'a> {
match ns {
TypeNS => Scope::ExternPrelude,
ValueNS => Scope::StdLibPrelude,
MacroNS => Scope::MacroUsePrelude,
MacroNS => Scope::RegisteredAttrs,
}
}
}
}
Scope::RegisteredAttrs => Scope::MacroUsePrelude,
Scope::MacroUsePrelude => Scope::StdLibPrelude,
Scope::BuiltinAttrs => Scope::LegacyPluginHelpers,
Scope::LegacyPluginHelpers => break, // nowhere else to search
Expand Down Expand Up @@ -1688,11 +1693,11 @@ impl<'a> Resolver<'a> {
if let Some(binding) = self.extern_prelude_get(ident, !record_used) {
return Some(LexicalScopeBinding::Item(binding));
}
}
if ns == TypeNS && KNOWN_TOOLS.contains(&ident.name) {
let binding = (Res::ToolMod, ty::Visibility::Public,
DUMMY_SP, ExpnId::root()).to_name_binding(self.arenas);
return Some(LexicalScopeBinding::Item(binding));
if let Some(ident) = self.registered_tools.get(&ident) {
let binding = (Res::ToolMod, ty::Visibility::Public,
ident.span, ExpnId::root()).to_name_binding(self.arenas);
return Some(LexicalScopeBinding::Item(binding));
}
}
if let Some(prelude) = self.prelude {
if let Ok(binding) = self.resolve_ident_in_module_unadjusted(
Expand Down
Loading

0 comments on commit 3fc30d8

Please sign in to comment.