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

expand: Stop using nonterminals for passing tokens to attribute and derive macros #73345

Merged
merged 3 commits into from
Jul 2, 2020
Merged
Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion src/librustc_ast/attr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ impl MetaItem {
let span = span.with_hi(segments.last().unwrap().ident.span.hi());
Path { span, segments }
}
Some(TokenTree::Token(Token { kind: token::Interpolated(nt, _), .. })) => match *nt {
Some(TokenTree::Token(Token { kind: token::Interpolated(nt), .. })) => match *nt {
token::Nonterminal::NtMeta(ref item) => return item.meta(item.path.span),
token::Nonterminal::NtPath(ref path) => path.clone(),
_ => return None,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_ast/mut_visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,7 @@ pub fn noop_visit_token<T: MutVisitor>(t: &mut Token, vis: &mut T) {
*span = ident.span;
return; // Avoid visiting the span for the second time.
}
token::Interpolated(nt, _) => {
token::Interpolated(nt) => {
let mut nt = Lrc::make_mut(nt);
vis.visit_interpolated(&mut nt);
}
Expand Down
51 changes: 31 additions & 20 deletions src/librustc_ast/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::tokenstream::TokenTree;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_data_structures::sync::Lrc;
use rustc_macros::HashStable_Generic;
use rustc_span::symbol::kw;
use rustc_span::symbol::{kw, sym};
use rustc_span::symbol::{Ident, Symbol};
use rustc_span::{self, Span, DUMMY_SP};
use std::borrow::Cow;
Expand Down Expand Up @@ -182,15 +182,6 @@ fn ident_can_begin_type(name: Symbol, span: Span, is_raw: bool) -> bool {
.contains(&name)
}

/// A hack used to pass AST fragments to attribute and derive macros
/// as a single nonterminal token instead of a token stream.
/// FIXME: It needs to be removed, but there are some compatibility issues (see #73345).
#[derive(Clone, PartialEq, RustcEncodable, RustcDecodable, Debug, HashStable_Generic)]
pub enum FlattenGroup {
Yes,
No,
}

#[derive(Clone, PartialEq, RustcEncodable, RustcDecodable, Debug, HashStable_Generic)]
pub enum TokenKind {
/* Expression-operator symbols. */
Expand Down Expand Up @@ -245,7 +236,7 @@ pub enum TokenKind {
/// treat regular and interpolated lifetime identifiers in the same way.
Lifetime(Symbol),

Interpolated(Lrc<Nonterminal>, FlattenGroup),
Interpolated(Lrc<Nonterminal>),

// Can be expanded into several tokens.
/// A doc comment.
Expand Down Expand Up @@ -352,7 +343,7 @@ impl Token {
/// if they keep spans or perform edition checks.
pub fn uninterpolated_span(&self) -> Span {
match &self.kind {
Interpolated(nt, _) => nt.span(),
Interpolated(nt) => nt.span(),
_ => self.span,
}
}
Expand Down Expand Up @@ -391,7 +382,7 @@ impl Token {
ModSep | // global path
Lifetime(..) | // labeled loop
Pound => true, // expression attributes
Interpolated(ref nt, _) => match **nt {
Interpolated(ref nt) => match **nt {
NtLiteral(..) |
NtExpr(..) |
NtBlock(..) |
Expand All @@ -417,7 +408,7 @@ impl Token {
Lifetime(..) | // lifetime bound in trait object
Lt | BinOp(Shl) | // associated path
ModSep => true, // global path
Interpolated(ref nt, _) => match **nt {
Interpolated(ref nt) => match **nt {
NtTy(..) | NtPath(..) => true,
_ => false,
},
Expand All @@ -429,7 +420,7 @@ impl Token {
pub fn can_begin_const_arg(&self) -> bool {
match self.kind {
OpenDelim(Brace) => true,
Interpolated(ref nt, _) => match **nt {
Interpolated(ref nt) => match **nt {
NtExpr(..) | NtBlock(..) | NtLiteral(..) => true,
_ => false,
},
Expand Down Expand Up @@ -464,7 +455,7 @@ impl Token {
match self.uninterpolate().kind {
Literal(..) | BinOp(Minus) => true,
Ident(name, false) if name.is_bool_lit() => true,
Interpolated(ref nt, _) => match &**nt {
Interpolated(ref nt) => match &**nt {
NtLiteral(_) => true,
NtExpr(e) => match &e.kind {
ast::ExprKind::Lit(_) => true,
Expand All @@ -485,7 +476,7 @@ impl Token {
// otherwise returns the original token.
pub fn uninterpolate(&self) -> Cow<'_, Token> {
match &self.kind {
Interpolated(nt, _) => match **nt {
Interpolated(nt) => match **nt {
NtIdent(ident, is_raw) => {
Cow::Owned(Token::new(Ident(ident.name, is_raw), ident.span))
}
Expand Down Expand Up @@ -532,7 +523,7 @@ impl Token {

/// Returns `true` if the token is an interpolated path.
fn is_path(&self) -> bool {
if let Interpolated(ref nt, _) = self.kind {
if let Interpolated(ref nt) = self.kind {
if let NtPath(..) = **nt {
return true;
}
Expand All @@ -544,7 +535,7 @@ impl Token {
/// That is, is this a pre-parsed expression dropped into the token stream
/// (which happens while parsing the result of macro expansion)?
pub fn is_whole_expr(&self) -> bool {
if let Interpolated(ref nt, _) = self.kind {
if let Interpolated(ref nt) = self.kind {
if let NtExpr(_) | NtLiteral(_) | NtPath(_) | NtIdent(..) | NtBlock(_) = **nt {
return true;
}
Expand All @@ -555,7 +546,7 @@ impl Token {

// Is the token an interpolated block (`$b:block`)?
pub fn is_whole_block(&self) -> bool {
if let Interpolated(ref nt, _) = self.kind {
if let Interpolated(ref nt) = self.kind {
if let NtBlock(..) = **nt {
return true;
}
Expand Down Expand Up @@ -785,6 +776,26 @@ impl Nonterminal {
NtTT(tt) => tt.span(),
}
}

/// This nonterminal looks like some specific enums from
/// `proc-macro-hack` and `procedural-masquerade` crates.
/// We need to maintain some special pretty-printing behavior for them due to incorrect
/// asserts in old versions of those crates and their wide use in the ecosystem.
/// See issue #73345 for more details.
/// FIXME(#73933): Remove this eventually.
pub fn pretty_printing_compatibility_hack(&self) -> bool {
if let NtItem(item) = self {
let name = item.ident.name;
if name == sym::ProceduralMasqueradeDummyType || name == sym::ProcMacroHack {
Copy link
Member

Choose a reason for hiding this comment

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

😅

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add an allow by default lint here? That way we can make it warn by default in 6 months acting as a nudge so that we can outright remove this code in a year.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We go through this code for both old and new versions of these crates, so we cannot discern between good and bad cases.
If we produced a warning, it would just be produced for all uses of proc-macro-hack and procedural-masquerade.

if let ast::ItemKind::Enum(enum_def, _) = &item.kind {
if let [variant] = &*enum_def.variants {
return variant.ident.name == sym::Input;
}
}
}
}
false
}
}

impl PartialEq for Nonterminal {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_ast/util/literal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ impl Lit {
token::Lit::new(token::Bool, name, None)
}
token::Literal(lit) => lit,
token::Interpolated(ref nt, _) => {
token::Interpolated(ref nt) => {
if let token::NtExpr(expr) | token::NtLiteral(expr) = &**nt {
if let ast::ExprKind::Lit(lit) = &expr.kind {
return Ok(lit.clone());
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_ast_lowering/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1027,7 +1027,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {

fn lower_token(&mut self, token: Token) -> TokenStream {
match token.kind {
token::Interpolated(nt, _) => {
token::Interpolated(nt) => {
let tts = (self.nt_to_tokenstream)(&nt, &self.sess.parse_sess, token.span);
self.lower_token_stream(tts)
}
Expand Down
20 changes: 16 additions & 4 deletions src/librustc_ast_pretty/pprust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,14 @@ pub fn to_string(f: impl FnOnce(&mut State<'_>)) -> String {
printer.s.eof()
}

// This makes comma-separated lists look slightly nicer,
// and also addresses a specific regression described in issue #63896.
// This makes printed token streams look slightly nicer,
// and also addresses some specific regressions described in #63896 and #73345.
fn tt_prepend_space(tt: &TokenTree, prev: &TokenTree) -> bool {
if let TokenTree::Token(token) = prev {
if let token::DocComment(s) = token.kind {
return !s.as_str().starts_with("//");
}
}
match tt {
TokenTree::Token(token) => match token.kind {
token::Comma => false,
Expand All @@ -163,7 +168,14 @@ fn tt_prepend_space(tt: &TokenTree, prev: &TokenTree) -> bool {
},
_ => true,
},
_ => true,
TokenTree::Delimited(_, DelimToken::Bracket, _) => match prev {
TokenTree::Token(token) => match token.kind {
token::Pound => false,
_ => true,
},
_ => true,
},
TokenTree::Delimited(..) => true,
}
}

Expand Down Expand Up @@ -266,7 +278,7 @@ fn token_kind_to_string_ext(tok: &TokenKind, convert_dollar_crate: Option<Span>)
token::Shebang(s) => format!("/* shebang: {}*/", s),
token::Unknown(s) => s.to_string(),

token::Interpolated(ref nt, _) => nonterminal_to_string(nt),
token::Interpolated(ref nt) => nonterminal_to_string(nt),
}
}

Expand Down
15 changes: 6 additions & 9 deletions src/librustc_expand/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ use crate::module::DirectoryOwnership;
use rustc_ast::ast::{self, Attribute, NodeId, PatKind};
use rustc_ast::mut_visit::{self, MutVisitor};
use rustc_ast::ptr::P;
use rustc_ast::token::{self, FlattenGroup};
use rustc_ast::tokenstream::{self, TokenStream, TokenTree};
use rustc_ast::token;
use rustc_ast::tokenstream::{self, TokenStream};
use rustc_ast::visit::{AssocCtxt, Visitor};
use rustc_attr::{self as attr, Deprecation, HasAttrs, Stability};
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::sync::{self, Lrc};
use rustc_errors::{DiagnosticBuilder, ErrorReported};
use rustc_parse::{self, parser, MACRO_ARGUMENTS};
use rustc_parse::{self, nt_to_tokenstream, parser, MACRO_ARGUMENTS};
use rustc_session::{parse::ParseSess, Limit};
use rustc_span::def_id::DefId;
use rustc_span::edition::Edition;
Expand Down Expand Up @@ -120,10 +120,7 @@ impl Annotatable {
}
}

crate fn into_tokens(self) -> TokenStream {
// `Annotatable` can be converted into tokens directly, but we
// are packing it into a nonterminal as a piece of AST to make
// the produced token stream look nicer in pretty-printed form.
crate fn into_tokens(self, sess: &ParseSess) -> TokenStream {
let nt = match self {
Annotatable::Item(item) => token::NtItem(item),
Annotatable::TraitItem(item) | Annotatable::ImplItem(item) => {
Expand All @@ -142,7 +139,7 @@ impl Annotatable {
| Annotatable::StructField(..)
| Annotatable::Variant(..) => panic!("unexpected annotatable"),
};
TokenTree::token(token::Interpolated(Lrc::new(nt), FlattenGroup::Yes), DUMMY_SP).into()
nt_to_tokenstream(&nt, sess, DUMMY_SP)
}

pub fn expect_item(self) -> P<ast::Item> {
Expand Down Expand Up @@ -374,7 +371,7 @@ where
impl MutVisitor for AvoidInterpolatedIdents {
fn visit_tt(&mut self, tt: &mut tokenstream::TokenTree) {
if let tokenstream::TokenTree::Token(token) = tt {
if let token::Interpolated(nt, _) = &token.kind {
if let token::Interpolated(nt) = &token.kind {
if let token::NtIdent(ident, is_raw) = **nt {
*tt = tokenstream::TokenTree::token(
token::Ident(ident.name, is_raw),
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_expand/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
SyntaxExtensionKind::Attr(expander) => {
self.gate_proc_macro_input(&item);
self.gate_proc_macro_attr_item(span, &item);
let tokens = item.into_tokens();
let tokens = item.into_tokens(self.cx.parse_sess);
let attr_item = attr.unwrap_normal_item();
if let MacArgs::Eq(..) = attr_item.args {
self.cx.span_err(span, "key-value macro attributes are not supported");
Expand Down
8 changes: 4 additions & 4 deletions src/librustc_expand/mbe/macro_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -790,7 +790,7 @@ fn may_begin_with(token: &Token, name: Symbol) -> bool {
},
sym::block => match token.kind {
token::OpenDelim(token::Brace) => true,
token::Interpolated(ref nt, _) => match **nt {
token::Interpolated(ref nt) => match **nt {
token::NtItem(_)
| token::NtPat(_)
| token::NtTy(_)
Expand All @@ -804,7 +804,7 @@ fn may_begin_with(token: &Token, name: Symbol) -> bool {
},
sym::path | sym::meta => match token.kind {
token::ModSep | token::Ident(..) => true,
token::Interpolated(ref nt, _) => match **nt {
token::Interpolated(ref nt) => match **nt {
token::NtPath(_) | token::NtMeta(_) => true,
_ => may_be_ident(&nt),
},
Expand All @@ -823,12 +823,12 @@ fn may_begin_with(token: &Token, name: Symbol) -> bool {
token::ModSep | // path
token::Lt | // path (UFCS constant)
token::BinOp(token::Shl) => true, // path (double UFCS)
token::Interpolated(ref nt, _) => may_be_ident(nt),
token::Interpolated(ref nt) => may_be_ident(nt),
_ => false,
},
sym::lifetime => match token.kind {
token::Lifetime(_) => true,
token::Interpolated(ref nt, _) => match **nt {
token::Interpolated(ref nt) => match **nt {
token::NtLifetime(_) | token::NtTT(_) => true,
_ => false,
},
Expand Down
7 changes: 2 additions & 5 deletions src/librustc_expand/mbe/transcribe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::mbe::macro_parser::{MatchedNonterminal, MatchedSeq, NamedMatch};

use rustc_ast::ast::MacCall;
use rustc_ast::mut_visit::{self, MutVisitor};
use rustc_ast::token::{self, FlattenGroup, NtTT, Token};
use rustc_ast::token::{self, NtTT, Token};
use rustc_ast::tokenstream::{DelimSpan, TokenStream, TokenTree, TreeAndJoint};
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::sync::Lrc;
Expand Down Expand Up @@ -240,10 +240,7 @@ pub(super) fn transcribe<'a>(
result.push(tt.clone().into());
} else {
marker.visit_span(&mut sp);
let token = TokenTree::token(
token::Interpolated(nt.clone(), FlattenGroup::No),
sp,
);
let token = TokenTree::token(token::Interpolated(nt.clone()), sp);
result.push(token.into());
}
} else {
Expand Down
13 changes: 9 additions & 4 deletions src/librustc_expand/proc_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ use crate::base::{self, *};
use crate::proc_macro_server;

use rustc_ast::ast::{self, ItemKind, MetaItemKind, NestedMetaItem};
use rustc_ast::token::{self, FlattenGroup};
use rustc_ast::tokenstream::{self, TokenStream};
use rustc_ast::token;
use rustc_ast::tokenstream::{TokenStream, TokenTree};
use rustc_data_structures::sync::Lrc;
use rustc_errors::{Applicability, ErrorReported};
use rustc_parse::nt_to_tokenstream;
use rustc_span::symbol::sym;
use rustc_span::{Span, DUMMY_SP};

Expand Down Expand Up @@ -102,8 +103,12 @@ impl MultiItemModifier for ProcMacroDerive {
}
}

let token = token::Interpolated(Lrc::new(token::NtItem(item)), FlattenGroup::Yes);
let input = tokenstream::TokenTree::token(token, DUMMY_SP).into();
let item = token::NtItem(item);
let input = if item.pretty_printing_compatibility_hack() {
TokenTree::token(token::Interpolated(Lrc::new(item)), DUMMY_SP).into()
} else {
nt_to_tokenstream(&item, ecx.parse_sess, DUMMY_SP)
};

let server = proc_macro_server::Rustc::new(ecx);
let stream = match self.client.run(&EXEC_STRATEGY, server, input) {
Expand Down
Loading