Skip to content

Commit

Permalink
rustc: Implement tokenization of nested items
Browse files Browse the repository at this point in the history
Ever plagued by #43081 the compiler can return surprising spans in situations
related to procedural macros. This is exhibited by #47983 where whenever a
procedural macro is invoked in a nested item context it would fail to have
correct span information.

While #43230 provided a "hack" to cache the token stream used for each item in
the compiler it's not a full-blown solution. This commit continues to extend
this "hack" a bit more to work for nested items.

Previously in the parser the `parse_item` method would collect the tokens for an
item into a cache on the item itself. It turned out, however, that nested items
were parsed through the `parse_item_` method, so they didn't receive similar
treatment. To remedy this situation the hook for collecting tokens was moved
into `parse_item_` instead of `parse_item`.

Afterwards the token collection scheme was updated to support nested collection
of tokens. This is implemented by tracking `TokenStream` tokens instead of
`TokenTree` to allow for collecting items into streams at intermediate layers
and having them interleaved in the upper layers.

All in all, this...

Closes #47983
  • Loading branch information
alexcrichton committed Jul 22, 2018
1 parent a57d5d7 commit d760aaf
Show file tree
Hide file tree
Showing 5 changed files with 156 additions and 43 deletions.
119 changes: 76 additions & 43 deletions src/libsyntax/parse/parser.rs
Expand Up @@ -288,8 +288,8 @@ struct TokenCursorFrame {
/// on the parser.
#[derive(Clone)]
enum LastToken {
Collecting(Vec<TokenTree>),
Was(Option<TokenTree>),
Collecting(Vec<TokenStream>),
Was(Option<TokenStream>),
}

impl TokenCursorFrame {
Expand Down Expand Up @@ -326,8 +326,8 @@ impl TokenCursor {
};

match self.frame.last_token {
LastToken::Collecting(ref mut v) => v.push(tree.clone()),
LastToken::Was(ref mut t) => *t = Some(tree.clone()),
LastToken::Collecting(ref mut v) => v.push(tree.clone().into()),
LastToken::Was(ref mut t) => *t = Some(tree.clone().into()),
}

match tree {
Expand Down Expand Up @@ -6723,11 +6723,49 @@ impl<'a> Parser<'a> {
}
}

fn parse_item_(
&mut self,
attrs: Vec<Attribute>,
macros_allowed: bool,
attributes_allowed: bool,
) -> PResult<'a, Option<P<Item>>> {
let (ret, tokens) = self.collect_tokens(|this| {
this.parse_item_implementation(attrs, macros_allowed, attributes_allowed)
})?;

// Once we've parsed an item and recorded the tokens we got while
// parsing we may want to store `tokens` into the item we're about to
// return. Note, though, that we specifically didn't capture tokens
// related to outer attributes. The `tokens` field here may later be
// used with procedural macros to convert this item back into a token
// stream, but during expansion we may be removing attributes as we go
// along.
//
// If we've got inner attributes then the `tokens` we've got above holds
// these inner attributes. If an inner attribute is expanded we won't
// actually remove it from the token stream, so we'll just keep yielding
// it (bad!). To work around this case for now we just avoid recording
// `tokens` if we detect any inner attributes. This should help keep
// expansion correct, but we should fix this bug one day!
Ok(ret.map(|item| {
item.map(|mut i| {
if !i.attrs.iter().any(|attr| attr.style == AttrStyle::Inner) {
i.tokens = Some(tokens);
}
i
})
}))
}

/// Parse one of the items allowed by the flags.
/// NB: this function no longer parses the items inside an
/// extern crate.
fn parse_item_(&mut self, attrs: Vec<Attribute>,
macros_allowed: bool, attributes_allowed: bool) -> PResult<'a, Option<P<Item>>> {
fn parse_item_implementation(
&mut self,
attrs: Vec<Attribute>,
macros_allowed: bool,
attributes_allowed: bool,
) -> PResult<'a, Option<P<Item>>> {
maybe_whole!(self, NtItem, |item| {
let mut item = item.into_inner();
let mut attrs = attrs;
Expand Down Expand Up @@ -7260,12 +7298,15 @@ impl<'a> Parser<'a> {
{
// Record all tokens we parse when parsing this item.
let mut tokens = Vec::new();
match self.token_cursor.frame.last_token {
LastToken::Collecting(_) => {
panic!("cannot collect tokens recursively yet")
let prev_collecting = match self.token_cursor.frame.last_token {
LastToken::Collecting(ref mut list) => {
Some(mem::replace(list, Vec::new()))
}
LastToken::Was(ref mut last) => tokens.extend(last.take()),
}
LastToken::Was(ref mut last) => {
tokens.extend(last.take());
None
}
};
self.token_cursor.frame.last_token = LastToken::Collecting(tokens);
let prev = self.token_cursor.stack.len();
let ret = f(self);
Expand All @@ -7274,52 +7315,44 @@ impl<'a> Parser<'a> {
} else {
&mut self.token_cursor.stack[prev].last_token
};
let mut tokens = match *last_token {

// Pull our the toekns that we've collected from the call to `f` above
let mut collected_tokens = match *last_token {
LastToken::Collecting(ref mut v) => mem::replace(v, Vec::new()),
LastToken::Was(_) => panic!("our vector went away?"),
};

// If we're not at EOF our current token wasn't actually consumed by
// `f`, but it'll still be in our list that we pulled out. In that case
// put it back.
if self.token == token::Eof {
*last_token = LastToken::Was(None);
let extra_token = if self.token != token::Eof {
collected_tokens.pop()
} else {
*last_token = LastToken::Was(tokens.pop());
None
};

// If we were previously collecting tokens, then this was a recursive
// call. In that case we need to record all the tokens we collected in
// our parent list as well. To do that we push a clone of our stream
// onto the previous list.
let stream = collected_tokens.into_iter().collect::<TokenStream>();
match prev_collecting {
Some(mut list) => {
list.push(stream.clone());
list.extend(extra_token);
*last_token = LastToken::Collecting(list);
}
None => {
*last_token = LastToken::Was(extra_token);
}
}

Ok((ret?, tokens.into_iter().collect()))
Ok((ret?, stream))
}

pub fn parse_item(&mut self) -> PResult<'a, Option<P<Item>>> {
let attrs = self.parse_outer_attributes()?;

let (ret, tokens) = self.collect_tokens(|this| {
this.parse_item_(attrs, true, false)
})?;

// Once we've parsed an item and recorded the tokens we got while
// parsing we may want to store `tokens` into the item we're about to
// return. Note, though, that we specifically didn't capture tokens
// related to outer attributes. The `tokens` field here may later be
// used with procedural macros to convert this item back into a token
// stream, but during expansion we may be removing attributes as we go
// along.
//
// If we've got inner attributes then the `tokens` we've got above holds
// these inner attributes. If an inner attribute is expanded we won't
// actually remove it from the token stream, so we'll just keep yielding
// it (bad!). To work around this case for now we just avoid recording
// `tokens` if we detect any inner attributes. This should help keep
// expansion correct, but we should fix this bug one day!
Ok(ret.map(|item| {
item.map(|mut i| {
if !i.attrs.iter().any(|attr| attr.style == AttrStyle::Inner) {
i.tokens = Some(tokens);
}
i
})
}))
self.parse_item_(attrs, true, false)
}

/// `::{` or `::*`
Expand Down
1 change: 1 addition & 0 deletions src/libsyntax/tokenstream.rs
Expand Up @@ -340,6 +340,7 @@ impl TokenStream {
}
}

#[derive(Clone)]
pub struct TokenStreamBuilder(Vec<TokenStream>);

impl TokenStreamBuilder {
Expand Down
22 changes: 22 additions & 0 deletions src/test/ui-fulldeps/proc-macro/auxiliary/nested-item-spans.rs
@@ -0,0 +1,22 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// no-prefer-dynamic

#![crate_type = "proc-macro"]

extern crate proc_macro;

use proc_macro::*;

#[proc_macro_attribute]
pub fn foo(_: TokenStream, item: TokenStream) -> TokenStream {
item.into_iter().collect()
}
36 changes: 36 additions & 0 deletions src/test/ui-fulldeps/proc-macro/nested-item-spans.rs
@@ -0,0 +1,36 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// aux-build:nested-item-spans.rs

#![feature(use_extern_macros)]

extern crate nested_item_spans;

use nested_item_spans::foo;

#[foo]
fn another() {
fn bar() {
let x: u32 = "x"; //~ ERROR: mismatched types
}

bar();
}

fn main() {
#[foo]
fn bar() {
let x: u32 = "x"; //~ ERROR: mismatched types
}

bar();
another();
}
21 changes: 21 additions & 0 deletions src/test/ui-fulldeps/proc-macro/nested-item-spans.stderr
@@ -0,0 +1,21 @@
error[E0308]: mismatched types
--> $DIR/nested-item-spans.rs:22:22
|
LL | let x: u32 = "x"; //~ ERROR: mismatched types
| ^^^ expected u32, found reference
|
= note: expected type `u32`
found type `&'static str`

error[E0308]: mismatched types
--> $DIR/nested-item-spans.rs:31:22
|
LL | let x: u32 = "x"; //~ ERROR: mismatched types
| ^^^ expected u32, found reference
|
= note: expected type `u32`
found type `&'static str`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0308`.

0 comments on commit d760aaf

Please sign in to comment.