-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Cleanup syntax::attr #63146
Merged
Merged
Cleanup syntax::attr #63146
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
804f0f3
Unify spanned and non-spanned Attribute ctors
Mark-Simulacrum 0a42bad
Remove AttrId from Attribute constructors
Mark-Simulacrum b2c5065
Remove Span argument from ExtCtxt::attribute
Mark-Simulacrum f78bf50
Remove span argument from mk_attr_{inner,outer}
Mark-Simulacrum c9bd4a0
Replace a few Attribute constructors with mk_attr
Mark-Simulacrum 0f98581
Replace AstBuilder with inherent methods
Mark-Simulacrum d4227f6
Use Ident::new over setting span position via builder
Mark-Simulacrum c146344
Decode AttrId via mk_attr_id
Mark-Simulacrum File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 { | ||
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. 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), | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This is due to the related change which implement Decodable/Encodable on
AttrId
viamk_attr_id
andencode_nil
. I'm not sure if they're actually equivalent, though. It feels like they should be.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.
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?
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.
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?
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.
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
andRustcDecodable
.(By incremental I meant some place in incremental compilation that decodes things from cache.)