Skip to content

Cleanup syntax::attr #63146

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

Merged
merged 8 commits into from
Aug 3, 2019
Merged
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
8 changes: 0 additions & 8 deletions src/doc/unstable-book/src/language-features/plugin.md
Original file line number Diff line number Diff line change
@@ -59,7 +59,6 @@ extern crate rustc_plugin;
use syntax::parse::token::{self, Token};
use syntax::tokenstream::TokenTree;
use syntax::ext::base::{ExtCtxt, MacResult, DummyResult, MacEager};
use syntax::ext::build::AstBuilder; // A trait for expr_usize.
use syntax_pos::Span;
use rustc_plugin::Registry;

@@ -164,13 +163,6 @@ can continue and find further errors.
To print syntax fragments for debugging, you can use `span_note` together with
`syntax::print::pprust::*_to_string`.

The example above produced an integer literal using `AstBuilder::expr_usize`.
As an alternative to the `AstBuilder` trait, `libsyntax` provides a set of
quasiquote macros. They are undocumented and very rough around the edges.
However, the implementation may be a good starting point for an improved
quasiquote as an ordinary plugin library.


# Lint plugins

Plugins can extend [Rust's lint
2 changes: 1 addition & 1 deletion src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
@@ -5168,7 +5168,7 @@ impl<'a> LoweringContext<'a> {
let uc_nested = attr::mk_nested_word_item(uc_ident);
attr::mk_list_item(e.span, allow_ident, vec![uc_nested])
};
attr::mk_spanned_attr_outer(e.span, attr::mk_attr_id(), allow)
attr::mk_attr_outer(allow)
};
let attrs = vec![attr];

9 changes: 1 addition & 8 deletions src/librustc_metadata/decoder.rs
Original file line number Diff line number Diff line change
@@ -980,14 +980,7 @@ impl<'a, 'tcx> CrateMetadata {
}

fn get_attributes(&self, item: &Entry<'tcx>, sess: &Session) -> Vec<ast::Attribute> {
item.attributes
.decode((self, sess))
.map(|mut attr| {
// Need new unique IDs: old thread-local IDs won't map to new threads.
attr.id = attr::mk_attr_id();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is due to the related change which implement Decodable/Encodable on AttrId via mk_attr_id and encode_nil. I'm not sure if they're actually equivalent, though. It feels like they should be.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the only place where attributes are decoded?
Perhaps incremental could decode them in the old way as an index, or something like that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT, this is the only place where attributes are decoded, but I'm not sure how to check (cc @eddyb?)

I'm not sure what you mean by "incremental could decode them in the old way" -- is that referring to a place where it'd be good to check whether we're doing the right decoding?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT, this is the only place where attributes are decoded

Yeah, looks like that, at least from searching .decode( and ::decode(.
I thought about maybe commenting out the impl and trying to build, but looks like that's not feasible because everything including attributes derives RustcEncodable and RustcDecodable.
(By incremental I meant some place in incremental compilation that decodes things from cache.)

attr
})
.collect()
item.attributes.decode((self, sess)).collect()
}

// Translate a DefId from the current compilation environment to a DefId
16 changes: 13 additions & 3 deletions src/libsyntax/ast.rs
Original file line number Diff line number Diff line change
@@ -2104,9 +2104,7 @@ pub enum AttrStyle {
Inner,
}

#[derive(
Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, PartialOrd, Ord, Copy,
)]
#[derive(Clone, PartialEq, Eq, Hash, Debug, PartialOrd, Ord, Copy)]
pub struct AttrId(pub usize);

impl Idx for AttrId {
@@ -2118,6 +2116,18 @@ impl Idx for AttrId {
}
}

impl rustc_serialize::Encodable for AttrId {
fn encode<S: Encoder>(&self, s: &mut S) -> Result<(), S::Error> {
s.emit_unit()
}
}

impl rustc_serialize::Decodable for AttrId {
fn decode<D: Decoder>(d: &mut D) -> Result<AttrId, D::Error> {
d.read_nil().map(|_| crate::attr::mk_attr_id())
}
}

/// Metadata associated with an item.
/// Doc-comments are promoted to attributes that have `is_sugared_doc = true`.
#[derive(Clone, RustcEncodable, RustcDecodable, Debug)]
3 changes: 1 addition & 2 deletions src/libsyntax/attr/builtin.rs
Original file line number Diff line number Diff line change
@@ -3,7 +3,6 @@
use crate::ast::{self, Attribute, MetaItem, NestedMetaItem};
use crate::early_buffered_lints::BufferedEarlyLintId;
use crate::ext::base::ExtCtxt;
use crate::ext::build::AstBuilder;
use crate::feature_gate::{Features, GatedCfg};
use crate::parse::ParseSess;

@@ -929,7 +928,7 @@ pub fn find_transparency(
pub fn check_builtin_macro_attribute(ecx: &ExtCtxt<'_>, meta_item: &MetaItem, name: Symbol) {
// All the built-in macro attributes are "words" at the moment.
let template = AttributeTemplate { word: true, list: None, name_value_str: None };
let attr = ecx.attribute(meta_item.span, meta_item.clone());
let attr = ecx.attribute(meta_item.clone());
check_builtin_attribute(ecx.parse_sess, &attr, name, template);
}

44 changes: 18 additions & 26 deletions src/libsyntax/attr/mod.rs
Original file line number Diff line number Diff line change
@@ -6,9 +6,10 @@ pub use builtin::*;
pub use IntType::*;
pub use ReprAttr::*;
pub use StabilityLevel::*;
pub use crate::ast::Attribute;

use crate::ast;
use crate::ast::{AttrId, Attribute, AttrStyle, Name, Ident, Path, PathSegment};
use crate::ast::{AttrId, AttrStyle, Name, Ident, Path, PathSegment};
use crate::ast::{MetaItem, MetaItemKind, NestedMetaItem};
use crate::ast::{Lit, LitKind, Expr, Item, Local, Stmt, StmtKind, GenericParam};
use crate::mut_visit::visit_clobber;
@@ -328,13 +329,14 @@ impl Attribute {
let meta = mk_name_value_item_str(
Ident::with_empty_ctxt(sym::doc),
dummy_spanned(Symbol::intern(&strip_doc_comment_decoration(&comment.as_str()))));
let mut attr = if self.style == ast::AttrStyle::Outer {
mk_attr_outer(self.span, self.id, meta)
} else {
mk_attr_inner(self.span, self.id, meta)
};
attr.is_sugared_doc = true;
f(&attr)
f(&Attribute {
id: self.id,
style: self.style,
path: meta.path,
tokens: meta.node.tokens(meta.span),
is_sugared_doc: true,
span: self.span,
})
} else {
f(self)
}
@@ -376,46 +378,36 @@ pub fn mk_attr_id() -> AttrId {
AttrId(id)
}

/// Returns an inner attribute with the given value.
pub fn mk_attr_inner(span: Span, id: AttrId, item: MetaItem) -> Attribute {
mk_spanned_attr_inner(span, id, item)
}

/// Returns an inner attribute with the given value and span.
pub fn mk_spanned_attr_inner(sp: Span, id: AttrId, item: MetaItem) -> Attribute {
pub fn mk_attr_inner(item: MetaItem) -> Attribute {
Attribute {
id,
id: mk_attr_id(),
style: ast::AttrStyle::Inner,
path: item.path,
tokens: item.node.tokens(item.span),
is_sugared_doc: false,
span: sp,
span: item.span,
}
}

/// Returns an outer attribute with the given value.
pub fn mk_attr_outer(span: Span, id: AttrId, item: MetaItem) -> Attribute {
mk_spanned_attr_outer(span, id, item)
}

/// Returns an outer attribute with the given value and span.
pub fn mk_spanned_attr_outer(sp: Span, id: AttrId, item: MetaItem) -> Attribute {
pub fn mk_attr_outer(item: MetaItem) -> Attribute {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inner/outer are identical save for style -- consider a helper function that takes in the style.

Attribute {
id,
id: mk_attr_id(),
style: ast::AttrStyle::Outer,
path: item.path,
tokens: item.node.tokens(item.span),
is_sugared_doc: false,
span: sp,
span: item.span,
}
}

pub fn mk_sugared_doc_attr(id: AttrId, text: Symbol, span: Span) -> Attribute {
pub fn mk_sugared_doc_attr(text: Symbol, span: Span) -> Attribute {
let style = doc_comment_style(&text.as_str());
let lit_kind = LitKind::Str(text, ast::StrStyle::Cooked);
let lit = Lit::from_lit_kind(lit_kind, span);
Attribute {
id,
id: mk_attr_id(),
style,
path: Path::from_ident(Ident::with_empty_ctxt(sym::doc).with_span_pos(span)),
tokens: MetaItemKind::NameValue(lit).tokens(span),
1 change: 0 additions & 1 deletion src/libsyntax/diagnostics/plugin.rs
Original file line number Diff line number Diff line change
@@ -4,7 +4,6 @@ use std::env;
use crate::ast::{self, Ident, Name};
use crate::source_map;
use crate::ext::base::{ExtCtxt, MacEager, MacResult};
use crate::ext::build::AstBuilder;
use crate::parse::token::{self, Token};
use crate::ptr::P;
use crate::symbol::kw;
Loading