Force token collection to run when parsing nonterminals

Fixes #81007

Previously, we would fail to collect tokens in the proper place when
only builtin attributes were present. As a result, we would end up with
attribute tokens in the collected `TokenStream`, leading to duplication
when we attempted to prepend the attributes from the AST node.

We now explicitly track when token collection must be performed due to
nomterminal parsing.
This commit is contained in:
Aaron Hill 2021-01-18 16:47:37 -05:00
parent a4cbb44ae2
commit 11b1e37016
No known key found for this signature in database
GPG Key ID: B4087E510E98B164
13 changed files with 233 additions and 72 deletions

View File

@ -5,7 +5,8 @@ use rustc_ast::tokenstream::TokenStream;
use rustc_ast_pretty::pprust;
use rustc_expand::base::{self, *};
use rustc_expand::module::DirectoryOwnership;
use rustc_parse::{self, new_parser_from_file, parser::Parser};
use rustc_parse::parser::{ForceCollect, Parser};
use rustc_parse::{self, new_parser_from_file};
use rustc_session::lint::builtin::INCOMPLETE_INCLUDE;
use rustc_span::symbol::Symbol;
use rustc_span::{self, Pos, Span};
@ -139,7 +140,7 @@ pub fn expand_include<'cx>(
fn make_items(mut self: Box<ExpandResult<'a>>) -> Option<SmallVec<[P<ast::Item>; 1]>> {
let mut ret = SmallVec::new();
while self.p.token != token::Eof {
match self.p.parse_item() {
match self.p.parse_item(ForceCollect::No) {
Err(mut err) => {
err.emit();
break;

View File

@ -20,7 +20,7 @@ use rustc_data_structures::map_in_place::MapInPlace;
use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_errors::{struct_span_err, Applicability, PResult};
use rustc_feature::Features;
use rustc_parse::parser::{AttemptLocalParseRecovery, Parser};
use rustc_parse::parser::{AttemptLocalParseRecovery, ForceCollect, Parser};
use rustc_parse::validate_attr;
use rustc_session::lint::builtin::UNUSED_DOC_COMMENTS;
use rustc_session::lint::BuiltinLintDiagnostics;
@ -913,7 +913,7 @@ pub fn parse_ast_fragment<'a>(
Ok(match kind {
AstFragmentKind::Items => {
let mut items = SmallVec::new();
while let Some(item) = this.parse_item()? {
while let Some(item) = this.parse_item(ForceCollect::No)? {
items.push(item);
}
AstFragment::Items(items)

View File

@ -8,6 +8,7 @@ use rustc_ast::{self as ast, PatKind};
use rustc_ast_pretty::pprust::item_to_string;
use rustc_errors::PResult;
use rustc_parse::new_parser_from_source_str;
use rustc_parse::parser::ForceCollect;
use rustc_session::parse::ParseSess;
use rustc_span::source_map::FilePathMapping;
use rustc_span::symbol::{kw, sym, Symbol};
@ -29,7 +30,7 @@ fn parse_item_from_source_str(
source: String,
sess: &ParseSess,
) -> PResult<'_, Option<P<ast::Item>>> {
new_parser_from_source_str(sess, name, source).parse_item()
new_parser_from_source_str(sess, name, source).parse_item(ForceCollect::No)
}
// Produces a `rustc_span::span`.
@ -44,7 +45,7 @@ fn string_to_expr(source_str: String) -> P<ast::Expr> {
/// Parses a string, returns an item.
fn string_to_item(source_str: String) -> Option<P<ast::Item>> {
with_error_checking_parse(source_str, &sess(), |p| p.parse_item())
with_error_checking_parse(source_str, &sess(), |p| p.parse_item(ForceCollect::No))
}
#[should_panic]

View File

@ -9,6 +9,7 @@ use rustc_data_structures::sync::Lrc;
use rustc_errors::{struct_span_err, Applicability, ErrorReported};
use rustc_lexer::is_ident;
use rustc_parse::nt_to_tokenstream;
use rustc_parse::parser::ForceCollect;
use rustc_span::symbol::sym;
use rustc_span::{Span, DUMMY_SP};
@ -117,7 +118,7 @@ impl MultiItemModifier for ProcMacroDerive {
let mut items = vec![];
loop {
match parser.parse_item() {
match parser.parse_item(ForceCollect::No) {
Ok(None) => break,
Ok(Some(item)) => {
if is_stmt {

View File

@ -472,7 +472,11 @@ impl<'a> Parser<'a> {
/// Parses a prefix-unary-operator expr.
fn parse_prefix_expr(&mut self, attrs: Option<AttrVec>) -> PResult<'a, P<Expr>> {
let attrs = self.parse_or_use_outer_attributes(attrs)?;
let needs_tokens = super::attr::maybe_needs_tokens(&attrs);
// FIXME: Use super::attr::maybe_needs_tokens(&attrs) once we come up
// with a good way of passing `force_tokens` through from `parse_nonterminal`.
// Checking !attrs.is_empty() is correct, but will cause us to unnecessarily
// capture tokens in some circumstances.
let needs_tokens = !attrs.is_empty();
let do_parse = |this: &mut Parser<'a>| {
let lo = this.token.span;
// Note: when adding new unary operators, don't forget to adjust TokenKind::can_begin_expr()

View File

@ -1,8 +1,8 @@
use super::diagnostics::{dummy_arg, ConsumeClosingDelim, Error};
use super::ty::{AllowPlus, RecoverQPath, RecoverReturnSign};
use super::{FollowedByType, Parser, PathStyle};
use super::{FollowedByType, ForceCollect, Parser, PathStyle};
use crate::maybe_whole;
use crate::{maybe_collect_tokens, maybe_whole};
use rustc_ast::ptr::P;
use rustc_ast::token::{self, TokenKind};
@ -69,7 +69,7 @@ impl<'a> Parser<'a> {
unsafety: Unsafe,
) -> PResult<'a, Mod> {
let mut items = vec![];
while let Some(item) = self.parse_item()? {
while let Some(item) = self.parse_item(ForceCollect::No)? {
items.push(item);
self.maybe_consume_incorrect_semicolon(&items);
}
@ -93,13 +93,17 @@ impl<'a> Parser<'a> {
pub(super) type ItemInfo = (Ident, ItemKind);
impl<'a> Parser<'a> {
pub fn parse_item(&mut self) -> PResult<'a, Option<P<Item>>> {
self.parse_item_(|_| true).map(|i| i.map(P))
pub fn parse_item(&mut self, force_collect: ForceCollect) -> PResult<'a, Option<P<Item>>> {
self.parse_item_(|_| true, force_collect).map(|i| i.map(P))
}
fn parse_item_(&mut self, req_name: ReqName) -> PResult<'a, Option<Item>> {
fn parse_item_(
&mut self,
req_name: ReqName,
force_collect: ForceCollect,
) -> PResult<'a, Option<Item>> {
let attrs = self.parse_outer_attributes()?;
self.parse_item_common(attrs, true, false, req_name)
self.parse_item_common(attrs, true, false, req_name, force_collect)
}
pub(super) fn parse_item_common(
@ -108,6 +112,7 @@ impl<'a> Parser<'a> {
mac_allowed: bool,
attrs_allowed: bool,
req_name: ReqName,
force_collect: ForceCollect,
) -> PResult<'a, Option<Item>> {
maybe_whole!(self, NtItem, |item| {
let mut item = item;
@ -116,16 +121,12 @@ impl<'a> Parser<'a> {
Some(item.into_inner())
});
let needs_tokens = super::attr::maybe_needs_tokens(&attrs);
let mut unclosed_delims = vec![];
let parse_item = |this: &mut Self| {
let item = maybe_collect_tokens!(self, force_collect, &attrs, |this: &mut Self| {
let item = this.parse_item_common_(attrs, mac_allowed, attrs_allowed, req_name);
unclosed_delims.append(&mut this.unclosed_delims);
item
};
let item = if needs_tokens { self.collect_tokens(parse_item) } else { parse_item(self) }?;
})?;
self.unclosed_delims.append(&mut unclosed_delims);
Ok(item)
@ -731,20 +732,22 @@ impl<'a> Parser<'a> {
/// Parses associated items.
fn parse_assoc_item(&mut self, req_name: ReqName) -> PResult<'a, Option<Option<P<AssocItem>>>> {
Ok(self.parse_item_(req_name)?.map(|Item { attrs, id, span, vis, ident, kind, tokens }| {
let kind = match AssocItemKind::try_from(kind) {
Ok(kind) => kind,
Err(kind) => match kind {
ItemKind::Static(a, _, b) => {
self.struct_span_err(span, "associated `static` items are not allowed")
.emit();
AssocItemKind::Const(Defaultness::Final, a, b)
}
_ => return self.error_bad_item_kind(span, &kind, "`trait`s or `impl`s"),
},
};
Some(P(Item { attrs, id, span, vis, ident, kind, tokens }))
}))
Ok(self.parse_item_(req_name, ForceCollect::No)?.map(
|Item { attrs, id, span, vis, ident, kind, tokens }| {
let kind = match AssocItemKind::try_from(kind) {
Ok(kind) => kind,
Err(kind) => match kind {
ItemKind::Static(a, _, b) => {
self.struct_span_err(span, "associated `static` items are not allowed")
.emit();
AssocItemKind::Const(Defaultness::Final, a, b)
}
_ => return self.error_bad_item_kind(span, &kind, "`trait`s or `impl`s"),
},
};
Some(P(Item { attrs, id, span, vis, ident, kind, tokens }))
},
))
}
/// Parses a `type` alias with the following grammar:
@ -921,19 +924,21 @@ impl<'a> Parser<'a> {
/// Parses a foreign item (one in an `extern { ... }` block).
pub fn parse_foreign_item(&mut self) -> PResult<'a, Option<Option<P<ForeignItem>>>> {
Ok(self.parse_item_(|_| true)?.map(|Item { attrs, id, span, vis, ident, kind, tokens }| {
let kind = match ForeignItemKind::try_from(kind) {
Ok(kind) => kind,
Err(kind) => match kind {
ItemKind::Const(_, a, b) => {
self.error_on_foreign_const(span, ident);
ForeignItemKind::Static(a, Mutability::Not, b)
}
_ => return self.error_bad_item_kind(span, &kind, "`extern` blocks"),
},
};
Some(P(Item { attrs, id, span, vis, ident, kind, tokens }))
}))
Ok(self.parse_item_(|_| true, ForceCollect::No)?.map(
|Item { attrs, id, span, vis, ident, kind, tokens }| {
let kind = match ForeignItemKind::try_from(kind) {
Ok(kind) => kind,
Err(kind) => match kind {
ItemKind::Const(_, a, b) => {
self.error_on_foreign_const(span, ident);
ForeignItemKind::Static(a, Mutability::Not, b)
}
_ => return self.error_bad_item_kind(span, &kind, "`extern` blocks"),
},
};
Some(P(Item { attrs, id, span, vis, ident, kind, tokens }))
},
))
}
fn error_bad_item_kind<T>(&self, span: Span, kind: &ItemKind, ctx: &str) -> Option<T> {
@ -1515,7 +1520,7 @@ impl<'a> Parser<'a> {
{
let kw_token = self.token.clone();
let kw_str = pprust::token_to_string(&kw_token);
let item = self.parse_item()?;
let item = self.parse_item(ForceCollect::No)?;
self.struct_span_err(
kw_token.span,

View File

@ -54,6 +54,13 @@ enum BlockMode {
Ignore,
}
/// Whether or not we should force collection of tokens for an AST node,
/// regardless of whether or not it has attributes
pub enum ForceCollect {
Yes,
No,
}
/// Like `maybe_whole_expr`, but for things other than expressions.
#[macro_export]
macro_rules! maybe_whole {
@ -1413,3 +1420,16 @@ fn make_token_stream(
assert!(stack.is_empty(), "Stack should be empty: final_buf={:?} stack={:?}", final_buf, stack);
TokenStream::new(final_buf.inner)
}
#[macro_export]
macro_rules! maybe_collect_tokens {
($self:ident, $force_collect:expr, $attrs:expr, $f:expr) => {
if matches!($force_collect, ForceCollect::Yes)
|| $crate::parser::attr::maybe_needs_tokens($attrs)
{
$self.collect_tokens($f)
} else {
$f($self)
}
};
}

View File

@ -5,7 +5,7 @@ use rustc_errors::PResult;
use rustc_span::symbol::{kw, Ident};
use crate::parser::pat::{GateOr, RecoverComma};
use crate::parser::{FollowedByType, Parser, PathStyle};
use crate::parser::{FollowedByType, ForceCollect, Parser, PathStyle};
impl<'a> Parser<'a> {
/// Checks whether a non-terminal may begin with a particular token.
@ -98,7 +98,7 @@ impl<'a> Parser<'a> {
// in advance whether or not a proc-macro will be (transitively) invoked,
// we always capture tokens for any `Nonterminal` which needs them.
Ok(match kind {
NonterminalKind::Item => match self.collect_tokens(|this| this.parse_item())? {
NonterminalKind::Item => match self.parse_item(ForceCollect::Yes)? {
Some(item) => token::NtItem(item),
None => {
return Err(self.struct_span_err(self.token.span, "expected an item keyword"));
@ -107,7 +107,7 @@ impl<'a> Parser<'a> {
NonterminalKind::Block => {
token::NtBlock(self.collect_tokens(|this| this.parse_block())?)
}
NonterminalKind::Stmt => match self.collect_tokens(|this| this.parse_stmt())? {
NonterminalKind::Stmt => match self.parse_stmt(ForceCollect::Yes)? {
Some(s) => token::NtStmt(s),
None => {
return Err(self.struct_span_err(self.token.span, "expected a statement"));

View File

@ -3,8 +3,8 @@ use super::diagnostics::{AttemptLocalParseRecovery, Error};
use super::expr::LhsExpr;
use super::pat::{GateOr, RecoverComma};
use super::path::PathStyle;
use super::{BlockMode, Parser, Restrictions, SemiColonMode};
use crate::maybe_whole;
use super::{BlockMode, ForceCollect, Parser, Restrictions, SemiColonMode};
use crate::{maybe_collect_tokens, maybe_whole};
use rustc_ast as ast;
use rustc_ast::attr::HasAttrs;
@ -24,17 +24,21 @@ impl<'a> Parser<'a> {
/// Parses a statement. This stops just before trailing semicolons on everything but items.
/// e.g., a `StmtKind::Semi` parses to a `StmtKind::Expr`, leaving the trailing `;` unconsumed.
// Public for rustfmt usage.
pub fn parse_stmt(&mut self) -> PResult<'a, Option<Stmt>> {
Ok(self.parse_stmt_without_recovery().unwrap_or_else(|mut e| {
pub fn parse_stmt(&mut self, force_collect: ForceCollect) -> PResult<'a, Option<Stmt>> {
Ok(self.parse_stmt_without_recovery(force_collect).unwrap_or_else(|mut e| {
e.emit();
self.recover_stmt_(SemiColonMode::Break, BlockMode::Ignore);
None
}))
}
fn parse_stmt_without_recovery(&mut self) -> PResult<'a, Option<Stmt>> {
/// If `force_capture` is true, forces collection of tokens regardless of whether
/// or not we have attributes
fn parse_stmt_without_recovery(
&mut self,
force_collect: ForceCollect,
) -> PResult<'a, Option<Stmt>> {
let mut attrs = self.parse_outer_attributes()?;
let has_attrs = !attrs.is_empty();
let lo = self.token.span;
maybe_whole!(self, NtStmt, |stmt| {
@ -46,7 +50,7 @@ impl<'a> Parser<'a> {
Some(stmt)
});
let parse_stmt_inner = |this: &mut Self| {
maybe_collect_tokens!(self, force_collect, &attrs, |this: &mut Self| {
let stmt = if this.eat_keyword(kw::Let) {
this.parse_local_mk(lo, attrs.into())?
} else if this.is_kw_followed_by_ident(kw::Mut) {
@ -69,7 +73,7 @@ impl<'a> Parser<'a> {
// Also, we avoid stealing syntax from `parse_item_`.
this.parse_stmt_path_start(lo, attrs)?
} else if let Some(item) =
this.parse_item_common(attrs.clone(), false, true, |_| true)?
this.parse_item_common(attrs.clone(), false, true, |_| true, force_collect)?
{
// FIXME: Bad copy of attrs
this.mk_stmt(lo.to(item.span), StmtKind::Item(P(item)))
@ -86,14 +90,7 @@ impl<'a> Parser<'a> {
return Ok(None);
};
Ok(Some(stmt))
};
let stmt = if has_attrs {
self.collect_tokens(parse_stmt_inner)?
} else {
parse_stmt_inner(self)?
};
Ok(stmt)
})
}
fn parse_stmt_path_start(&mut self, lo: Span, attrs: Vec<Attribute>) -> PResult<'a, Stmt> {
@ -292,7 +289,7 @@ impl<'a> Parser<'a> {
// bar;
//
// which is valid in other languages, but not Rust.
match self.parse_stmt_without_recovery() {
match self.parse_stmt_without_recovery(ForceCollect::No) {
// If the next token is an open brace (e.g., `if a b {`), the place-
// inside-a-block suggestion would be more likely wrong than right.
Ok(Some(_))
@ -395,7 +392,7 @@ impl<'a> Parser<'a> {
// Skip looking for a trailing semicolon when we have an interpolated statement.
maybe_whole!(self, NtStmt, |x| Some(x));
let mut stmt = match self.parse_stmt_without_recovery()? {
let mut stmt = match self.parse_stmt_without_recovery(ForceCollect::No)? {
Some(stmt) => stmt,
None => return Ok(None),
};

View File

@ -423,6 +423,7 @@ crate fn make_test(
use rustc_errors::emitter::{Emitter, EmitterWriter};
use rustc_errors::Handler;
use rustc_parse::maybe_new_parser_from_source_str;
use rustc_parse::parser::ForceCollect;
use rustc_session::parse::ParseSess;
use rustc_span::source_map::FilePathMapping;
@ -459,7 +460,7 @@ crate fn make_test(
};
loop {
match parser.parse_item() {
match parser.parse_item(ForceCollect::No) {
Ok(Some(item)) => {
if !found_main {
if let ast::ItemKind::Fn(..) = item.kind {

View File

@ -0,0 +1,31 @@
// check-pass
// edition:2018
// compile-flags: -Z span-debug
// aux-build:test-macros.rs
#![feature(rustc_attrs)]
#![no_std] // Don't load unnecessary hygiene information from std
extern crate std;
#[macro_use] extern crate test_macros;
macro_rules! capture_item {
($item:item) => {
#[print_attr]
$item
}
}
capture_item! {
/// A doc comment
struct Foo {}
}
capture_item! {
#[rustc_dummy]
/// Another comment comment
struct Bar {}
}
fn main() {}

View File

@ -0,0 +1,99 @@
PRINT-ATTR INPUT (DISPLAY): #[doc = r" A doc comment"] struct Foo { }
PRINT-ATTR INPUT (DEBUG): TokenStream [
Punct {
ch: '#',
spacing: Alone,
span: $DIR/issue-81007-item-attrs.rs:21:5: 21:22 (#0),
},
Group {
delimiter: Bracket,
stream: TokenStream [
Ident {
ident: "doc",
span: $DIR/issue-81007-item-attrs.rs:21:5: 21:22 (#0),
},
Punct {
ch: '=',
spacing: Alone,
span: $DIR/issue-81007-item-attrs.rs:21:5: 21:22 (#0),
},
Literal {
kind: StrRaw(0),
symbol: " A doc comment",
suffix: None,
span: $DIR/issue-81007-item-attrs.rs:21:5: 21:22 (#0),
},
],
span: $DIR/issue-81007-item-attrs.rs:21:5: 21:22 (#0),
},
Ident {
ident: "struct",
span: $DIR/issue-81007-item-attrs.rs:22:5: 22:11 (#0),
},
Ident {
ident: "Foo",
span: $DIR/issue-81007-item-attrs.rs:22:12: 22:15 (#0),
},
Group {
delimiter: Brace,
stream: TokenStream [],
span: $DIR/issue-81007-item-attrs.rs:22:16: 22:18 (#0),
},
]
PRINT-ATTR INPUT (DISPLAY): #[rustc_dummy] #[doc = r" Another comment comment"] struct Bar { }
PRINT-ATTR INPUT (DEBUG): TokenStream [
Punct {
ch: '#',
spacing: Alone,
span: $DIR/issue-81007-item-attrs.rs:26:5: 26:6 (#0),
},
Group {
delimiter: Bracket,
stream: TokenStream [
Ident {
ident: "rustc_dummy",
span: $DIR/issue-81007-item-attrs.rs:26:7: 26:18 (#0),
},
],
span: $DIR/issue-81007-item-attrs.rs:26:6: 26:19 (#0),
},
Punct {
ch: '#',
spacing: Alone,
span: $DIR/issue-81007-item-attrs.rs:27:5: 27:32 (#0),
},
Group {
delimiter: Bracket,
stream: TokenStream [
Ident {
ident: "doc",
span: $DIR/issue-81007-item-attrs.rs:27:5: 27:32 (#0),
},
Punct {
ch: '=',
spacing: Alone,
span: $DIR/issue-81007-item-attrs.rs:27:5: 27:32 (#0),
},
Literal {
kind: StrRaw(0),
symbol: " Another comment comment",
suffix: None,
span: $DIR/issue-81007-item-attrs.rs:27:5: 27:32 (#0),
},
],
span: $DIR/issue-81007-item-attrs.rs:27:5: 27:32 (#0),
},
Ident {
ident: "struct",
span: $DIR/issue-81007-item-attrs.rs:28:5: 28:11 (#0),
},
Ident {
ident: "Bar",
span: $DIR/issue-81007-item-attrs.rs:28:12: 28:15 (#0),
},
Group {
delimiter: Brace,
stream: TokenStream [],
span: $DIR/issue-81007-item-attrs.rs:28:16: 28:18 (#0),
},
]

View File

@ -12,6 +12,7 @@ use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty;
use rustc_parse::maybe_new_parser_from_source_str;
use rustc_parse::parser::ForceCollect;
use rustc_session::parse::ParseSess;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::edition::Edition;
@ -483,7 +484,7 @@ fn check_code(cx: &LateContext<'_>, text: &str, edition: Edition, span: Span) {
let mut relevant_main_found = false;
loop {
match parser.parse_item() {
match parser.parse_item(ForceCollect::No) {
Ok(Some(item)) => match &item.kind {
// Tests with one of these items are ignored
ItemKind::Static(..)